-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Fix completions at type-only export specifiers
#62651
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -179,6 +179,7 @@ import { | |
| isImportDeclaration, | ||
| isImportEqualsDeclaration, | ||
| isImportKeyword, | ||
| isImportOrExportSpecifier, | ||
| isImportSpecifier, | ||
| isInComment, | ||
| isIndexSignatureDeclaration, | ||
|
|
@@ -4651,7 +4652,7 @@ function getCompletionData( | |
| if (!namedImportsOrExports) return GlobalsSearch.Continue; | ||
|
|
||
| // We can at least offer `type` at `import { |` | ||
| if (!isTypeKeywordTokenOrIdentifier(contextToken)) { | ||
| if (!isTypeKeywordTokenOrIdentifier(contextToken) && !isTypeOnlyImportOrExportDeclaration(contextToken.kind === SyntaxKind.OpenBraceToken || contextToken.kind === SyntaxKind.CommaToken ? contextToken.parent.parent : contextToken.parent)) { | ||
| keywordFilters = KeywordCompletionFilters.TypeKeyword; | ||
| } | ||
|
|
||
|
|
@@ -5007,7 +5008,8 @@ function getCompletionData( | |
|
|
||
| case SyntaxKind.TypeKeyword: | ||
| // import { type foo| } | ||
| return containingNodeKind !== SyntaxKind.ImportSpecifier; | ||
| // export { type foo| } | ||
| return !isImportOrExportSpecifier(parent); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the only change needed to accomplish the goal of this PR. I threw in the rest of the changes to fix some redundant |
||
|
|
||
| case SyntaxKind.AsteriskToken: | ||
| return isFunctionLike(contextToken.parent) && !isMethodDeclaration(contextToken.parent); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| /// <reference path='fourslash.ts'/> | ||
|
|
||
| // @Filename: m1.ts | ||
| //// export var foo: number = 1; | ||
| //// export var jkl: number = 2; | ||
| //// export type MyType = string; | ||
|
|
||
| // @Filename: m2.ts | ||
| //// export { /*1*/ } from "./m1" | ||
| //// export type { /*2*/ } from "./m1" | ||
| //// export { type /*3*/ } from "./m1" | ||
| //// export { foo as foo1, /*4*/ } from "./m1" | ||
| //// export type { foo as foo2, /*5*/ } from "./m1" | ||
| //// | ||
| //// export { M/*6*/ } from "./m1" | ||
| //// export type { M/*7*/ } from "./m1" | ||
| //// export { type M/*8*/ } from "./m1" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixing this is the main goal of this PR. At the moment, on the
|
||
|
|
||
| const type = { name: "type", sortText: completion.SortText.GlobalsOrKeywords }; | ||
|
|
||
| verify.completions( | ||
| { marker: ["1", "6"], exact: ["foo", "jkl", "MyType", type] }, | ||
| { marker: ["2", "3", "7", "8"], exact: ["foo", "jkl", "MyType"] }, | ||
| { marker: "4", exact: ["jkl", "MyType", type] }, | ||
| { marker: "5", exact: ["jkl", "MyType"] }, | ||
| ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| /// <reference path='fourslash.ts'/> | ||
|
|
||
| // @Filename: m1.ts | ||
| //// export var foo: number = 1; | ||
| //// export var jkl: number = 2; | ||
| //// export type MyType = string; | ||
|
|
||
| // @Filename: m2.ts | ||
| //// import { /*1*/ } from "./m1" | ||
| //// import type { /*2*/ } from "./m1" | ||
| //// import { type /*3*/ } from "./m1" | ||
| //// import { foo as foo1, /*4*/ } from "./m1" | ||
| //// import type { foo as foo2, /*5*/ } from "./m1" | ||
| //// | ||
| //// import { M/*6*/ } from "./m1" | ||
| //// import type { M/*7*/ } from "./m1" | ||
| //// import { type M/*8*/ } from "./m1" | ||
|
|
||
| const type = { name: "type", sortText: completion.SortText.GlobalsOrKeywords }; | ||
|
|
||
| verify.completions( | ||
| { marker: ["1", "6"], exact: ["foo", "jkl", "MyType", type] }, | ||
| { marker: ["2", "3", "7", "8"], exact: ["foo", "jkl", "MyType"] }, | ||
| { marker: "4", exact: ["jkl", "MyType", type] }, | ||
| { marker: "5", exact: ["jkl", "MyType"] }, | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those extra requirements were quite odd here (and got in a way of the tests I was adding for
typekeyword inclusion/exclusion). An export declaration should be treated as type-only regardless of the existence of other fields on that export declaration AST node. The introduced change also matches the import-oriented counterpart of the containing function (isTypeOnlyImportDeclaration) where there are no extra conditions in theSyntaxKind.ImportSpecifiercase.