Skip to content

Fixed a crash in completions on functionlike types#56257

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.

Sign up for GitHub

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
Jake Bailey (jakebailey) merged 2 commits into from
Nov 15, 2023
Merged

Fixed a crash in completions on functionlike types #56257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c4fae6
Fixed a crash in completions on functionlike types
Andarist Oct 30, 2023
0009f7f
Tighten checks even more
Andarist Oct 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1868,7 +1868,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const nodeLinks = getNodeLinks(node);
cachedResolvedSignatures.push([nodeLinks, nodeLinks.resolvedSignature] as const);
nodeLinks.resolvedSignature = undefined;
if (isFunctionLike(node)) {
if (isFunctionExpressionOrArrowFunction(node)) {
const symbolLinks = getSymbolLinks(getSymbolOfDeclaration(node));

@jakebaileyJake Bailey (jakebailey)Nov 13, 2023

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.

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?

@AndaristMateusz Burzyński (Andarist)Nov 13, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

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.

@jakebaileyJake Bailey (jakebailey)Nov 13, 2023

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.

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.

const type = symbolLinks.type;
cachedTypes.push([symbolLinks, type] as const);
Expand Down
7 changes: 5 additions & 2 deletions src/compiler/utilitiesPublic.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,6 +5,7 @@ import {
ArrayBindingElement,
ArrayBindingOrAssignmentElement,
ArrayBindingOrAssignmentPattern,
ArrowFunction,
AssertionExpression,
AssignmentDeclarationKind,
AssignmentPattern,
Expand DownExpand Up@@ -64,6 +65,7 @@ import {
ForInitializer,
ForInOrOfStatement,
FunctionBody,
FunctionExpression,
FunctionLikeDeclaration,
FunctionTypeNode,
GeneratedIdentifier,
Expand DownExpand Up@@ -119,6 +121,7 @@ import {
isExportSpecifier,
isFunctionBlock,
isFunctionExpression,
isFunctionExpressionOrArrowFunction,
isFunctionTypeNode,
isIdentifier,
isImportSpecifier,
Expand DownExpand Up@@ -1928,8 +1931,8 @@ export function isPropertyAccessOrQualifiedName(node: Node): node is PropertyAcc
}

/** @internal */
export function isCallLikeOrFunctionLikeExpression(node: Node): node is CallLikeExpression | SignatureDeclaration {

@jakebaileyJake Bailey (jakebailey)Nov 13, 2023

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.

Was this function really not used elsewhere? Super surprised, but it does seem to be true.

return isCallLikeExpression(node) || isFunctionLike(node);
export function isCallLikeOrFunctionLikeExpression(node: Node): node is CallLikeExpression | FunctionExpression | ArrowFunction {
return isCallLikeExpression(node) || isFunctionExpressionOrArrowFunction(node);
}

export function isCallLikeExpression(node: Node): node is CallLikeExpression {
Expand Down
13 changes: 13 additions & 0 deletions tests/cases/fourslash/jsDocFunctionTypeCompletionsNoCrash.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
///<reference path="fourslash.ts" />

//// /**
//// * @returns {function/**/(): string}
//// */
//// function updateCalendarEvent() {
//// return "";
//// }

verify.completions({
marker: "",
exact: completion.globalTypes
});