-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Fix getTypeAtLocation for as const
to not issue a diagnostic
#36741
Conversation
@@ -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` |
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.
Instead of this shouldn't getTypeOfNode
just check if it is const
of as const
expression should forward it to correct node instead?
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.
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).
Is this related to #36490? I believe that PR was also about handling LS requests in larger spans (i.e. not just on identifiers). |
Not as far as I know - this is just about fixing us giving a bogus result (and adding a diagnostic) for the type of |
So it changes from wrong to right, rather than from nothing to something? Makes sense. |
Fixes #34913
checkAssertion
in the normal codepath avoids calling intogetTypeFromTypeNode
on theconst
inas const
expressions, however the checkergetTypeAtLocation
API may invoke it directly. Previously, this'd cause as resolution error, as we'd resolve an (entirely unused) variable namedconst
and return its' type. With this change, we instead return the type of the expression associated with theconst
(which should be more useful to API consumers, I hope, and prevents any spurious errors from being added).