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 15 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
9 changes: 9 additions & 0 deletions src/services/completions.ts
View file
Open in desktop
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]?.getSourceFile()) || !!(symbol.flags & SymbolFlags.Type));
sandersn marked this conversation as resolved.
Show resolved Hide resolved
}
}

function getLabelCompletionAtPosition(node: BreakOrContinueStatement): CompletionInfo | undefined {
Expand Down
37 changes: 33 additions & 4 deletions src/testRunner/unittests/tsserver/completions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ 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 = {
Expand All @@ -51,7 +53,17 @@ 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,
},
},
],
},
});

Expand All @@ -63,7 +75,17 @@ 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);
Expand Down Expand Up @@ -102,6 +124,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 @@ -128,6 +151,8 @@ import {
| string
| ((props: any, context?: any) => any)
| (new (props: any, context?: any) => any);

export const PropTypes = {};
`,
};

Expand Down Expand Up @@ -195,6 +220,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
View file
Open in desktop
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": []
}
]
}
}
]
Loading
Loading