From bf9dd25431e1e13dc054db03171e0f2b9b2a1019 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 31 Mar 2023 21:54:58 -0400 Subject: [PATCH 01/15] Filtered out types from import suggestions in JS files --- src/services/completions.ts | 16 ++ .../reference/jsFileImportNoTypes.baseline | 208 ++++++++++++++++++ tests/cases/fourslash/jsFileImportNoTypes.ts | 23 ++ 3 files changed, 247 insertions(+) create mode 100644 tests/baselines/reference/jsFileImportNoTypes.baseline create mode 100644 tests/cases/fourslash/jsFileImportNoTypes.ts diff --git a/src/services/completions.ts b/src/services/completions.ts index e8ff98ea7a0db..b233324661705 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1282,6 +1282,10 @@ function createCompletionEntry( isRightOfOpenTag: boolean | undefined, includeSymbol: boolean ): CompletionEntry | undefined { + if (isInJSFile(sourceFile) && !symbolHasValueDeclaration(symbol)) { + return undefined; + } + let insertText: string | undefined; let replacementSpan = getReplacementSpanForContextToken(replacementToken); let data: CompletionEntryData | undefined; @@ -1440,6 +1444,18 @@ function createCompletionEntry( }; } +function symbolHasValueDeclaration(symbol: Symbol): boolean { + return !!symbol?.declarations?.some(declaration => { + switch (declaration.kind) { + case SyntaxKind.InterfaceDeclaration: + case SyntaxKind.TypeAliasDeclaration: + return false; + default: + return true; + } + }); +} + function isClassLikeMemberCompletion(symbol: Symbol, location: Node, sourceFile: SourceFile): boolean { // TODO: support JS files. if (isInJSFile(location)) { diff --git a/tests/baselines/reference/jsFileImportNoTypes.baseline b/tests/baselines/reference/jsFileImportNoTypes.baseline new file mode 100644 index 0000000000000..5e012e4a4e296 --- /dev/null +++ b/tests/baselines/reference/jsFileImportNoTypes.baseline @@ -0,0 +1,208 @@ +=== /a.js === +// import { } from './declarations.ts' +// ^ +// | ---------------------------------------------------------------------- +// | class TestClass +// | class TestClassInterfaceMerged +// | interface TestClassInterfaceMerged +// | enum TestEnum +// | function testFunction(): void +// | namespace TestNamespace +// | const testValue: {} +// | ---------------------------------------------------------------------- + +[ + { + "marker": { + "fileName": "/a.js", + "position": 9, + "name": "" + }, + "item": { + "flags": 0, + "isGlobalCompletion": false, + "isMemberCompletion": true, + "isNewIdentifierLocation": false, + "entries": [ + { + "name": "TestClass", + "kind": "class", + "kindModifiers": "export", + "sortText": "11", + "displayParts": [ + { + "text": "class", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "TestClass", + "kind": "className" + } + ], + "documentation": [] + }, + { + "name": "TestClassInterfaceMerged", + "kind": "class", + "kindModifiers": "export", + "sortText": "11", + "displayParts": [ + { + "text": "class", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "TestClassInterfaceMerged", + "kind": "className" + }, + { + "text": "\n", + "kind": "lineBreak" + }, + { + "text": "interface", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "TestClassInterfaceMerged", + "kind": "className" + } + ], + "documentation": [] + }, + { + "name": "TestEnum", + "kind": "enum", + "kindModifiers": "export", + "sortText": "11", + "displayParts": [ + { + "text": "enum", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "TestEnum", + "kind": "enumName" + } + ], + "documentation": [] + }, + { + "name": "testFunction", + "kind": "function", + "kindModifiers": "export", + "sortText": "11", + "displayParts": [ + { + "text": "function", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "testFunction", + "kind": "functionName" + }, + { + "text": "(", + "kind": "punctuation" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "void", + "kind": "keyword" + } + ], + "documentation": [] + }, + { + "name": "TestNamespace", + "kind": "module", + "kindModifiers": "export", + "sortText": "11", + "displayParts": [ + { + "text": "namespace", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "TestNamespace", + "kind": "moduleName" + } + ], + "documentation": [] + }, + { + "name": "testValue", + "kind": "const", + "kindModifiers": "export", + "sortText": "11", + "displayParts": [ + { + "text": "const", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "testValue", + "kind": "localName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "{", + "kind": "punctuation" + }, + { + "text": "}", + "kind": "punctuation" + } + ], + "documentation": [] + } + ] + } + } +] \ No newline at end of file diff --git a/tests/cases/fourslash/jsFileImportNoTypes.ts b/tests/cases/fourslash/jsFileImportNoTypes.ts new file mode 100644 index 0000000000000..e5fdeb4c2dd60 --- /dev/null +++ b/tests/cases/fourslash/jsFileImportNoTypes.ts @@ -0,0 +1,23 @@ +/// + +// @allowJs: true + +// @filename: /declarations.ts +//// export class TestClass {} +//// export const testValue = {}; +//// export enum TestEnum {} +//// export function testFunction() {} +//// export interface testInterface {} +//// export namespace TestNamespace {} +//// export type testType = {}; +//// +//// export interface TestInterfaceMerged {} +//// export interface TestInterfaceMerged {} +//// +//// export interface TestClassInterfaceMerged {} +//// export class TestClassInterfaceMerged {} + +// @filename: /a.js +////import { /**/ } from './declarations.ts' + +verify.baselineCompletions(); From c7ff6b2a59f81f180d1305f3c963d416e3abaed4 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 31 Mar 2023 22:18:38 -0400 Subject: [PATCH 02/15] Account for JSDoc type locations in JS files --- src/services/completions.ts | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index b233324661705..8781744bb7de4 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1282,10 +1282,6 @@ function createCompletionEntry( isRightOfOpenTag: boolean | undefined, includeSymbol: boolean ): CompletionEntry | undefined { - if (isInJSFile(sourceFile) && !symbolHasValueDeclaration(symbol)) { - return undefined; - } - let insertText: string | undefined; let replacementSpan = getReplacementSpanForContextToken(replacementToken); let data: CompletionEntryData | undefined; @@ -1444,18 +1440,6 @@ function createCompletionEntry( }; } -function symbolHasValueDeclaration(symbol: Symbol): boolean { - return !!symbol?.declarations?.some(declaration => { - switch (declaration.kind) { - case SyntaxKind.InterfaceDeclaration: - case SyntaxKind.TypeAliasDeclaration: - return false; - default: - return true; - } - }); -} - function isClassLikeMemberCompletion(symbol: Symbol, location: Node, sourceFile: SourceFile): boolean { // TODO: support JS files. if (isInJSFile(location)) { @@ -2100,6 +2084,10 @@ export function getCompletionEntriesFromSymbols( continue; } + if (!isTypeOnlyLocation && isInJSFile(sourceFile) && !symbolHasValueDeclaration(symbol)) { + continue; + } + const { name, needsConvertPropertyAccess } = info; const originalSortText = symbolToSortTextMap?.[getSymbolId(symbol)] ?? SortText.LocationPriority; const sortText = (isDeprecated(symbol, typeChecker) ? SortText.Deprecated(originalSortText) : originalSortText); @@ -2196,6 +2184,18 @@ export function getCompletionEntriesFromSymbols( } } +function symbolHasValueDeclaration(symbol: Symbol): boolean { + return !symbol?.declarations?.length || symbol.declarations.some(declaration => { + switch (declaration.kind) { + case SyntaxKind.InterfaceDeclaration: + case SyntaxKind.TypeAliasDeclaration: + return false; + default: + return true; + } + }); +} + function getLabelCompletionAtPosition(node: BreakOrContinueStatement): CompletionInfo | undefined { const entries = getLabelStatementCompletions(node); if (entries.length) { From 3773af1c2a9bcf29aded7d657e76c2aa4e6f4481 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 1 Apr 2023 10:43:04 -0400 Subject: [PATCH 03/15] Added a bit more to the test case --- .../reference/jsFileImportNoTypes.baseline | 56 +++++++++++++++++++ tests/cases/fourslash/jsFileImportNoTypes.ts | 5 ++ 2 files changed, 61 insertions(+) diff --git a/tests/baselines/reference/jsFileImportNoTypes.baseline b/tests/baselines/reference/jsFileImportNoTypes.baseline index 5e012e4a4e296..1f8bf26a518a0 100644 --- a/tests/baselines/reference/jsFileImportNoTypes.baseline +++ b/tests/baselines/reference/jsFileImportNoTypes.baseline @@ -2,6 +2,8 @@ // import { } from './declarations.ts' // ^ // | ---------------------------------------------------------------------- +// | class DeclaredClass +// | const declaredVariable: number // | class TestClass // | class TestClassInterfaceMerged // | interface TestClassInterfaceMerged @@ -24,6 +26,60 @@ "isMemberCompletion": true, "isNewIdentifierLocation": false, "entries": [ + { + "name": "DeclaredClass", + "kind": "class", + "kindModifiers": "export,declare", + "sortText": "11", + "displayParts": [ + { + "text": "class", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "DeclaredClass", + "kind": "className" + } + ], + "documentation": [] + }, + { + "name": "declaredVariable", + "kind": "const", + "kindModifiers": "export,declare", + "sortText": "11", + "displayParts": [ + { + "text": "const", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "declaredVariable", + "kind": "localName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "documentation": [] + }, { "name": "TestClass", "kind": "class", diff --git a/tests/cases/fourslash/jsFileImportNoTypes.ts b/tests/cases/fourslash/jsFileImportNoTypes.ts index e5fdeb4c2dd60..ca0b64199f01f 100644 --- a/tests/cases/fourslash/jsFileImportNoTypes.ts +++ b/tests/cases/fourslash/jsFileImportNoTypes.ts @@ -16,6 +16,11 @@ //// //// export interface TestClassInterfaceMerged {} //// export class TestClassInterfaceMerged {} +//// +//// export declare const declaredVariable: number; +//// export declare class DeclaredClass {} +//// export declare interface DeclaredInterface {} +//// export declare type DeclaredType = {}; // @filename: /a.js ////import { /**/ } from './declarations.ts' From 119a10b83c7e86fa252b34766703110e6b470b92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Tue, 4 Apr 2023 19:46:48 -0400 Subject: [PATCH 04/15] Update src/services/completions.ts Co-authored-by: Ryan Cavanaugh --- src/services/completions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 8781744bb7de4..8a9a671347707 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2084,7 +2084,7 @@ export function getCompletionEntriesFromSymbols( continue; } - if (!isTypeOnlyLocation && isInJSFile(sourceFile) && !symbolHasValueDeclaration(symbol)) { + if (!isTypeOnlyLocation && isInJSFile(sourceFile) && symbol.flags & SymbolFlags.Value) { continue; } From 1ab7b75e7f11881a61c6b4825a44312bf173852c Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 4 Apr 2023 19:51:41 -0400 Subject: [PATCH 05/15] Post suggestion: remove unused function --- src/services/completions.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 8a9a671347707..8684a0a5142b4 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2184,18 +2184,6 @@ export function getCompletionEntriesFromSymbols( } } -function symbolHasValueDeclaration(symbol: Symbol): boolean { - return !symbol?.declarations?.length || symbol.declarations.some(declaration => { - switch (declaration.kind) { - case SyntaxKind.InterfaceDeclaration: - case SyntaxKind.TypeAliasDeclaration: - return false; - default: - return true; - } - }); -} - function getLabelCompletionAtPosition(node: BreakOrContinueStatement): CompletionInfo | undefined { const entries = getLabelStatementCompletions(node); if (entries.length) { From ae5fc3d410cb8121bd7523cd7a31fee6c16a44de Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 6 Apr 2023 00:18:15 -0400 Subject: [PATCH 06/15] Correcting to !(symbol.flags & SymbolFlags.Value) --- src/services/completions.ts | 2 +- .../reference/jsFileImportNoTypes.baseline | 22 ----------------- ...ed-from-two-different-drives-of-windows.js | 24 +++++++++---------- 3 files changed, 13 insertions(+), 35 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 8684a0a5142b4..d6a48252a6fa9 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2084,7 +2084,7 @@ export function getCompletionEntriesFromSymbols( continue; } - if (!isTypeOnlyLocation && isInJSFile(sourceFile) && symbol.flags & SymbolFlags.Value) { + if (!isTypeOnlyLocation && isInJSFile(sourceFile) && !(symbol.flags & SymbolFlags.Value)) { continue; } diff --git a/tests/baselines/reference/jsFileImportNoTypes.baseline b/tests/baselines/reference/jsFileImportNoTypes.baseline index 1f8bf26a518a0..10aa4ebd938ca 100644 --- a/tests/baselines/reference/jsFileImportNoTypes.baseline +++ b/tests/baselines/reference/jsFileImportNoTypes.baseline @@ -9,7 +9,6 @@ // | interface TestClassInterfaceMerged // | enum TestEnum // | function testFunction(): void -// | namespace TestNamespace // | const testValue: {} // | ---------------------------------------------------------------------- @@ -200,27 +199,6 @@ ], "documentation": [] }, - { - "name": "TestNamespace", - "kind": "module", - "kindModifiers": "export", - "sortText": "11", - "displayParts": [ - { - "text": "namespace", - "kind": "keyword" - }, - { - "text": " ", - "kind": "space" - }, - { - "text": "TestNamespace", - "kind": "moduleName" - } - ], - "documentation": [] - }, { "name": "testValue", "kind": "const", diff --git a/tests/baselines/reference/tsserver/completions/works-when-files-are-included-from-two-different-drives-of-windows.js b/tests/baselines/reference/tsserver/completions/works-when-files-are-included-from-two-different-drives-of-windows.js index fe0b41a3e8bad..c1e13f9964e06 100644 --- a/tests/baselines/reference/tsserver/completions/works-when-files-are-included-from-two-different-drives-of-windows.js +++ b/tests/baselines/reference/tsserver/completions/works-when-files-are-included-from-two-different-drives-of-windows.js @@ -268,18 +268,6 @@ Info 37 [00:02:13.000] response: "isMemberCompletion": false, "isNewIdentifierLocation": false, "entries": [ - { - "name": "React", - "kind": "alias", - "kindModifiers": "", - "sortText": "11" - }, - { - "name": "Router", - "kind": "alias", - "kindModifiers": "", - "sortText": "11" - }, { "name": "as", "kind": "keyword", @@ -555,6 +543,18 @@ Info 37 [00:02:13.000] response: "kind": "warning", "kindModifiers": "", "sortText": "18" + }, + { + "name": "React", + "kind": "warning", + "kindModifiers": "", + "sortText": "18" + }, + { + "name": "Router", + "kind": "warning", + "kindModifiers": "", + "sortText": "18" } ] }, From b4b7b98d12155950b7da931a646193e672d79081 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 6 Apr 2023 00:49:25 -0400 Subject: [PATCH 07/15] Went back to symbol check function --- src/services/completions.ts | 14 ++++++++++- .../reference/jsFileImportNoTypes.baseline | 22 +++++++++++++++++ ...ed-from-two-different-drives-of-windows.js | 24 +++++++++---------- 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index d6a48252a6fa9..e15399b0d274a 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2084,7 +2084,7 @@ export function getCompletionEntriesFromSymbols( continue; } - if (!isTypeOnlyLocation && isInJSFile(sourceFile) && !(symbol.flags & SymbolFlags.Value)) { + if (!isTypeOnlyLocation && isInJSFile(sourceFile) && !symbolHasNoTypeOnlyDeclaration(symbol)) { continue; } @@ -2184,6 +2184,18 @@ export function getCompletionEntriesFromSymbols( } } +function symbolHasNoTypeOnlyDeclaration(symbol: Symbol): boolean { + return !symbol?.declarations?.length || symbol.declarations.some(declaration => { + switch (declaration.kind) { + case SyntaxKind.InterfaceDeclaration: + case SyntaxKind.TypeAliasDeclaration: + return false; + default: + return true; + } + }); +} + function getLabelCompletionAtPosition(node: BreakOrContinueStatement): CompletionInfo | undefined { const entries = getLabelStatementCompletions(node); if (entries.length) { diff --git a/tests/baselines/reference/jsFileImportNoTypes.baseline b/tests/baselines/reference/jsFileImportNoTypes.baseline index 10aa4ebd938ca..1f8bf26a518a0 100644 --- a/tests/baselines/reference/jsFileImportNoTypes.baseline +++ b/tests/baselines/reference/jsFileImportNoTypes.baseline @@ -9,6 +9,7 @@ // | interface TestClassInterfaceMerged // | enum TestEnum // | function testFunction(): void +// | namespace TestNamespace // | const testValue: {} // | ---------------------------------------------------------------------- @@ -199,6 +200,27 @@ ], "documentation": [] }, + { + "name": "TestNamespace", + "kind": "module", + "kindModifiers": "export", + "sortText": "11", + "displayParts": [ + { + "text": "namespace", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "TestNamespace", + "kind": "moduleName" + } + ], + "documentation": [] + }, { "name": "testValue", "kind": "const", diff --git a/tests/baselines/reference/tsserver/completions/works-when-files-are-included-from-two-different-drives-of-windows.js b/tests/baselines/reference/tsserver/completions/works-when-files-are-included-from-two-different-drives-of-windows.js index c1e13f9964e06..fe0b41a3e8bad 100644 --- a/tests/baselines/reference/tsserver/completions/works-when-files-are-included-from-two-different-drives-of-windows.js +++ b/tests/baselines/reference/tsserver/completions/works-when-files-are-included-from-two-different-drives-of-windows.js @@ -268,6 +268,18 @@ Info 37 [00:02:13.000] response: "isMemberCompletion": false, "isNewIdentifierLocation": false, "entries": [ + { + "name": "React", + "kind": "alias", + "kindModifiers": "", + "sortText": "11" + }, + { + "name": "Router", + "kind": "alias", + "kindModifiers": "", + "sortText": "11" + }, { "name": "as", "kind": "keyword", @@ -543,18 +555,6 @@ Info 37 [00:02:13.000] response: "kind": "warning", "kindModifiers": "", "sortText": "18" - }, - { - "name": "React", - "kind": "warning", - "kindModifiers": "", - "sortText": "18" - }, - { - "name": "Router", - "kind": "warning", - "kindModifiers": "", - "sortText": "18" } ] }, From be9fba8f17c48d2a5c077bdf6e6489d0d5787af0 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 7 Apr 2023 10:50:02 -0400 Subject: [PATCH 08/15] Naming and comments --- src/services/completions.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index e15399b0d274a..9b96d2e0f732f 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2084,7 +2084,8 @@ export function getCompletionEntriesFromSymbols( continue; } - if (!isTypeOnlyLocation && isInJSFile(sourceFile) && !symbolHasNoTypeOnlyDeclaration(symbol)) { + // When in a value-time location in a JS file, ignore symbols that definitely seem to be type-only + if (!isTypeOnlyLocation && isInJSFile(sourceFile) && !symbolMayHaveValueDeclaration(symbol)) { continue; } @@ -2184,7 +2185,12 @@ export function getCompletionEntriesFromSymbols( } } -function symbolHasNoTypeOnlyDeclaration(symbol: Symbol): boolean { +/** + * When filling completions for value-time locations in JS files, we'll want + * to only consider symbols that seem to have a value declaration. If a + * symbol no known declarations we cautiously include them just to be safe. + */ +function symbolMayHaveValueDeclaration(symbol: Symbol): boolean { return !symbol?.declarations?.length || symbol.declarations.some(declaration => { switch (declaration.kind) { case SyntaxKind.InterfaceDeclaration: From dd9ff9b71d31e29b46354651cc82e2ba178df67d Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 13 Apr 2023 11:37:22 -0400 Subject: [PATCH 09/15] Almost working with SymbolFlags.Value --- src/services/completions.ts | 21 ++------------- .../unittests/tsserver/completions.ts | 7 +++-- .../reference/jsFileImportNoTypes.baseline | 6 ++--- ...ed-from-two-different-drives-of-windows.js | 27 +++++++++++++++---- .../importStatementCompletions_js.ts | 4 ++- .../importStatementCompletions_js2.ts | 4 ++- tests/cases/fourslash/jsFileImportNoTypes.ts | 5 +++- 7 files changed, 42 insertions(+), 32 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 9b96d2e0f732f..6e60aad4d34e4 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2085,7 +2085,7 @@ export function getCompletionEntriesFromSymbols( } // When in a value-time location in a JS file, ignore symbols that definitely seem to be type-only - if (!isTypeOnlyLocation && isInJSFile(sourceFile) && !symbolMayHaveValueDeclaration(symbol)) { + if (!isTypeOnlyLocation && isInJSFile(sourceFile) && !(symbol.flags & SymbolFlags.Value) && !isInJSFile(symbol.declarations?.[0]?.getSourceFile())) { continue; } @@ -2137,7 +2137,7 @@ export function getCompletionEntriesFromSymbols( has: name => uniques.has(name), add: name => uniques.set(name, true), }; - + function shouldIncludeSymbol(symbol: Symbol, symbolToSortTextMap: SymbolSortTextMap): boolean { let allFlags = symbol.flags; if (!isSourceFile(location)) { @@ -2185,23 +2185,6 @@ export function getCompletionEntriesFromSymbols( } } -/** - * When filling completions for value-time locations in JS files, we'll want - * to only consider symbols that seem to have a value declaration. If a - * symbol no known declarations we cautiously include them just to be safe. - */ -function symbolMayHaveValueDeclaration(symbol: Symbol): boolean { - return !symbol?.declarations?.length || symbol.declarations.some(declaration => { - switch (declaration.kind) { - case SyntaxKind.InterfaceDeclaration: - case SyntaxKind.TypeAliasDeclaration: - return false; - default: - return true; - } - }); -} - function getLabelCompletionAtPosition(node: BreakOrContinueStatement): CompletionInfo | undefined { const entries = getLabelStatementCompletions(node); if (entries.length) { diff --git a/src/testRunner/unittests/tsserver/completions.ts b/src/testRunner/unittests/tsserver/completions.ts index 9904ea91ab663..71f810a76c28e 100644 --- a/src/testRunner/unittests/tsserver/completions.ts +++ b/src/testRunner/unittests/tsserver/completions.ts @@ -101,7 +101,8 @@ import { const localReact: File = { path: `${localAtTypes}/react/index.d.ts`, content: `import * as PropTypes from 'prop-types'; -` +export class Component {} +`, }; const localReactRouterDomPackage: File = { path: `${localNodeModules}/react-router-dom/package.json`, @@ -127,7 +128,9 @@ import { | string | ((props: any, context?: any) => any) | (new (props: any, context?: any) => any); -` + +export const PropTypes = {}; +`, }; const globalCacheLocation = `c:/typescript`; diff --git a/tests/baselines/reference/jsFileImportNoTypes.baseline b/tests/baselines/reference/jsFileImportNoTypes.baseline index 1f8bf26a518a0..112ad8bfc3dd2 100644 --- a/tests/baselines/reference/jsFileImportNoTypes.baseline +++ b/tests/baselines/reference/jsFileImportNoTypes.baseline @@ -9,7 +9,7 @@ // | interface TestClassInterfaceMerged // | enum TestEnum // | function testFunction(): void -// | namespace TestNamespace +// | namespace TestNamespaceWithValue // | const testValue: {} // | ---------------------------------------------------------------------- @@ -201,7 +201,7 @@ "documentation": [] }, { - "name": "TestNamespace", + "name": "TestNamespaceWithValue", "kind": "module", "kindModifiers": "export", "sortText": "11", @@ -215,7 +215,7 @@ "kind": "space" }, { - "text": "TestNamespace", + "text": "TestNamespaceWithValue", "kind": "moduleName" } ], diff --git a/tests/baselines/reference/tsserver/completions/works-when-files-are-included-from-two-different-drives-of-windows.js b/tests/baselines/reference/tsserver/completions/works-when-files-are-included-from-two-different-drives-of-windows.js index fe0b41a3e8bad..84793a5734f42 100644 --- a/tests/baselines/reference/tsserver/completions/works-when-files-are-included-from-two-different-drives-of-windows.js +++ b/tests/baselines/reference/tsserver/completions/works-when-files-are-included-from-two-different-drives-of-windows.js @@ -10,6 +10,7 @@ import { //// [e:/myproject/node_modules/@types/react/index.d.ts] import * as PropTypes from 'prop-types'; +export class Component {} //// [e:/myproject/node_modules/@types/prop-types/index.d.ts] @@ -17,6 +18,8 @@ export type ReactComponentLike = | string | ((props: any, context?: any) => any) | (new (props: any, context?: any) => any); + +export const PropTypes = {}; //// [c:/typescript/node_modules/@types/react-router-dom/index.d.ts] @@ -30,6 +33,7 @@ export interface BrowserRouterProps { //// [c:/typescript/node_modules/@types/react/index.d.ts] import * as PropTypes from 'prop-types'; +export class Component {} //// [e:/myproject/package.json] @@ -97,9 +101,9 @@ Info 19 [00:01:17.000] Finishing updateGraphWorker: Project: /dev/null/inferre Info 20 [00:01:18.000] Project '/dev/null/inferredProject1*' (Inferred) Info 21 [00:01:19.000] Files (6) c:/a/lib/lib.d.ts Text-1 "/// \ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array { length: number; [n: number]: T; }" - e:/myproject/node_modules/@types/prop-types/index.d.ts Text-1 "export type ReactComponentLike =\n | string\n | ((props: any, context?: any) => any)\n | (new (props: any, context?: any) => any);\n" - e:/myproject/node_modules/@types/react/index.d.ts Text-1 "import * as PropTypes from 'prop-types';\n" - c:/typescript/node_modules/@types/react/index.d.ts Text-1 "import * as PropTypes from 'prop-types';\n" + e:/myproject/node_modules/@types/prop-types/index.d.ts Text-1 "export type ReactComponentLike =\n | string\n | ((props: any, context?: any) => any)\n | (new (props: any, context?: any) => any);\n\nexport const PropTypes = {};\n" + e:/myproject/node_modules/@types/react/index.d.ts Text-1 "import * as PropTypes from 'prop-types';\nexport class Component {}\n" + c:/typescript/node_modules/@types/react/index.d.ts Text-1 "import * as PropTypes from 'prop-types';\nexport class Component {}\n" c:/typescript/node_modules/@types/react-router-dom/index.d.ts Text-1 "import * as React from 'react';\nexport interface BrowserRouterProps {\n basename?: string;\n getUserConfirmation?: ((message: string, callback: (ok: boolean) => void) => void);\n forceRefresh?: boolean;\n keyLength?: number;\n}" e:/myproject/src/app.js SVC-1-0 "import React from 'react';\nimport {\n BrowserRouter as Router,\n} from \"react-router-dom\";\n" @@ -255,8 +259,8 @@ Info 28 [00:02:04.000] getCompletionData: Get previous token: * Info 29 [00:02:05.000] getCompletionsAtPosition: isCompletionListBlocker: * Info 30 [00:02:06.000] getExportInfoMap: cache miss or empty; calculating new results Info 31 [00:02:07.000] getExportInfoMap: done in * ms -Info 32 [00:02:08.000] collectAutoImports: resolved 0 module specifiers, plus 0 ambient and 0 from cache -Info 33 [00:02:09.000] collectAutoImports: response is complete +Info 32 [00:02:08.000] collectAutoImports: resolved 0 module specifiers, plus 0 ambient and 2 from cache +Info 33 [00:02:09.000] collectAutoImports: response is incomplete Info 34 [00:02:10.000] collectAutoImports: * Info 35 [00:02:11.000] getCompletionData: Semantic work: * Info 36 [00:02:12.000] getCompletionsAtPosition: getCompletionEntriesFromSymbols: * @@ -550,6 +554,19 @@ Info 37 [00:02:13.000] response: "kindModifiers": "", "sortText": "15" }, + { + "name": "Component", + "kind": "class", + "kindModifiers": "export,declare", + "sortText": "16", + "hasAction": true, + "source": "e:/myproject/node_modules/@types/react/index", + "data": { + "exportName": "Component", + "exportMapKey": "Component|*|", + "fileName": "e:/myproject/node_modules/@types/react/index.d.ts" + } + }, { "name": "BrowserRouter", "kind": "warning", diff --git a/tests/cases/fourslash/importStatementCompletions_js.ts b/tests/cases/fourslash/importStatementCompletions_js.ts index fe4e0babad584..2473041b9b2c5 100644 --- a/tests/cases/fourslash/importStatementCompletions_js.ts +++ b/tests/cases/fourslash/importStatementCompletions_js.ts @@ -10,7 +10,9 @@ // @allowSyntheticDefaultImports: true // @Filename: /node_modules/react/index.d.ts -//// declare namespace React {} +//// declare namespace React { +//// export class Component {} +//// } //// export = React; // @Filename: /test.js diff --git a/tests/cases/fourslash/importStatementCompletions_js2.ts b/tests/cases/fourslash/importStatementCompletions_js2.ts index 431543ac58a3b..9bc00dcf4d7d9 100644 --- a/tests/cases/fourslash/importStatementCompletions_js2.ts +++ b/tests/cases/fourslash/importStatementCompletions_js2.ts @@ -12,7 +12,9 @@ // @noEmit: true // @Filename: /node_modules/react/index.d.ts -//// declare namespace React {} +//// declare namespace React { +//// export class Component {} +//// } //// export = React; // @Filename: /test.js diff --git a/tests/cases/fourslash/jsFileImportNoTypes.ts b/tests/cases/fourslash/jsFileImportNoTypes.ts index ca0b64199f01f..1cfc36fd23d55 100644 --- a/tests/cases/fourslash/jsFileImportNoTypes.ts +++ b/tests/cases/fourslash/jsFileImportNoTypes.ts @@ -8,7 +8,10 @@ //// export enum TestEnum {} //// export function testFunction() {} //// export interface testInterface {} -//// export namespace TestNamespace {} +//// export namespace TestNamespaceTypeOnly {} +//// export namespace TestNamespaceWithValue { +//// export const testValueInner = true; +//// } //// export type testType = {}; //// //// export interface TestInterfaceMerged {} From 336aaecd388c626de0358315e7e04d86429b8a2b Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 13 Apr 2023 11:40:45 -0400 Subject: [PATCH 10/15] looking forward to dprint --- src/services/completions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 6e60aad4d34e4..04cb9d1f5416a 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2137,7 +2137,7 @@ export function getCompletionEntriesFromSymbols( has: name => uniques.has(name), add: name => uniques.set(name, true), }; - + function shouldIncludeSymbol(symbol: Symbol, symbolToSortTextMap: SymbolSortTextMap): boolean { let allFlags = symbol.flags; if (!isSourceFile(location)) { From 012679f8be9d0d408542805ada5e86a9973d942c Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 13 Apr 2023 11:44:09 -0400 Subject: [PATCH 11/15] Twitch review-as-a-service --- src/services/completions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index cf9473804b0cb..c896db562c680 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2404,7 +2404,7 @@ export function getCompletionEntriesFromSymbols( continue; } - // When in a value-time location in a JS file, ignore symbols that definitely seem to be type-only + // When in a value location in a JS file, ignore symbols that definitely seem to be type-only if (!isTypeOnlyLocation && isInJSFile(sourceFile) && !(symbol.flags & SymbolFlags.Value) && !isInJSFile(symbol.declarations?.[0]?.getSourceFile())) { continue; } From e344ec88e7fbbf911c223e2c91f947b7e760b75e Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 17 Apr 2023 15:53:04 -0400 Subject: [PATCH 12/15] Added javascriptModulesTypeImport --- .../fourslash/javascriptModulesTypeImport.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 tests/cases/fourslash/javascriptModulesTypeImport.ts diff --git a/tests/cases/fourslash/javascriptModulesTypeImport.ts b/tests/cases/fourslash/javascriptModulesTypeImport.ts new file mode 100644 index 0000000000000..674971ab0ca21 --- /dev/null +++ b/tests/cases/fourslash/javascriptModulesTypeImport.ts @@ -0,0 +1,19 @@ +/// +// @allowJs: true + +// @Filename: types.js +//// /** +//// * @typedef {Object} Pet +//// * @prop {string} name +//// */ +//// module.exports = { a: 1 }; + +// @Filename: app.js +//// /** +//// * @param { import("./types")./**/ } p +//// */ +//// function walk(p) { +//// console.log(`Walking ${p.name}...`); +//// } + +verify.completions({ marker: "", includes: "Pet" }); From 5644a4f603e8f6f66d739abf2c488e50c4fea284 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 18 Apr 2023 11:36:12 -0400 Subject: [PATCH 13/15] Account for JS file type-only exports --- src/services/completions.ts | 6 +++++- .../javascriptModulesTypeImportAsValue.ts | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/javascriptModulesTypeImportAsValue.ts diff --git a/src/services/completions.ts b/src/services/completions.ts index c896db562c680..e343dee36b766 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2405,7 +2405,7 @@ export function getCompletionEntriesFromSymbols( } // When in a value location in a JS file, ignore symbols that definitely seem to be type-only - if (!isTypeOnlyLocation && isInJSFile(sourceFile) && !(symbol.flags & SymbolFlags.Value) && !isInJSFile(symbol.declarations?.[0]?.getSourceFile())) { + if (!isTypeOnlyLocation && isInJSFile(sourceFile) && symbolAppearsToBeTypeOnly(symbol)) { continue; } @@ -2503,6 +2503,10 @@ export function getCompletionEntriesFromSymbols( // expressions are value space (which includes the value namespaces) return !!(allFlags & SymbolFlags.Value); } + + function symbolAppearsToBeTypeOnly(symbol: Symbol): boolean { + return !(symbol.flags & SymbolFlags.Value) && (!isInJSFile(symbol.declarations?.[0]?.getSourceFile()) || !!(symbol.flags & SymbolFlags.Type)); + } } function getLabelCompletionAtPosition(node: BreakOrContinueStatement): CompletionInfo | undefined { diff --git a/tests/cases/fourslash/javascriptModulesTypeImportAsValue.ts b/tests/cases/fourslash/javascriptModulesTypeImportAsValue.ts new file mode 100644 index 0000000000000..f35230dbc17e0 --- /dev/null +++ b/tests/cases/fourslash/javascriptModulesTypeImportAsValue.ts @@ -0,0 +1,14 @@ +/// +// @allowJs: true + +// @Filename: types.js +//// /** +//// * @typedef {Object} Pet +//// * @prop {string} name +//// */ +//// module.exports = { a: 1 }; + +// @Filename: app.js +//// import { /**/ } from "./types" + +verify.completions({ marker: "", excludes: "Pet" }); From 9eecdd5290ffd00a128e71846f4b8eeb8abb9b30 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 21 Oct 2023 09:36:16 -0400 Subject: [PATCH 14/15] Add a sub-namespace type, and undid some formatting changes --- .../unittests/tsserver/completions.ts | 28 ++----------------- tests/cases/fourslash/jsFileImportNoTypes.ts | 7 +++-- 2 files changed, 8 insertions(+), 27 deletions(-) diff --git a/src/testRunner/unittests/tsserver/completions.ts b/src/testRunner/unittests/tsserver/completions.ts index cf3a624c3c8a0..78858bf08b3c1 100644 --- a/src/testRunner/unittests/tsserver/completions.ts +++ b/src/testRunner/unittests/tsserver/completions.ts @@ -29,9 +29,7 @@ describe("unittests:: tsserver:: completions", () => { }; const host = createServerHost([aTs, bTs, tsconfig]); - const session = createSession(host, { - logger: createLoggerWithInMemoryLogs(host), - }); + const session = createSession(host, { logger: createLoggerWithInMemoryLogs(host) }); openFilesForSession([aTs, bTs], session); const requestLocation: ts.server.protocol.FileLocationRequestArgs = { @@ -53,17 +51,7 @@ describe("unittests:: tsserver:: completions", () => { command: ts.server.protocol.CommandTypes.CompletionDetails, arguments: { ...requestLocation, - entryNames: [ - { - name: "foo", - source: "/a", - data: { - exportName: "foo", - fileName: "/a.ts", - exportMapKey, - }, - }, - ], + entryNames: [{ name: "foo", source: "/a", data: { exportName: "foo", fileName: "/a.ts", exportMapKey } }], }, }); @@ -75,17 +63,7 @@ describe("unittests:: tsserver:: completions", () => { command: ts.server.protocol.CommandTypes.CompletionDetailsFull, arguments: { ...requestLocation, - entryNames: [ - { - name: "foo", - source: "/a", - data: { - exportName: "foo", - fileName: "/a.ts", - exportMapKey, - }, - }, - ], + entryNames: [{ name: "foo", source: "/a", data: { exportName: "foo", fileName: "/a.ts", exportMapKey } }], }, }); baselineTsserverLogs("completions", "works", session); diff --git a/tests/cases/fourslash/jsFileImportNoTypes.ts b/tests/cases/fourslash/jsFileImportNoTypes.ts index 1cfc36fd23d55..26e946f8afa48 100644 --- a/tests/cases/fourslash/jsFileImportNoTypes.ts +++ b/tests/cases/fourslash/jsFileImportNoTypes.ts @@ -8,9 +8,12 @@ //// export enum TestEnum {} //// export function testFunction() {} //// export interface testInterface {} -//// export namespace TestNamespaceTypeOnly {} +//// export namespace TestNamespaceEmpty {} +//// export namespace TestNamespaceWithType { +//// export type testTypeInner = boolean; +//// } //// export namespace TestNamespaceWithValue { -//// export const testValueInner = true; +//// export const testValueInner = true; //// } //// export type testType = {}; //// From 8de257da80e9124af45ab2d1e134600e4e20c9ba Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 6 Nov 2023 12:02:35 +0000 Subject: [PATCH 15/15] Remove unnecessary getSourceFile --- src/services/completions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index c062b720a8367..f85179da4e9c8 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2672,7 +2672,7 @@ export function getCompletionEntriesFromSymbols( } function symbolAppearsToBeTypeOnly(symbol: Symbol): boolean { - return !(symbol.flags & SymbolFlags.Value) && (!isInJSFile(symbol.declarations?.[0]?.getSourceFile()) || !!(symbol.flags & SymbolFlags.Type)); + return !(symbol.flags & SymbolFlags.Value) && (!isInJSFile(symbol.declarations?.[0]) || !!(symbol.flags & SymbolFlags.Type)); } }