Skip to content

Commit

Permalink
Almost working with SymbolFlags.Value
Browse files Browse the repository at this point in the history
  • Loading branch information
JoshuaKGoldberg committed Apr 13, 2023
1 parent be9fba8 commit 1a16619
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 69 deletions.
21 changes: 2 additions & 19 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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) {
Expand Down
112 changes: 73 additions & 39 deletions src/testRunner/unittests/tsserver/completions.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
import * as ts from "../../_namespaces/ts";
import {
createServerHost,
File,
libFile,
} from "../virtualFileSystemWithWatch";
import { createServerHost, File, libFile } from "../virtualFileSystemWithWatch";
import {
baselineTsserverLogs,
createLoggerWithInMemoryLogs,
Expand All @@ -28,7 +24,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 @@ -37,33 +35,55 @@ describe("unittests:: tsserver:: completions", () => {
offset: 3,
};

const response = session.executeCommandSeq<ts.server.protocol.CompletionsRequest>({
command: ts.server.protocol.CommandTypes.CompletionInfo,
arguments: {
...requestLocation,
includeExternalModuleExports: true,
prefix: "foo",
}
}).response as ts.server.protocol.CompletionInfo;
const response =
session.executeCommandSeq<ts.server.protocol.CompletionsRequest>({
command: ts.server.protocol.CommandTypes.CompletionInfo,
arguments: {
...requestLocation,
includeExternalModuleExports: true,
prefix: "foo",
},
}).response as ts.server.protocol.CompletionInfo;
const exportMapKey = (response?.entries[0].data as any)?.exportMapKey;
session.executeCommandSeq<ts.server.protocol.CompletionDetailsRequest>({
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,
},
},
],
},
});

interface CompletionDetailsFullRequest extends ts.server.protocol.FileLocationRequest {
interface CompletionDetailsFullRequest
extends ts.server.protocol.FileLocationRequest {
readonly command: ts.server.protocol.CommandTypes.CompletionDetailsFull;
readonly arguments: ts.server.protocol.CompletionDetailsRequestArgs;
}
session.executeCommandSeq<CompletionDetailsFullRequest>({
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 All @@ -76,18 +96,18 @@ describe("unittests:: tsserver:: completions", () => {
name: "test",
version: "0.1.0",
dependencies: {
"react": "^16.12.0",
react: "^16.12.0",
"react-router-dom": "^5.1.2",
}
})
},
}),
};
const appFile: File = {
path: `${projectRoot}/src/app.js`,
content: `import React from 'react';
import {
BrowserRouter as Router,
} from "react-router-dom";
`
`,
};
const localNodeModules = `${projectRoot}/node_modules`;
const localAtTypes = `${localNodeModules}/@types`;
Expand All @@ -96,38 +116,41 @@ import {
content: JSON.stringify({
name: "@types/react",
version: "16.9.14",
})
}),
};
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`,
content: JSON.stringify({
name: "react-router-dom",
version: "5.1.2",
})
}),
};
const localReactRouterDom: File = {
path: `${localNodeModules}/react-router-dom/index.js`,
content: `export function foo() {}`
content: `export function foo() {}`,
};
const localPropTypesPackage: File = {
path: `${localAtTypes}/prop-types/package.json`,
content: JSON.stringify({
name: "@types/prop-types",
version: "15.7.3",
})
}),
};
const localPropTypes: File = {
path: `${localAtTypes}/prop-types/index.d.ts`,
content: `export type ReactComponentLike =
| string
| ((props: any, context?: any) => any)
| (new (props: any, context?: any) => any);
`
export const PropTypes = {};
`,
};

const globalCacheLocation = `c:/typescript`;
Expand All @@ -137,7 +160,7 @@ import {
content: JSON.stringify({
name: "@types/react-router-dom",
version: "5.1.2",
})
}),
};
const globalReactRouterDom: File = {
path: `${globalAtTypes}/react-router-dom/index.d.ts`,
Expand All @@ -147,15 +170,15 @@ export interface BrowserRouterProps {
getUserConfirmation?: ((message: string, callback: (ok: boolean) => void) => void);
forceRefresh?: boolean;
keyLength?: number;
}`
}`,
};
const globalReactPackage: File = {
path: `${globalAtTypes}/react/package.json`,
content: localReactPackage.content
content: localReactPackage.content,
};
const globalReact: File = {
path: `${globalAtTypes}/react/index.d.ts`,
content: localReact.content
content: localReact.content,
};

const filesInProject = [
Expand All @@ -167,9 +190,11 @@ export interface BrowserRouterProps {
];
const files = [
...filesInProject,
appPackage, libFile,
appPackage,
libFile,
localReactPackage,
localReactRouterDomPackage, localReactRouterDom,
localReactRouterDomPackage,
localReactRouterDom,
localPropTypesPackage,
globalReactRouterDomPackage,
globalReactPackage,
Expand All @@ -178,7 +203,12 @@ export interface BrowserRouterProps {
const host = createServerHost(files, { windowsStyleRoot: "c:/" });
const logger = createLoggerWithInMemoryLogs(host);
const session = createSession(host, {
typingsInstaller: new TestTypingsInstaller(globalCacheLocation, /*throttleLimit*/ 5, host, logger),
typingsInstaller: new TestTypingsInstaller(
globalCacheLocation,
/*throttleLimit*/ 5,
host,
logger
),
logger,
});
openFilesForSession([appFile], session);
Expand All @@ -189,9 +219,13 @@ export interface BrowserRouterProps {
line: 5,
offset: 1,
includeExternalModuleExports: true,
includeInsertTextCompletions: true
}
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
);
});
});
6 changes: 3 additions & 3 deletions tests/baselines/reference/jsFileImportNoTypes.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// | interface TestClassInterfaceMerged
// | enum TestEnum
// | function testFunction(): void
// | namespace TestNamespace
// | namespace TestNamespaceWithValue
// | const testValue: {}
// | ----------------------------------------------------------------------

Expand Down Expand Up @@ -201,7 +201,7 @@
"documentation": []
},
{
"name": "TestNamespace",
"name": "TestNamespaceWithValue",
"kind": "module",
"kindModifiers": "export",
"sortText": "11",
Expand All @@ -215,7 +215,7 @@
"kind": "space"
},
{
"text": "TestNamespace",
"text": "TestNamespaceWithValue",
"kind": "moduleName"
}
],
Expand Down
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
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
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

0 comments on commit 1a16619

Please sign in to comment.