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.

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

Filtered out types from import suggestions in JS files #53619

Merged
merged 19 commits into from
Nov 16, 2023
Merged

Filtered out types from import suggestions in JS files #53619

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
bf9dd25
Filtered out types from import suggestions in JS files
JoshuaKGoldberg Apr 1, 2023
c7ff6b2
Account for JSDoc type locations in JS files
JoshuaKGoldberg Apr 1, 2023
3773af1
Added a bit more to the test case
JoshuaKGoldberg Apr 1, 2023
119a10b
Update src/services/completions.ts
JoshuaKGoldberg Apr 4, 2023
1ab7b75
Post suggestion: remove unused function
JoshuaKGoldberg Apr 4, 2023
ae5fc3d
Correcting to !(symbol.flags & SymbolFlags.Value)
JoshuaKGoldberg Apr 6, 2023
b4b7b98
Went back to symbol check function
JoshuaKGoldberg Apr 6, 2023
be9fba8
Naming and comments
JoshuaKGoldberg Apr 7, 2023
dd9ff9b
Almost working with SymbolFlags.Value
JoshuaKGoldberg Apr 13, 2023
336aaec
looking forward to dprint
JoshuaKGoldberg Apr 13, 2023
b8c99c2
Merge branch 'main'
JoshuaKGoldberg Apr 13, 2023
012679f
Twitch review-as-a-service
JoshuaKGoldberg Apr 13, 2023
e344ec8
Added javascriptModulesTypeImport
JoshuaKGoldberg Apr 17, 2023
5644a4f
Account for JS file type-only exports
JoshuaKGoldberg Apr 18, 2023
f30827c
Merge branch 'main'
JoshuaKGoldberg Oct 18, 2023
c1f7cb4
Merge branch 'main' into no-types-in-esm-imports
JoshuaKGoldberg Oct 21, 2023
9eecdd5
Add a sub-namespace type, and undid some formatting changes
JoshuaKGoldberg Oct 21, 2023
5bc890c
Merge branch 'main' into no-types-in-esm-imports
sandersn Oct 26, 2023
8de257d
Remove unnecessary getSourceFile
JoshuaKGoldberg Nov 6, 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
7 changes: 6 additions & 1 deletion src/services/completions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2084,6 +2084,11 @@ export function getCompletionEntriesFromSymbols(
continue;
}

// When in a value-time 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())) {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -2132,7 +2137,7 @@ export function getCompletionEntriesFromSymbols(
has: name => uniques.has(name),
add: name => uniques.set(name, true),
};

JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
function shouldIncludeSymbol(symbol: Symbol, symbolToSortTextMap: SymbolSortTextMap): boolean {
let allFlags = symbol.flags;
if (!isSourceFile(location)) {
Expand Down
7 changes: 5 additions & 2 deletions src/testRunner/unittests/tsserver/completions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ 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 = {
path: `${localNodeModules}/react-router-dom/package.json`,
Expand All @@ -127,7 +128,9 @@ import {
| string
| ((props: any, context?: any) => any)
| (new (props: any, context?: any) => any);
`

export const PropTypes = {};
`,
};

const globalCacheLocation = `c:/typescript`;
Expand Down
264 changes: 264 additions & 0 deletions tests/baselines/reference/jsFileImportNoTypes.baseline
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,264 @@
=== /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": []
}
]
}
}
]
27 changes: 22 additions & 5 deletions ...sserver/completions/works-when-files-are-included-from-two-different-drives-of-windows.js
View file
Open in desktop
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 @@ -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 "/// <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 @@ -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: *
Expand Down Expand Up @@ -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",
Expand Down
4 changes: 3 additions & 1 deletion tests/cases/fourslash/importStatementCompletions_js.ts
View file
Open in desktop
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
4 changes: 3 additions & 1 deletion tests/cases/fourslash/importStatementCompletions_js2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading