From 69e7e57b15d535875fb3cd1aa630c24971320d88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Fri, 5 Apr 2024 00:45:35 +0200 Subject: [PATCH] Fixed crashes when looking up symbols of jsdoc nodes in TS files (#57110) Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> --- src/compiler/checker.ts | 4 + src/services/services.ts | 11 +- ...ndingPatternInJsdocNoCrash1.baseline.jsonc | 45 +++++++ ...ndingPatternInJsdocNoCrash2.baseline.jsonc | 45 +++++++ .../findReferencesSeeTagInTs.baseline.jsonc | 111 ++++++++++++++++++ ...ReferencesBindingPatternInJsdocNoCrash1.ts | 33 ++++++ ...ReferencesBindingPatternInJsdocNoCrash2.ts | 33 ++++++ .../fourslash/findReferencesSeeTagInTs.ts | 9 ++ .../quickInfoBindingPatternInJsdocNoCrash1.ts | 10 ++ .../fourslash/quickInfoInJsdocInTsFile1.ts | 62 ++++++++++ 10 files changed, 362 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/findReferencesBindingPatternInJsdocNoCrash1.baseline.jsonc create mode 100644 tests/baselines/reference/findReferencesBindingPatternInJsdocNoCrash2.baseline.jsonc create mode 100644 tests/baselines/reference/findReferencesSeeTagInTs.baseline.jsonc create mode 100644 tests/cases/fourslash/findReferencesBindingPatternInJsdocNoCrash1.ts create mode 100644 tests/cases/fourslash/findReferencesBindingPatternInJsdocNoCrash2.ts create mode 100644 tests/cases/fourslash/findReferencesSeeTagInTs.ts create mode 100644 tests/cases/fourslash/quickInfoBindingPatternInJsdocNoCrash1.ts create mode 100644 tests/cases/fourslash/quickInfoInJsdocInTsFile1.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index b063de5cc1a..3c344f32f9a 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11272,6 +11272,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { } if (isParameter(declaration)) { + if (!declaration.symbol) { + // parameters of function types defined in JSDoc in TS files don't have symbols + return; + } const func = declaration.parent as FunctionLikeDeclaration; // For a parameter of a set accessor, use the type of the get accessor if one is present if (func.kind === SyntaxKind.SetAccessor && hasBindableName(func)) { diff --git a/src/services/services.ts b/src/services/services.ts index 32a54a89724..c175153f240 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -151,6 +151,7 @@ import { isIdentifier, isImportMeta, isInComment, + isInJSFile, isInsideJsxElement, isInsideJsxElementOrAttribute, isInString, @@ -245,6 +246,7 @@ import { PrivateIdentifier, Program, PropertyName, + PropertySignature, QuickInfo, refactor, RefactorContext, @@ -2067,7 +2069,6 @@ export function createLanguageService( const typeChecker = program.getTypeChecker(); const nodeForQuickInfo = getNodeForQuickInfo(node); const symbol = getSymbolAtLocationForQuickInfo(nodeForQuickInfo, typeChecker); - if (!symbol || typeChecker.isUnknownSymbol(symbol)) { const type = shouldGetType(sourceFile, nodeForQuickInfo, position) ? typeChecker.getTypeAtLocation(nodeForQuickInfo) : undefined; return type && { @@ -2110,6 +2111,14 @@ export function createLanguageService( function shouldGetType(sourceFile: SourceFile, node: Node, position: number): boolean { switch (node.kind) { case SyntaxKind.Identifier: + if ( + node.flags & NodeFlags.JSDoc && !isInJSFile(node) && + ((node.parent.kind === SyntaxKind.PropertySignature && (node.parent as PropertySignature).name === node) || + findAncestor(node, n => n.kind === SyntaxKind.Parameter)) + ) { + // if we'd request type at those locations we'd get `errorType` that displays confusingly as `any` + return false; + } return !isLabelName(node) && !isTagName(node) && !isConstTypeReference(node.parent); case SyntaxKind.PropertyAccessExpression: case SyntaxKind.QualifiedName: diff --git a/tests/baselines/reference/findReferencesBindingPatternInJsdocNoCrash1.baseline.jsonc b/tests/baselines/reference/findReferencesBindingPatternInJsdocNoCrash1.baseline.jsonc new file mode 100644 index 00000000000..dab26747742 --- /dev/null +++ b/tests/baselines/reference/findReferencesBindingPatternInJsdocNoCrash1.baseline.jsonc @@ -0,0 +1,45 @@ +// === findAllReferences === +// === /tests/cases/fourslash/src/index.ts === +// import { useQuery } from "use-query"; +// <|const { /*FIND ALL REFS*/[|{| isWriteAccess: true, isDefinition: true |}data|] } = useQuery();|> + + // === Definitions === + // === /tests/cases/fourslash/src/index.ts === + // import { useQuery } from "use-query"; + // <|const { /*FIND ALL REFS*/[|data|] } = useQuery();|> + + // === Details === + [ + { + "containerKind": "", + "containerName": "", + "kind": "const", + "name": "const data: any", + "displayParts": [ + { + "text": "const", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "data", + "kind": "localName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "any", + "kind": "keyword" + } + ] + } + ] \ No newline at end of file diff --git a/tests/baselines/reference/findReferencesBindingPatternInJsdocNoCrash2.baseline.jsonc b/tests/baselines/reference/findReferencesBindingPatternInJsdocNoCrash2.baseline.jsonc new file mode 100644 index 00000000000..dab26747742 --- /dev/null +++ b/tests/baselines/reference/findReferencesBindingPatternInJsdocNoCrash2.baseline.jsonc @@ -0,0 +1,45 @@ +// === findAllReferences === +// === /tests/cases/fourslash/src/index.ts === +// import { useQuery } from "use-query"; +// <|const { /*FIND ALL REFS*/[|{| isWriteAccess: true, isDefinition: true |}data|] } = useQuery();|> + + // === Definitions === + // === /tests/cases/fourslash/src/index.ts === + // import { useQuery } from "use-query"; + // <|const { /*FIND ALL REFS*/[|data|] } = useQuery();|> + + // === Details === + [ + { + "containerKind": "", + "containerName": "", + "kind": "const", + "name": "const data: any", + "displayParts": [ + { + "text": "const", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "data", + "kind": "localName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "any", + "kind": "keyword" + } + ] + } + ] \ No newline at end of file diff --git a/tests/baselines/reference/findReferencesSeeTagInTs.baseline.jsonc b/tests/baselines/reference/findReferencesSeeTagInTs.baseline.jsonc new file mode 100644 index 00000000000..35b6ad75d10 --- /dev/null +++ b/tests/baselines/reference/findReferencesSeeTagInTs.baseline.jsonc @@ -0,0 +1,111 @@ +// === findAllReferences === +// === /tests/cases/fourslash/findReferencesSeeTagInTs.ts === +// <|function [|{| isWriteAccess: true, isDefinition: true |}doStuffWithStuff|]/*FIND ALL REFS*/(stuff: { quantity: number }) {}|> +// +// declare const stuff: { quantity: number }; +// /** @see {[|doStuffWithStuff|]} */ +// if (stuff.quantity) {} + + // === Definitions === + // === /tests/cases/fourslash/findReferencesSeeTagInTs.ts === + // <|function [|doStuffWithStuff|]/*FIND ALL REFS*/(stuff: { quantity: number }) {}|> + // + // declare const stuff: { quantity: number }; + // /** @see {doStuffWithStuff} */ + // if (stuff.quantity) {} + + // === Details === + [ + { + "containerKind": "", + "containerName": "", + "kind": "function", + "name": "function doStuffWithStuff(stuff: {\n quantity: number;\n}): void", + "displayParts": [ + { + "text": "function", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "doStuffWithStuff", + "kind": "functionName" + }, + { + "text": "(", + "kind": "punctuation" + }, + { + "text": "stuff", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "{", + "kind": "punctuation" + }, + { + "text": "\n", + "kind": "lineBreak" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "quantity", + "kind": "propertyName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + }, + { + "text": ";", + "kind": "punctuation" + }, + { + "text": "\n", + "kind": "lineBreak" + }, + { + "text": "}", + "kind": "punctuation" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "void", + "kind": "keyword" + } + ] + } + ] \ No newline at end of file diff --git a/tests/cases/fourslash/findReferencesBindingPatternInJsdocNoCrash1.ts b/tests/cases/fourslash/findReferencesBindingPatternInJsdocNoCrash1.ts new file mode 100644 index 00000000000..0da1724d3b5 --- /dev/null +++ b/tests/cases/fourslash/findReferencesBindingPatternInJsdocNoCrash1.ts @@ -0,0 +1,33 @@ +/// + +// @moduleResolution: node + +// @Filename: node_modules/use-query/package.json +////{ +//// "name": "use-query", +//// "types": "index.d.ts" +////} +// @Filename: node_modules/use-query/index.d.ts +////declare function useQuery(): { +//// data: string[]; +////}; + +// @Filename: node_modules/other/package.json +////{ +//// "name": "other", +//// "types": "index.d.ts" +////} +// @Filename: node_modules/other/index.d.ts +////interface BottomSheetModalProps { +//// /** +//// * A scrollable node or normal view. +//// * @type {({ data: any }?) => any} +//// */ +//// children: ({ data: any }?) => any; +////} + +// @Filename: src/index.ts +////import { useQuery } from "use-query"; +////const { /*1*/data } = useQuery(); + +verify.baselineFindAllReferences('1'); diff --git a/tests/cases/fourslash/findReferencesBindingPatternInJsdocNoCrash2.ts b/tests/cases/fourslash/findReferencesBindingPatternInJsdocNoCrash2.ts new file mode 100644 index 00000000000..f1f05aee205 --- /dev/null +++ b/tests/cases/fourslash/findReferencesBindingPatternInJsdocNoCrash2.ts @@ -0,0 +1,33 @@ +/// + +// @moduleResolution: node + +// @Filename: node_modules/use-query/package.json +////{ +//// "name": "use-query", +//// "types": "index.d.ts" +////} +// @Filename: node_modules/use-query/index.d.ts +////declare function useQuery(): { +//// data: string[]; +////}; + +// @Filename: node_modules/use-query/package.json +////{ +//// "name": "other", +//// "types": "index.d.ts" +////} +// @Filename: node_modules/other/index.d.ts +////interface BottomSheetModalProps { +//// /** +//// * A scrollable node or normal view. +//// * @type null | (({ data: any }?) => any) +//// */ +//// children: null | (({ data: any }?) => any); +////} + +// @Filename: src/index.ts +////import { useQuery } from "use-query"; +////const { /*1*/data } = useQuery(); + +verify.baselineFindAllReferences('1'); diff --git a/tests/cases/fourslash/findReferencesSeeTagInTs.ts b/tests/cases/fourslash/findReferencesSeeTagInTs.ts new file mode 100644 index 00000000000..4629e07eef4 --- /dev/null +++ b/tests/cases/fourslash/findReferencesSeeTagInTs.ts @@ -0,0 +1,9 @@ +/// + +//// function doStuffWithStuff/*1*/(stuff: { quantity: number }) {} +//// +//// declare const stuff: { quantity: number }; +//// /** @see {doStuffWithStuff} */ +//// if (stuff.quantity) {} + +verify.baselineFindAllReferences("1"); diff --git a/tests/cases/fourslash/quickInfoBindingPatternInJsdocNoCrash1.ts b/tests/cases/fourslash/quickInfoBindingPatternInJsdocNoCrash1.ts new file mode 100644 index 00000000000..fffc324e668 --- /dev/null +++ b/tests/cases/fourslash/quickInfoBindingPatternInJsdocNoCrash1.ts @@ -0,0 +1,10 @@ +/// + +/////** @type {({ /*1*/data: any }?) => { data: string[] }} */ +////function useQuery({ data }): { data: string[] } { +//// return { +//// data, +//// }; +////} + +verify.quickInfoAt("1", ""); diff --git a/tests/cases/fourslash/quickInfoInJsdocInTsFile1.ts b/tests/cases/fourslash/quickInfoInJsdocInTsFile1.ts new file mode 100644 index 00000000000..6de25ac0c28 --- /dev/null +++ b/tests/cases/fourslash/quickInfoInJsdocInTsFile1.ts @@ -0,0 +1,62 @@ +/// + +//// /** @type {() => { /*1*/data: string[] }} */ +//// function test(): { data: string[] } { +//// return { +//// data: [], +//// }; +//// } +//// +//// /** @returns {{ /*2*/data: string[] }} */ +//// function test2(): { data: string[] } { +//// return { +//// data: [], +//// }; +//// } +//// +//// /** @type {{ /*3*/bar: string; }} */ +//// const test3 = { bar: '' }; +//// +//// type SomeObj = { bar: string; }; +//// /** @type {SomeObj/*4*/} */ +//// const test4 = { bar: '' } +//// +//// /** +//// * @param/*5*/ stuff/*6*/ Stuff to do stuff with +//// */ +//// function doStuffWithStuff(stuff: { quantity: number }) {} +//// +//// declare const stuff: { quantity: number }; +//// /** @see {doStuffWithStuff/*7*/} */ +//// if (stuff.quantity) {} +//// +//// /** @type {(a/*8*/: string) => void} */ +//// function test2(a: string) {} + +verify.quickInfoAt("1", ""); +verify.quickInfoAt("2", ""); +verify.quickInfoAt("3", ""); +verify.quickInfoAt("4", `type SomeObj = { + bar: string; +}`); +verify.quickInfoAt( + "5", + `(parameter) stuff: { + quantity: number; +}`, + "Stuff to do stuff with" +); +verify.quickInfoAt( + "6", + `(parameter) stuff: { + quantity: number; +}`, + "Stuff to do stuff with" +); +verify.quickInfoAt( + "7", + `function doStuffWithStuff(stuff: { + quantity: number; +}): void`, +); +verify.quickInfoAt("8", "");