Skip to content

Fix getTypeAtLocation for as const to not issue a diagnostic #36741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

weswigham
Copy link
Member

@weswigham weswigham commented Feb 11, 2020

Fixes #34913

checkAssertion in the normal codepath avoids calling into getTypeFromTypeNode on the const in as const expressions, however the checker getTypeAtLocation API may invoke it directly. Previously, this'd cause as resolution error, as we'd resolve an (entirely unused) variable named const and return its' type. With this change, we instead return the type of the expression associated with the const (which should be more useful to API consumers, I hope, and prevents any spurious errors from being added).

@@ -11336,6 +11336,11 @@ namespace ts {
function getTypeFromTypeReference(node: TypeReferenceType): Type {
const links = getNodeLinks(node);
if (!links.resolvedType) {
// handle LS queries on the `const` in `x as const` by resolving to the type of `x`
Copy link
Member

Choose a reason for hiding this comment

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

Instead of this shouldn't getTypeOfNode just check if it is const of as const expression should forward it to correct node instead?

Copy link
Member Author

@weswigham weswigham Feb 11, 2020

Choose a reason for hiding this comment

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

Ehhhhhh, it could, but I'd rather push the handling to here, to prevent us from ever attempting to resolve the const in as const as a real type reference, however we may get there in the future (plus, by doing it here the result is cached).

@weswigham weswigham merged commit 2b64731 into microsoft:master Feb 12, 2020
@weswigham weswigham deleted the fix-ls-gettypeatlocation-for-as-const branch February 12, 2020 21:43
@amcasey
Copy link
Member

amcasey commented Feb 12, 2020

Is this related to #36490? I believe that PR was also about handling LS requests in larger spans (i.e. not just on identifiers).

@weswigham
Copy link
Member Author

Not as far as I know - this is just about fixing us giving a bogus result (and adding a diagnostic) for the type of const in x as const when the LS requests the type at const.

@amcasey
Copy link
Member

amcasey commented Feb 12, 2020

So it changes from wrong to right, rather than from nothing to something? Makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using getTypeAtLocation on a const type reference creates a TS2304 diagnostic
4 participants