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 dd9ff9b
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 32 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
7 changes: 5 additions & 2 deletions src/testRunner/unittests/tsserver/completions.ts
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 {}
`,
};
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
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
5 changes: 4 additions & 1 deletion tests/cases/fourslash/jsFileImportNoTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down

0 comments on commit dd9ff9b

Please sign in to comment.