Fixed a crash in completions on functionlike types #56257
Conversation
typescript-bot
added
the
For Uncommitted Bug
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
src/compiler/utilitiesPublic.ts
Outdated
| export function isCallLikeOrFunctionLikeExpression(node: Node): node is CallLikeExpression | FunctionLikeDeclaration { | ||
| return isCallLikeExpression(node) || isFunctionLikeDeclaration(node); | ||
| } |
There was a problem hiding this comment.
I believe that this reflects the name better and perhaps the change reflects the original intention
There was a problem hiding this comment.
I guess I don't expect this to return true for function declarations, except those in expression positions... am I missing something?
There was a problem hiding this comment.
Yee, you are right. I limited this further to only return true for function expressions and arrows
There was a problem hiding this comment.
Are you trying to limit to functions that can be contextually typed? What about object literal method declarations?
There was a problem hiding this comment.
Are you trying to limit to functions that can be contextually typed?
Somewhat. IIRC I meant to limit it to expression-space functions and both of those "sets" are almost the same. I think that either would be OK here (at least for the purpose of my fix).
What about object literal method declarations?
They are already covered by isFunctionLikeDeclaration check
|
It feels like this is partially related to #51104. But I'm okay with a limited fix. |
Yeah, it looks very much related. |
| } | ||
|
|
||
| /** @internal */ | ||
| export function isCallLikeOrFunctionLikeExpression(node: Node): node is CallLikeExpression | SignatureDeclaration { |
There was a problem hiding this comment.
Was this function really not used elsewhere? Super surprised, but it does seem to be true.
| nodeLinks.resolvedSignature = undefined; | ||
| if (isFunctionLike(node)) { | ||
| if (isFunctionExpressionOrArrowFunction(node)) { | ||
| const symbolLinks = getSymbolLinks(getSymbolOfDeclaration(node)); |
There was a problem hiding this comment.
Maybe I'm wrong here, but is it the case that this could also be fixed by instead checking if the declaration has a symbol?
There was a problem hiding this comment.
Maybe I'm wrong here
You are not 😉
but is it the case that this could also be fixed by instead checking if the declaration has a symbol?
It could have been fixed like that but the possibility of lacking a symbol isn't reflected by the types. I also just noticed that this kinda happened because I was using a function that was matching more types than I wanted so this just seemed like a slightly better fix.
There was a problem hiding this comment.
the possibility of lacking a symbol isn't reflected by the types
This is an ongoing struggle; I should really resurrect my PR which attempts to make it known-optional.
|
@DanielRosenwasser This is a new crash in 5.3; I don't know that it's required pre-release, but I think this one should be backported. |
|
@typescript-bot cherry-pick this to release-5.3 |
|
Heya @jakebailey, I've started to run the task to cherry-pick this into |
|
Hey @jakebailey, I couldn't open a PR with the cherry-pick. (You can check the log here). You may need to squash and pick this PR into release-5.3 manually. |
5 participants
reported here
closes #56402