GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Create type aliases for unresolved type symbols by ahejlsberg · Pull Request #45976 · microsoft/TypeScript

Conversation

Anders Hejlsberg (ahejlsberg) commented Sep 20, 2021

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

treybrisbane, tonivj5, DanielRosenwasser, and fowl2 reacted with thumbs up emoji

All reactions

👍 4 reactions

Anders Hejlsberg (ahejlsberg) added 3 commits

September 20, 2021 14:26


          Create type aliases for unresolved type symbols

981cbb3


          Accept new baselines

17332e1


          Update fourslash tests

3d34ca7

TypeScript Bot (typescript-bot) assigned Anders Hejlsberg (ahejlsberg)

Sep 20, 2021

TypeScript Bot (typescript-bot) added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels

Sep 20, 2021

Anders Hejlsberg (ahejlsberg) requested review from Daniel Rosenwasser (DanielRosenwasser) and Andrew Branch (andrewbranch)

September 20, 2021 23:54

Daniel Rosenwasser (DanielRosenwasser) requested changes

Sep 21, 2021

View reviewed changes

Daniel Rosenwasser (DanielRosenwasser) left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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 };

Daniel Rosenwasser (DanielRosenwasser) Sep 21, 2021

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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 };

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Anders Hejlsberg (ahejlsberg) Sep 22, 2021

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sure, I'll add a comment.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/compiler/checker.ts

		}

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

Daniel Rosenwasser (DanielRosenwasser) Sep 21, 2021

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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;

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Anders Hejlsberg (ahejlsberg) Sep 22, 2021

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

DanielRosenwasser reacted with thumbs up emoji

All reactions

👍 1 reaction

Comment thread

tests/baselines/reference/asyncFunctionDeclaration13_es6.types

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

Daniel Rosenwasser (DanielRosenwasser) Sep 21, 2021

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Anders Hejlsberg (ahejlsberg) Sep 22, 2021

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Daniel Rosenwasser (DanielRosenwasser) Sep 22, 2021

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Anders Hejlsberg (ahejlsberg) Sep 23, 2021

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Daniel Rosenwasser (DanielRosenwasser) commented Sep 21, 2021 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Unresolved import aliases create tagged unresolved symbols

8f828c4

Anders Hejlsberg (ahejlsberg) commented Sep 22, 2021

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Daniel Rosenwasser (DanielRosenwasser) commented Sep 22, 2021

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Daniel Rosenwasser (DanielRosenwasser) commented Sep 22, 2021

Copy link

Copy Markdown

Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Anders Hejlsberg (ahejlsberg) added 4 commits

September 22, 2021 17:49


          Add comments

891f961


          Merge branch 'main' into fix45893

e949c85

# Conflicts:
#	src/compiler/checker.ts
#	tests/cases/fourslash/findReferencesJSXTagName.ts
#	tests/cases/fourslash/tsxFindAllReferences10.ts


          Accept new baselines


          Add fourslash tests

9d01571

Andrew Branch (andrewbranch) approved these changes

Sep 23, 2021

View reviewed changes

Anders Hejlsberg (ahejlsberg) merged commit a4f9bf0 into main

Sep 23, 2021

Anders Hejlsberg (ahejlsberg) deleted the fix45893 branch

September 23, 2021 20:21

Microsoft (microsoft) locked as resolved and limited conversation to collaborators

Oct 21, 2025

Reviewers

Daniel Rosenwasser (DanielRosenwasser)

DanielRosenwasser requested changes

Andrew Branch (andrewbranch)

andrewbranch approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

Anders Hejlsberg (ahejlsberg)

Labels

Author: Team For Milestone Bug

PRs that fix a bug with a specific milestone

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve display experience for unresolved symbols

Uh oh!

There was an error while loading. Please reload this page.

4 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

Anders Hejlsberg (ahejlsberg) commented Sep 20, 2021

Uh oh!

Daniel Rosenwasser (DanielRosenwasser) left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Daniel Rosenwasser (DanielRosenwasser) commented Sep 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Anders Hejlsberg (ahejlsberg) commented Sep 22, 2021

Uh oh!

Daniel Rosenwasser (DanielRosenwasser) commented Sep 22, 2021

Uh oh!

Daniel Rosenwasser (DanielRosenwasser) commented Sep 22, 2021

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Daniel Rosenwasser (DanielRosenwasser) left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Daniel Rosenwasser (DanielRosenwasser) commented Sep 21, 2021 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.