From 67a3846fbf6b5fe75ee1ce2735a5b7d0ec91648b Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Mon, 16 Jan 2023 15:32:24 -0300 Subject: [PATCH] Fix `replacementSpan` for class member snippet completion entries (#52231) --- src/harness/fourslashImpl.ts | 6 +++- src/services/completions.ts | 20 ++++++++--- src/services/stringCompletions.ts | 4 ++- .../completionsClassMembers2.baseline | 4 --- .../completionsOverridingMethod17.ts | 34 +++++++++++++++++++ .../fourslash/importStatementCompletions1.ts | 2 +- 6 files changed, 59 insertions(+), 11 deletions(-) create mode 100644 tests/cases/fourslash/completionsOverridingMethod17.ts diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index ee0e6e599eb..a0d32959563 100644 --- a/src/harness/fourslashImpl.ts +++ b/src/harness/fourslashImpl.ts @@ -984,8 +984,9 @@ export class TestState { if (actual.insertText !== expected.insertText) { this.raiseError(`At entry ${actual.name}: Completion insert text did not match: ${showTextDiff(expected.insertText || "", actual.insertText || "")}`); } + const convertedReplacementSpan = expected.replacementSpan && ts.createTextSpanFromRange(expected.replacementSpan); - if (convertedReplacementSpan?.length) { + if (convertedReplacementSpan) { try { assert.deepEqual(actual.replacementSpan, convertedReplacementSpan); } @@ -993,6 +994,9 @@ export class TestState { this.raiseError(`At entry ${actual.name}: Expected completion replacementSpan to be ${stringify(convertedReplacementSpan)}, got ${stringify(actual.replacementSpan)}`); } } + else if (ts.hasProperty(expected, "replacementSpan")) { // Expected `replacementSpan` is explicitly set as `undefined`. + assert.equal(actual.replacementSpan, undefined, `At entry ${actual.name}: Expected 'replacementSpan' properties to match`); + } if (expected.kind !== undefined || expected.kindModifiers !== undefined) { assert.equal(actual.kind, expected.kind, `At entry ${actual.name}: Expected 'kind' for ${actual.name} to match`); diff --git a/src/services/completions.ts b/src/services/completions.ts index 02427425fdf..9ff348a2f5b 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -927,6 +927,7 @@ function completionInfoFromData( /*replacementToken*/ undefined, contextToken, location, + position, sourceFile, host, program, @@ -1316,6 +1317,7 @@ function createCompletionEntry( replacementToken: Node | undefined, contextToken: Node | undefined, location: Node, + position: number, sourceFile: SourceFile, host: LanguageServiceHost, program: Program, @@ -1406,7 +1408,8 @@ function createCompletionEntry( completionKind === CompletionKind.MemberLike && isClassLikeMemberCompletion(symbol, location, sourceFile)) { let importAdder; - ({ insertText, isSnippet, importAdder, replacementSpan } = getEntryForMemberCompletion(host, program, options, preferences, name, symbol, location, contextToken, formatContext)); + ({ insertText, isSnippet, importAdder, replacementSpan } = + getEntryForMemberCompletion(host, program, options, preferences, name, symbol, location, position, contextToken, formatContext)); sortText = SortText.ClassMemberSnippets; // sortText has to be lower priority than the sortText for keywords. See #47852. if (importAdder?.hasFixes()) { hasAction = true; @@ -1545,6 +1548,7 @@ function getEntryForMemberCompletion( name: string, symbol: Symbol, location: Node, + position: number, contextToken: Node | undefined, formatContext: formatting.FormatContext | undefined, ): { insertText: string, isSnippet?: true, importAdder?: codefix.ImportAdder, replacementSpan?: TextSpan } { @@ -1586,7 +1590,7 @@ function getEntryForMemberCompletion( let modifiers = ModifierFlags.None; // Whether the suggested member should be abstract. // e.g. in `abstract class C { abstract | }`, we should offer abstract method signatures at position `|`. - const { modifiers: presentModifiers, span: modifiersSpan } = getPresentModifiers(contextToken); + const { modifiers: presentModifiers, span: modifiersSpan } = getPresentModifiers(contextToken, sourceFile, position); const isAbstract = !!(presentModifiers & ModifierFlags.Abstract); const completionNodes: Node[] = []; codefix.addNewNodeForMemberSymbol( @@ -1650,8 +1654,13 @@ function getEntryForMemberCompletion( return { insertText, isSnippet, importAdder, replacementSpan }; } -function getPresentModifiers(contextToken: Node | undefined): { modifiers: ModifierFlags, span?: TextSpan } { - if (!contextToken) { +function getPresentModifiers( + contextToken: Node | undefined, + sourceFile: SourceFile, + position: number): { modifiers: ModifierFlags, span?: TextSpan } { + if (!contextToken || + getLineAndCharacterOfPosition(sourceFile, position).line + > getLineAndCharacterOfPosition(sourceFile, contextToken.getEnd()).line) { return { modifiers: ModifierFlags.None }; } let modifiers = ModifierFlags.None; @@ -2086,6 +2095,7 @@ export function getCompletionEntriesFromSymbols( replacementToken: Node | undefined, contextToken: Node | undefined, location: Node, + position: number, sourceFile: SourceFile, host: LanguageServiceHost, program: Program, @@ -2132,6 +2142,7 @@ export function getCompletionEntriesFromSymbols( replacementToken, contextToken, location, + position, sourceFile, host, program, @@ -2471,6 +2482,7 @@ function getCompletionEntryCodeActionsAndSourceDisplay( name, symbol, location, + position, contextToken, formatContext); if (importAdder) { diff --git a/src/services/stringCompletions.ts b/src/services/stringCompletions.ts index 310c550dc33..6ff21d4159b 100644 --- a/src/services/stringCompletions.ts +++ b/src/services/stringCompletions.ts @@ -198,7 +198,7 @@ export function getStringLiteralCompletions( if (isInString(sourceFile, position, contextToken)) { if (!contextToken || !isStringLiteralLike(contextToken)) return undefined; const entries = getStringLiteralCompletionEntries(sourceFile, contextToken, position, program.getTypeChecker(), options, host, preferences); - return convertStringLiteralCompletions(entries, contextToken, sourceFile, host, program, log, options, preferences); + return convertStringLiteralCompletions(entries, contextToken, sourceFile, host, program, log, options, preferences, position); } } @@ -211,6 +211,7 @@ function convertStringLiteralCompletions( log: Log, options: CompilerOptions, preferences: UserPreferences, + position: number, ): CompletionInfo | undefined { if (completion === undefined) { return undefined; @@ -228,6 +229,7 @@ function convertStringLiteralCompletions( contextToken, contextToken, sourceFile, + position, sourceFile, host, program, diff --git a/tests/baselines/reference/completionsClassMembers2.baseline b/tests/baselines/reference/completionsClassMembers2.baseline index eb68eaf45a2..ee7b6c59877 100644 --- a/tests/baselines/reference/completionsClassMembers2.baseline +++ b/tests/baselines/reference/completionsClassMembers2.baseline @@ -173,10 +173,6 @@ "kindModifiers": "", "sortText": "17", "insertText": "method(): void {\r\n}", - "replacementSpan": { - "start": 71, - "length": 16 - }, "displayParts": [ { "text": "(", diff --git a/tests/cases/fourslash/completionsOverridingMethod17.ts b/tests/cases/fourslash/completionsOverridingMethod17.ts new file mode 100644 index 00000000000..20bf195d485 --- /dev/null +++ b/tests/cases/fourslash/completionsOverridingMethod17.ts @@ -0,0 +1,34 @@ +/// + +// @Filename: a.ts +// @newline: LF + +// Issue #52211 + +//// interface Interface { +//// method(): void; +//// } +//// +//// export class Class implements Interface { +//// property = "yadda"; +//// +//// /**/ +//// } + +verify.completions({ + marker: "", + isNewIdentifierLocation: true, + preferences: { + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: false, + includeCompletionsWithClassMemberSnippets: true, + }, + includes: [ + { + name: "method", + sortText: completion.SortText.ClassMemberSnippets, + insertText: "method(): void {\n}", + replacementSpan: undefined, + }, + ], +}); diff --git a/tests/cases/fourslash/importStatementCompletions1.ts b/tests/cases/fourslash/importStatementCompletions1.ts index 0e970254346..4364d64d741 100644 --- a/tests/cases/fourslash/importStatementCompletions1.ts +++ b/tests/cases/fourslash/importStatementCompletions1.ts @@ -20,7 +20,7 @@ //// [|import f/*4*/ =|] // @Filename: /index5.ts -//// import f/*5*/ from ""; +//// [|import f/*5*/ from "";|] ([[0, true], [1, true], [2, false], [3, true], [4, true], [5, true]] as const).forEach(([marker, typeKeywordValid]) => { verify.completions({