From a65957cc7cf6a5400a932726db8c5fc4acf9fd6f Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Tue, 6 Dec 2022 12:14:16 -0800 Subject: [PATCH 1/6] WIP: filter existing values in case completions --- src/compiler/factory/nodeTests.ts | 6 ++ src/compiler/types.ts | 1 + src/services/completions.ts | 24 ++++- tests/cases/fourslash/switchCompletions.ts | 107 +++++++++++++++++++++ 4 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 tests/cases/fourslash/switchCompletions.ts diff --git a/src/compiler/factory/nodeTests.ts b/src/compiler/factory/nodeTests.ts index 14e296610c54f..59525500a6c19 100644 --- a/src/compiler/factory/nodeTests.ts +++ b/src/compiler/factory/nodeTests.ts @@ -23,6 +23,7 @@ import { CallSignatureDeclaration, CaseBlock, CaseClause, + CaseKeyword, CatchClause, ClassDeclaration, ClassExpression, @@ -379,6 +380,11 @@ export function isImportKeyword(node: Node): node is ImportExpression { return node.kind === SyntaxKind.ImportKeyword; } +/** @internal */ +export function isCaseKeyword(node: Node): node is CaseKeyword { + return node.kind === SyntaxKind.CaseKeyword; +} + // Names export function isQualifiedName(node: Node): node is QualifiedName { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 7ce54d3dd6c02..28ce587a04957 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -1344,6 +1344,7 @@ export interface KeywordToken extends Token; export type AssertKeyword = KeywordToken; export type AwaitKeyword = KeywordToken; +export type CaseKeyword = KeywordToken; /** @deprecated Use `AwaitKeyword` instead. */ export type AwaitKeywordToken = AwaitKeyword; diff --git a/src/services/completions.ts b/src/services/completions.ts index a09aebcb7d1a3..8f4862dd1f43e 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -352,6 +352,8 @@ import { UserPreferences, VariableDeclaration, walkUpParenthesizedExpressions, + isCaseKeyword, + isNodeDescendantOf, } from "./_namespaces/ts"; import { StringCompletions } from "./_namespaces/ts.Completions"; @@ -859,7 +861,6 @@ function completionInfoFromData( location, propertyAccessToConvert, keywordFilters, - literals, symbolToOriginInfoMap, recommendedCompletion, isJsxInitializer, @@ -868,9 +869,12 @@ function completionInfoFromData( isRightOfOpenTag, importStatementCompletion, insideJsDocTagTypeExpression, - symbolToSortTextMap: symbolToSortTextMap, + symbolToSortTextMap, hasUnresolvedAutoImports, } = completionData; + let literals = completionData.literals; + + const checker = program.getTypeChecker(); // Verify if the file is JSX language variant if (getLanguageVariant(sourceFile.scriptKind) === LanguageVariant.JSX) { @@ -880,6 +884,22 @@ function completionInfoFromData( } } + let caseClause = findAncestor(contextToken, isCaseClause); + if (caseClause && (isCaseKeyword(contextToken!) || isNodeDescendantOf(contextToken!, caseClause.expression))) { // >> TODO: this needs to be expanded, for cases like `case foo.bar.| etc (what others?) + // Filter existing literals + const tracker = newCaseClauseTracker(checker, caseClause.parent.clauses); + literals = literals.filter(literal => !tracker.hasValue(literal)); + symbols = symbols.filter(symbol => { + if (symbol.valueDeclaration && isEnumMember(symbol.valueDeclaration)) { // >> TODO: if isEnum + // const enumValue = type.symbol.valueDeclaration && checker.getConstantValue(type.symbol.valueDeclaration as EnumMember); + const value = checker.getConstantValue(symbol.valueDeclaration); + return !value || !tracker.hasValue(value); + } + return true; + }); // >> TODO: can't really use filter and modify symbol array because symbolToSortTextMap and symbolToOriginMap are arrays that depend on symbol's index + + } + const entries = createSortedArray(); const isChecked = isCheckedFile(sourceFile, compilerOptions); if (isChecked && !isNewIdentifierLocation && (!symbols || symbols.length === 0) && keywordFilters === KeywordCompletionFilters.None) { diff --git a/tests/cases/fourslash/switchCompletions.ts b/tests/cases/fourslash/switchCompletions.ts new file mode 100644 index 0000000000000..a01c5cab76aba --- /dev/null +++ b/tests/cases/fourslash/switchCompletions.ts @@ -0,0 +1,107 @@ +/// + +//// enum E { A, B } +//// declare const e: E; +//// switch (e) { +//// case E.A: +//// return 0; +//// case E./*1*/ +//// } +//// declare const f: 1 | 2 | 3; +//// switch (f) { +//// case 1: +//// return 1; +//// case /*2*/ +//// } + +verify.completions( + { + marker: "1", + isNewIdentifierLocation: false, + includes: ["B"], + excludes: "A", + }, + { + marker: "2", + isNewIdentifierLocation: false, + excludes: "1", + includes: ["2", "3"], + } +); + +// verify.completions( +// { +// marker: "1", +// isNewIdentifierLocation: false, +// includes: [ +// { +// name: "case E.A: ...", +// source: completion.CompletionSource.SwitchCases, +// sortText: completion.SortText.GlobalsOrKeywords, +// insertText: +// `case E.A: +// case E.B: +// case 1:`, +// }, +// ], +// preferences: { +// includeCompletionsWithInsertText: true, +// }, +// }, +// { +// marker: "2", +// isNewIdentifierLocation: false, +// includes: [ +// { +// name: "case E.A: ...", +// source: completion.CompletionSource.SwitchCases, +// sortText: completion.SortText.GlobalsOrKeywords, +// insertText: +// `case E.A: +// case E.B: +// case E.C:`, +// }, +// ], +// preferences: { +// includeCompletionsWithInsertText: true, +// }, +// }, +// { +// marker: "3", +// isNewIdentifierLocation: false, +// includes: [ +// { +// name: "case F.D: ...", +// source: completion.CompletionSource.SwitchCases, +// sortText: completion.SortText.GlobalsOrKeywords, +// insertText: +// `case F.D: +// case F.E: +// case F.F:`, +// }, +// ], +// preferences: { +// includeCompletionsWithInsertText: true, +// }, +// }, +// { +// marker: "3", +// isNewIdentifierLocation: false, +// includes: [ +// { +// name: "case F.D: ...", +// source: completion.CompletionSource.SwitchCases, +// sortText: completion.SortText.GlobalsOrKeywords, +// isSnippet: true, +// insertText: +// `case F.D:$1 +// case F.E:$2 +// case F.F:$3`, +// }, +// ], +// preferences: { +// includeCompletionsWithInsertText: true, +// includeCompletionsWithSnippetText: true, +// }, +// }, +// ); \ No newline at end of file From 43bf85a47c17c4ab2ae493396f0b6d280c403c5f Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Tue, 6 Dec 2022 13:22:25 -0800 Subject: [PATCH 2/6] filter existing enum symbols --- src/services/completions.ts | 24 ++++--- tests/cases/fourslash/switchCompletions.ts | 79 +--------------------- 2 files changed, 17 insertions(+), 86 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 8f4862dd1f43e..d517fe4aa3cb5 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -431,6 +431,7 @@ export const enum SymbolOriginInfoKind { ResolvedExport = 1 << 5, TypeOnlyAlias = 1 << 6, ObjectLiteralMethod = 1 << 7, + Ignore = 1 << 8, SymbolMemberNoExport = SymbolMember, SymbolMemberExport = SymbolMember | Export, @@ -509,6 +510,10 @@ function originIsObjectLiteralMethod(origin: SymbolOriginInfo | undefined): orig return !!(origin && origin.kind & SymbolOriginInfoKind.ObjectLiteralMethod); } +function originIsIgnore(origin: SymbolOriginInfo | undefined): boolean { + return !!(origin && origin.kind & SymbolOriginInfoKind.Ignore); +} + /** @internal */ export interface UniqueNameSet { add(name: string): void; @@ -885,19 +890,19 @@ function completionInfoFromData( } let caseClause = findAncestor(contextToken, isCaseClause); - if (caseClause && (isCaseKeyword(contextToken!) || isNodeDescendantOf(contextToken!, caseClause.expression))) { // >> TODO: this needs to be expanded, for cases like `case foo.bar.| etc (what others?) + if (caseClause && (isCaseKeyword(contextToken!) || isNodeDescendantOf(contextToken!, caseClause.expression))) { // Filter existing literals const tracker = newCaseClauseTracker(checker, caseClause.parent.clauses); literals = literals.filter(literal => !tracker.hasValue(literal)); - symbols = symbols.filter(symbol => { - if (symbol.valueDeclaration && isEnumMember(symbol.valueDeclaration)) { // >> TODO: if isEnum - // const enumValue = type.symbol.valueDeclaration && checker.getConstantValue(type.symbol.valueDeclaration as EnumMember); + // Mark already present symbols to be ignored + symbols.forEach((symbol, i) => { + if (symbol.valueDeclaration && isEnumMember(symbol.valueDeclaration)) { const value = checker.getConstantValue(symbol.valueDeclaration); - return !value || !tracker.hasValue(value); + if (value !== undefined && tracker.hasValue(value)) { + symbolToOriginInfoMap[i] = { kind: SymbolOriginInfoKind.Ignore }; + } } - return true; - }); // >> TODO: can't really use filter and modify symbol array because symbolToSortTextMap and symbolToOriginMap are arrays that depend on symbol's index - + }); } const entries = createSortedArray(); @@ -4529,6 +4534,9 @@ function getCompletionEntryDisplayNameForSymbol( kind: CompletionKind, jsxIdentifierExpected: boolean, ): CompletionEntryDisplayNameForSymbol | undefined { + if (originIsIgnore(origin)) { + return undefined; + } const name = originIncludesSymbolName(origin) ? origin.symbolName : symbol.name; if (name === undefined // If the symbol is external module, don't show it in the completion list diff --git a/tests/cases/fourslash/switchCompletions.ts b/tests/cases/fourslash/switchCompletions.ts index a01c5cab76aba..425ae9c89451f 100644 --- a/tests/cases/fourslash/switchCompletions.ts +++ b/tests/cases/fourslash/switchCompletions.ts @@ -27,81 +27,4 @@ verify.completions( excludes: "1", includes: ["2", "3"], } -); - -// verify.completions( -// { -// marker: "1", -// isNewIdentifierLocation: false, -// includes: [ -// { -// name: "case E.A: ...", -// source: completion.CompletionSource.SwitchCases, -// sortText: completion.SortText.GlobalsOrKeywords, -// insertText: -// `case E.A: -// case E.B: -// case 1:`, -// }, -// ], -// preferences: { -// includeCompletionsWithInsertText: true, -// }, -// }, -// { -// marker: "2", -// isNewIdentifierLocation: false, -// includes: [ -// { -// name: "case E.A: ...", -// source: completion.CompletionSource.SwitchCases, -// sortText: completion.SortText.GlobalsOrKeywords, -// insertText: -// `case E.A: -// case E.B: -// case E.C:`, -// }, -// ], -// preferences: { -// includeCompletionsWithInsertText: true, -// }, -// }, -// { -// marker: "3", -// isNewIdentifierLocation: false, -// includes: [ -// { -// name: "case F.D: ...", -// source: completion.CompletionSource.SwitchCases, -// sortText: completion.SortText.GlobalsOrKeywords, -// insertText: -// `case F.D: -// case F.E: -// case F.F:`, -// }, -// ], -// preferences: { -// includeCompletionsWithInsertText: true, -// }, -// }, -// { -// marker: "3", -// isNewIdentifierLocation: false, -// includes: [ -// { -// name: "case F.D: ...", -// source: completion.CompletionSource.SwitchCases, -// sortText: completion.SortText.GlobalsOrKeywords, -// isSnippet: true, -// insertText: -// `case F.D:$1 -// case F.E:$2 -// case F.F:$3`, -// }, -// ], -// preferences: { -// includeCompletionsWithInsertText: true, -// includeCompletionsWithSnippetText: true, -// }, -// }, -// ); \ No newline at end of file +); \ No newline at end of file From 333979d9c1033040c440634f52a7dec0b6676130 Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Tue, 6 Dec 2022 14:08:08 -0800 Subject: [PATCH 3/6] add comment --- src/services/completions.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index d517fe4aa3cb5..6c19ca65530e8 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -889,12 +889,12 @@ function completionInfoFromData( } } + // When the completion is for the expression of a case clause (e.g. `case |`), + // filter literals & enum symbols whose values are already present in existing case clauses. let caseClause = findAncestor(contextToken, isCaseClause); if (caseClause && (isCaseKeyword(contextToken!) || isNodeDescendantOf(contextToken!, caseClause.expression))) { - // Filter existing literals const tracker = newCaseClauseTracker(checker, caseClause.parent.clauses); literals = literals.filter(literal => !tracker.hasValue(literal)); - // Mark already present symbols to be ignored symbols.forEach((symbol, i) => { if (symbol.valueDeclaration && isEnumMember(symbol.valueDeclaration)) { const value = checker.getConstantValue(symbol.valueDeclaration); From f95a16943dfbd8ea7a5e26ed6a95f7af5c79427b Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Tue, 6 Dec 2022 14:30:42 -0800 Subject: [PATCH 4/6] fix lint errors --- src/services/completions.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 6c19ca65530e8..a72b38bd9bb3b 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -127,6 +127,7 @@ import { isCallExpression, isCaseBlock, isCaseClause, + isCaseKeyword, isCheckJsEnabledForFile, isClassElement, isClassLike, @@ -191,6 +192,7 @@ import { isNamedImports, isNamedImportsOrExports, isNamespaceImport, + isNodeDescendantOf, isObjectBindingPattern, isObjectLiteralExpression, isObjectTypeDeclaration, @@ -352,8 +354,6 @@ import { UserPreferences, VariableDeclaration, walkUpParenthesizedExpressions, - isCaseKeyword, - isNodeDescendantOf, } from "./_namespaces/ts"; import { StringCompletions } from "./_namespaces/ts.Completions"; @@ -891,7 +891,7 @@ function completionInfoFromData( // When the completion is for the expression of a case clause (e.g. `case |`), // filter literals & enum symbols whose values are already present in existing case clauses. - let caseClause = findAncestor(contextToken, isCaseClause); + const caseClause = findAncestor(contextToken, isCaseClause); if (caseClause && (isCaseKeyword(contextToken!) || isNodeDescendantOf(contextToken!, caseClause.expression))) { const tracker = newCaseClauseTracker(checker, caseClause.parent.clauses); literals = literals.filter(literal => !tracker.hasValue(literal)); From bd14233d0091f1a378b3df93ca6b4f673b00aec4 Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Tue, 6 Dec 2022 14:53:20 -0800 Subject: [PATCH 5/6] update baselines --- tests/baselines/reference/api/tsserverlibrary.d.ts | 1 + tests/baselines/reference/api/typescript.d.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index e3e8466ccd1e7..16991df4f7ca7 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -4547,6 +4547,7 @@ declare namespace ts { type AssertsKeyword = KeywordToken; type AssertKeyword = KeywordToken; type AwaitKeyword = KeywordToken; + type CaseKeyword = KeywordToken; /** @deprecated Use `AwaitKeyword` instead. */ type AwaitKeywordToken = AwaitKeyword; /** @deprecated Use `AssertsKeyword` instead. */ diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 7bc06c330da8a..7f8076812c4ab 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -611,6 +611,7 @@ declare namespace ts { type AssertsKeyword = KeywordToken; type AssertKeyword = KeywordToken; type AwaitKeyword = KeywordToken; + type CaseKeyword = KeywordToken; /** @deprecated Use `AwaitKeyword` instead. */ type AwaitKeywordToken = AwaitKeyword; /** @deprecated Use `AssertsKeyword` instead. */ From ede43772e8af423a7ab852d2775724a0695232cb Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Tue, 13 Dec 2022 12:27:20 -0800 Subject: [PATCH 6/6] add comment --- src/services/completions.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/services/completions.ts b/src/services/completions.ts index a72b38bd9bb3b..d2f33ae70e4a6 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -895,6 +895,9 @@ function completionInfoFromData( if (caseClause && (isCaseKeyword(contextToken!) || isNodeDescendantOf(contextToken!, caseClause.expression))) { const tracker = newCaseClauseTracker(checker, caseClause.parent.clauses); literals = literals.filter(literal => !tracker.hasValue(literal)); + // The `symbols` array cannot be filtered directly, because to each symbol at position i in `symbols`, + // there might be a corresponding origin at position i in `symbolToOriginInfoMap`. + // So instead of filtering the `symbols` array, we mark symbols to be ignored. symbols.forEach((symbol, i) => { if (symbol.valueDeclaration && isEnumMember(symbol.valueDeclaration)) { const value = checker.getConstantValue(symbol.valueDeclaration);