FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Create type aliases for unresolved type symbols by ahejlsberg · Pull Request #45976 · microsoft/TypeScript · GitHub

Create type aliases for unresolved type symbols - #45976

Merged
Anders Hejlsberg (ahejlsberg) merged 8 commits into
mainfrom
fix45893
Sep 23, 2021
Merged

Create type aliases for unresolved type symbols#45976
Anders Hejlsberg (ahejlsberg) merged 8 commits into
mainfrom
fix45893

Conversation

Copy link
Copy Markdown
Member

With this PR we create type aliases for unresolved symbols such that quick info and error messages list back the unresolved name instead of just any.

In this example where ItemData is not defined:

function foo(items: ItemData[]) {
    let item = items[0];
}

hovering over item and items now shows ItemData and ItemData[] where previously we'd just show any and any[]. Furthermore, hovering over the unresolved ItemData reference shows type ItemData = /*unresolved*/ any.

Fixes #45893.

Daniel Rosenwasser (DanielRosenwasser) left a comment
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Largely looks good, though I think this needs fourslash and possibly syntax server tests, specifically for

  • find all references
  • go to definition (at least the fact that it doesn't work)
  • quick info

Comment thread src/compiler/checker.ts
const typeNode = nodeBuilder.typeToTypeNode(type, enclosingDeclaration, toNodeBuilderFlags(flags) | NodeBuilderFlags.IgnoreErrors | (noTruncation ? NodeBuilderFlags.NoTruncation : 0), writer);
if (typeNode === undefined) return Debug.fail("should always get typenode");
const options = { removeComments: true };
const options = { removeComments: type !== unresolvedType };

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Would be helpful to leave a comment on this one (pun intended).

Suggested change
const options = { removeComments: type !== unresolvedType };
// The unresolved type gets a synthesized comment on `any`
// to hint to users that it's not a plain `any`.
// Otherwise, we always strip comments out.
const options = { removeComments: type !== unresolvedType };

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sure, I'll add a comment.

Comment thread src/compiler/checker.ts
}

function isErrorType(type: Type) {
return type === errorType || !!(type.flags & TypeFlags.Any && type.aliasSymbol);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Why not just this? Seems like it'd be cheaper too.

Suggested change
return type === errorType || !!(type.flags & TypeFlags.Any && type.aliasSymbol);
return type === errorType || type === unresolvedType;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

No, that wouldn't work. I'll add a clarifying comment, but basically we want to identify the special TypeFlags.Any types produced by getTypeForTypeAliasReference for an unresolved symbol. The sole purpose of unresolvedType is to act as the declared type for the special type alias symbols we create for unresolved names. The type is never actually returned by anything because getTypeForTypeAliasReference maps it into a special TypeFlags.Any type with an aliasSymbol.

// Legal to use 'await' in a type context.
var v: await;
>v : any
>v : await

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is there some way to make the type writer a little smarter here so that the team can differentiate between an unresolved and a concrete type?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We certainly have the ability to tell the difference, but what did you have in mind in terms of output? Remember that we're often dealing with composed types, e.g. for var v: Promise<await[]> we can't easily attach a comment to the await identifier in the output.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

A parenthesized type might work like (/*unresolved*/ await)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm not really sure we want to do that. We're already generate an error at every reference to an unresolved symbol, I don't think we also want the extra noise in the type baselines.

Daniel Rosenwasser (DanielRosenwasser) commented Sep 21, 2021
edited
Loading

Copy link
Copy Markdown
Member

I guess this doesn't technically fix #38836 because this only applies to types, right?

Copy link
Copy Markdown
Member Author

Right, this doesn't fix #38836. One complication there would be what scope to introduce the unknown value symbols into. Should they be locals of the innermost enclosing function, or just globals like we do for the type symbols? The latter typically makes sense for types, but less clear for unresolved value symbols.

Copy link
Copy Markdown
Member

or just globals like we do for the type symbols

I think just a global scope makes enough sense; if you do have a bunch of unresolved variables spread throughout your codebase, it's convenient to find-all-references, and likely to be a global anyhow; if it's mostly local (e.g. a user forgot to locally declare it), it doesn't really matter where you put it. But I'd understand if we wanted to take it one step at a time.

Copy link
Copy Markdown
Member

Looks like there's a few conflicts that need to be resolved.

# Conflicts:
#	src/compiler/checker.ts
#	tests/cases/fourslash/findReferencesJSXTagName.ts
#	tests/cases/fourslash/tsxFindAllReferences10.ts
Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Milestone Bug PRs that fix a bug with a specific milestone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve display experience for unresolved symbols

4 participants


Back | FazBrowse Home | New Git URL