Skip to content
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

Filtered out types from import suggestions in JS files #53619

Merged
merged 19 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2550,6 +2550,11 @@ export function getCompletionEntriesFromSymbols(
continue;
}

// When in a value location in a JS file, ignore symbols that definitely seem to be type-only
if (!isTypeOnlyLocation && isInJSFile(sourceFile) && symbolAppearsToBeTypeOnly(symbol)) {
continue;
}

const { name, needsConvertPropertyAccess } = info;
const originalSortText = symbolToSortTextMap?.[getSymbolId(symbol)] ?? SortText.LocationPriority;
const sortText = isDeprecated(symbol, typeChecker) ? SortText.Deprecated(originalSortText) : originalSortText;
Expand Down Expand Up @@ -2665,6 +2670,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]) || !!(symbol.flags & SymbolFlags.Type));
}
}

function getLabelCompletionAtPosition(node: BreakOrContinueStatement): CompletionInfo | undefined {
Expand Down
9 changes: 8 additions & 1 deletion src/testRunner/unittests/tsserver/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ import {
const localReact: File = {
path: `${localAtTypes}/react/index.d.ts`,
content: `import * as PropTypes from 'prop-types';
export class Component {}
`,
sandersn marked this conversation as resolved.
Show resolved Hide resolved
};
const localReactRouterDomPackage: File = {
Expand All @@ -131,6 +132,8 @@ import {
| string
| ((props: any, context?: any) => any)
| (new (props: any, context?: any) => any);

export const PropTypes = {};
`,
};

Expand Down Expand Up @@ -198,6 +201,10 @@ export interface BrowserRouterProps {
includeInsertTextCompletions: true,
},
});
baselineTsserverLogs("completions", "works when files are included from two different drives of windows", session);
baselineTsserverLogs(
"completions",
"works when files are included from two different drives of windows",
session,
);
});
});
265 changes: 265 additions & 0 deletions tests/baselines/reference/jsFileImportNoTypes.baseline
Original file line number Diff line number Diff line change
@@ -0,0 +1,265 @@
// === Completions ===
=== /a.js ===
// import { } from './declarations.ts'
// ^
// | ----------------------------------------------------------------------
// | class DeclaredClass
// | const declaredVariable: number
// | class TestClass
// | class TestClassInterfaceMerged
// | interface TestClassInterfaceMerged
// | enum TestEnum
// | function testFunction(): void
// | namespace TestNamespaceWithValue
// | const testValue: {}
// | ----------------------------------------------------------------------

[
{
"marker": {
"fileName": "/a.js",
"position": 9,
"name": ""
},
"item": {
"flags": 0,
"isGlobalCompletion": false,
"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",
"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": "TestNamespaceWithValue",
"kind": "module",
"kindModifiers": "export",
"sortText": "11",
"displayParts": [
{
"text": "namespace",
"kind": "keyword"
},
{
"text": " ",
"kind": "space"
},
{
"text": "TestNamespaceWithValue",
"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": []
}
]
}
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@ 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]
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]
Expand All @@ -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]
Expand Down Expand Up @@ -125,9 +129,9 @@ Info seq [hh:mm:ss:mss] Finishing updateGraphWorker: Project: /dev/null/inferre
Info seq [hh:mm:ss:mss] Project '/dev/null/inferredProject1*' (Inferred)
Info seq [hh:mm:ss:mss] Files (6)
c:/a/lib/lib.d.ts Text-1 "/// <reference no-default-lib=\"true\"/>\ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array<T> { 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"

Expand Down Expand Up @@ -353,8 +357,8 @@ Info seq [hh:mm:ss:mss] getCompletionData: Get previous token: *
Info seq [hh:mm:ss:mss] getCompletionsAtPosition: isCompletionListBlocker: *
Info seq [hh:mm:ss:mss] getExportInfoMap: cache miss or empty; calculating new results
Info seq [hh:mm:ss:mss] getExportInfoMap: done in * ms
Info seq [hh:mm:ss:mss] collectAutoImports: resolved 0 module specifiers, plus 0 ambient and 0 from cache
Info seq [hh:mm:ss:mss] collectAutoImports: response is complete
Info seq [hh:mm:ss:mss] collectAutoImports: resolved 0 module specifiers, plus 0 ambient and 2 from cache
Info seq [hh:mm:ss:mss] collectAutoImports: response is incomplete
Info seq [hh:mm:ss:mss] collectAutoImports: *
Info seq [hh:mm:ss:mss] getCompletionData: Semantic work: *
Info seq [hh:mm:ss:mss] getCompletionsAtPosition: getCompletionEntriesFromSymbols: *
Expand Down Expand Up @@ -648,6 +652,19 @@ Info seq [hh:mm:ss:mss] 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": "9 * Component ",
"fileName": "e:/myproject/node_modules/@types/react/index.d.ts"
}
},
{
"name": "BrowserRouter",
"kind": "warning",
Expand Down
4 changes: 3 additions & 1 deletion tests/cases/fourslash/importStatementCompletions_js.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading