diff --git a/src/services/completions.ts b/src/services/completions.ts index ca10ba189b5a6..6faa7fe88e9df 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -768,7 +768,7 @@ namespace ts.Completions { completionKind === CompletionKind.MemberLike && isClassLikeMemberCompletion(symbol, location)) { let importAdder; - ({ insertText, isSnippet, importAdder } = getEntryForMemberCompletion(host, program, options, preferences, name, symbol, location, contextToken, formatContext)); + ({ insertText, isSnippet, importAdder, replacementSpan } = getEntryForMemberCompletion(host, program, options, preferences, name, symbol, location, contextToken, formatContext)); if (importAdder?.hasFixes()) { hasAction = true; source = CompletionSource.ClassMemberSnippet; @@ -894,13 +894,14 @@ namespace ts.Completions { location: Node, contextToken: Node | undefined, formatContext: formatting.FormatContext | undefined, - ): { insertText: string, isSnippet?: true, importAdder?: codefix.ImportAdder } { + ): { insertText: string, isSnippet?: true, importAdder?: codefix.ImportAdder, replacementSpan?: TextSpan } { const classLikeDeclaration = findAncestor(location, isClassLike); if (!classLikeDeclaration) { return { insertText: name }; } let isSnippet: true | undefined; + let replacementSpan: TextSpan | undefined; let insertText: string = name; const checker = program.getTypeChecker(); @@ -932,10 +933,8 @@ namespace ts.Completions { 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 `|`. - // Note: We are relying on checking if the context token is `abstract`, - // since other visibility modifiers (e.g. `protected`) should come *before* `abstract`. - // However, that is not true for the e.g. `override` modifier, so this check has its limitations. - const isAbstract = contextToken && isModifierLike(contextToken) === SyntaxKind.AbstractKeyword; + const { modifiers: presentModifiers, span: modifiersSpan } = getPresentModifiers(contextToken); + const isAbstract = !!(presentModifiers & ModifierFlags.Abstract); const completionNodes: Node[] = []; codefix.addNewNodeForMemberSymbol( symbol, @@ -961,19 +960,14 @@ namespace ts.Completions { requiredModifiers |= ModifierFlags.Override; } - let presentModifiers = ModifierFlags.None; if (!completionNodes.length) { - // Omit already present modifiers from the first completion node/signature. - if (contextToken) { - presentModifiers = getPresentModifiers(contextToken); - } // Keep track of added missing required modifiers and modifiers already present. // This is needed when we have overloaded signatures, // so this callback will be called for multiple nodes/signatures, // and we need to make sure the modifiers are uniform for all nodes/signatures. modifiers = node.modifierFlagsCache | requiredModifiers | presentModifiers; } - node = factory.updateModifiers(node, modifiers & (~presentModifiers)); + node = factory.updateModifiers(node, modifiers); completionNodes.push(node); }, body, @@ -981,6 +975,7 @@ namespace ts.Completions { isAbstract); if (completionNodes.length) { + replacementSpan = modifiersSpan; // If we have access to formatting settings, we print the nodes using the emitter, // and then format the printed text. if (formatContext) { @@ -1015,11 +1010,15 @@ namespace ts.Completions { } } - return { insertText, isSnippet, importAdder }; + return { insertText, isSnippet, importAdder, replacementSpan }; } - function getPresentModifiers(contextToken: Node): ModifierFlags { + function getPresentModifiers(contextToken: Node | undefined): { modifiers: ModifierFlags, span?: TextSpan } { + if (!contextToken) { + return { modifiers: ModifierFlags.None }; + } let modifiers = ModifierFlags.None; + let span; let contextMod; /* Cases supported: @@ -1042,11 +1041,13 @@ namespace ts.Completions { */ if (contextMod = isModifierLike(contextToken)) { modifiers |= modifierToFlag(contextMod); + span = createTextSpanFromNode(contextToken); } if (isPropertyDeclaration(contextToken.parent)) { modifiers |= modifiersToFlags(contextToken.parent.modifiers); + span = createTextSpanFromNode(contextToken.parent); } - return modifiers; + return { modifiers, span }; } function isModifierLike(node: Node): ModifierSyntaxKind | undefined { diff --git a/tests/cases/fourslash/completionsOverridingMethod.ts b/tests/cases/fourslash/completionsOverridingMethod.ts index 00654d9149f78..dc2623ea18b3e 100644 --- a/tests/cases/fourslash/completionsOverridingMethod.ts +++ b/tests/cases/fourslash/completionsOverridingMethod.ts @@ -93,7 +93,7 @@ //// ////class HSub extends HBase { //// /*h1*/ -//// static /*h2*/ +//// [|static|] /*h2*/ ////} // @Filename: i.ts @@ -124,11 +124,6 @@ verify.completions({ { name: "foo", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "foo(param1: string, param2: boolean): Promise {\n}", } ], @@ -146,11 +141,6 @@ verify.completions({ { name: "foo", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "foo(a: string, b: string): string {\n}", } ], @@ -168,11 +158,6 @@ verify.completions({ { name: "foo", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "foo(a: string): string {\n}", } ], @@ -190,11 +175,6 @@ verify.completions({ { name: "foo", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "foo(a: string): string {\n}", } ], @@ -212,11 +192,6 @@ verify.completions({ { name: "foo", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "foo(a: string): string {\n}", } ], @@ -234,11 +209,6 @@ verify.completions({ { name: "foo", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "foo(a: string): string {\n}", } ], @@ -256,11 +226,6 @@ verify.completions({ { name: "foo", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: `foo(a: string): string; foo(a: undefined, b: number): string; @@ -292,12 +257,8 @@ verify.completions({ { name: "met", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, - insertText: "met(n: number): number {\n}", + replacementSpan: test.ranges()[0], + insertText: "static met(n: number): number {\n}", } ], }); @@ -314,21 +275,11 @@ verify.completions({ { name: "met", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "met(t: T): T {\n}", }, { name: "metcons", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "metcons(t: T): T {\n}", } ], diff --git a/tests/cases/fourslash/completionsOverridingMethod1.ts b/tests/cases/fourslash/completionsOverridingMethod1.ts index 8b59ca919bf24..8158e0cda52e6 100644 --- a/tests/cases/fourslash/completionsOverridingMethod1.ts +++ b/tests/cases/fourslash/completionsOverridingMethod1.ts @@ -25,11 +25,6 @@ verify.completions({ { name: "foo", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "override foo(a: string): void {\n}", } ], diff --git a/tests/cases/fourslash/completionsOverridingMethod10.ts b/tests/cases/fourslash/completionsOverridingMethod10.ts index 64fe76eb933be..44180fe547ee0 100644 --- a/tests/cases/fourslash/completionsOverridingMethod10.ts +++ b/tests/cases/fourslash/completionsOverridingMethod10.ts @@ -26,21 +26,11 @@ verify.completions({ { name: "a", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "a: string;", }, { name: "b", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: `b(a: string): void { }`, @@ -48,11 +38,6 @@ verify.completions({ { name: "c", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: `c(a: string): string; c(a: number): number; diff --git a/tests/cases/fourslash/completionsOverridingMethod11.ts b/tests/cases/fourslash/completionsOverridingMethod11.ts index 0ea880b98021a..e613bbdf078a3 100644 --- a/tests/cases/fourslash/completionsOverridingMethod11.ts +++ b/tests/cases/fourslash/completionsOverridingMethod11.ts @@ -32,21 +32,11 @@ verify.completions({ { name: "a", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "a: string", }, { name: "b", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: `b(a: string): void { }`, @@ -54,11 +44,6 @@ verify.completions({ { name: "c", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: `c(a: string): string c(a: number): number diff --git a/tests/cases/fourslash/completionsOverridingMethod12.ts b/tests/cases/fourslash/completionsOverridingMethod12.ts new file mode 100644 index 0000000000000..6a13f2ae577fd --- /dev/null +++ b/tests/cases/fourslash/completionsOverridingMethod12.ts @@ -0,0 +1,54 @@ +/// + +// @Filename: a.ts +// @newline: LF +// Case: modifier order +////abstract class A { +//// public get P(): string { +//// return ""; +//// } +////} +//// +////abstract class B extends A { +//// [|abstract|] /*a*/ +////} +//// +////abstract class B1 extends A { +//// [|abstract override|] /*b*/ +////} + +verify.completions({ + marker: "a", + isNewIdentifierLocation: true, + preferences: { + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: false, + includeCompletionsWithClassMemberSnippets: true, + }, + includes: [ + { + name: "P", + sortText: completion.SortText.LocationPriority, + replacementSpan: test.ranges()[0], + insertText: "public abstract get P(): string;", + }, + ], +}); + +verify.completions({ + marker: "b", + isNewIdentifierLocation: true, + preferences: { + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: false, + includeCompletionsWithClassMemberSnippets: true, + }, + includes: [ + { + name: "P", + sortText: completion.SortText.LocationPriority, + replacementSpan: test.ranges()[1], + insertText: "public abstract override get P(): string;", + }, + ], +}); \ No newline at end of file diff --git a/tests/cases/fourslash/completionsOverridingMethod2.ts b/tests/cases/fourslash/completionsOverridingMethod2.ts index f479259d18c42..5a134e8aeab26 100644 --- a/tests/cases/fourslash/completionsOverridingMethod2.ts +++ b/tests/cases/fourslash/completionsOverridingMethod2.ts @@ -23,11 +23,6 @@ verify.completions({ { name: "$usd", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, isSnippet: true, insertText: "\"\\$usd\"(a: number): number {\n $0\n}", } diff --git a/tests/cases/fourslash/completionsOverridingMethod3.ts b/tests/cases/fourslash/completionsOverridingMethod3.ts index c6ffd11c37a14..ac4081cdd2662 100644 --- a/tests/cases/fourslash/completionsOverridingMethod3.ts +++ b/tests/cases/fourslash/completionsOverridingMethod3.ts @@ -24,11 +24,6 @@ verify.completions({ { name: "boo", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "boo(): string;", } ], diff --git a/tests/cases/fourslash/completionsOverridingMethod4.ts b/tests/cases/fourslash/completionsOverridingMethod4.ts index 6b70dda01154c..5c0f0f1ae06fc 100644 --- a/tests/cases/fourslash/completionsOverridingMethod4.ts +++ b/tests/cases/fourslash/completionsOverridingMethod4.ts @@ -43,21 +43,11 @@ verify.completions({ { name: "hint", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "protected hint(): string {\n}", }, { name: "refuse", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "public refuse(): string {\n}", } ], diff --git a/tests/cases/fourslash/completionsOverridingMethod5.ts b/tests/cases/fourslash/completionsOverridingMethod5.ts index 6fa1564180fe7..d2638434146ec 100644 --- a/tests/cases/fourslash/completionsOverridingMethod5.ts +++ b/tests/cases/fourslash/completionsOverridingMethod5.ts @@ -12,8 +12,8 @@ //// ////abstract class Abc extends Ab { //// /*a*/ -//// abstract /*b*/ -//// abstract m/*c*/ +//// [|abstract|] /*b*/ +//// [|abstract m|]/*c*/ ////} @@ -29,21 +29,11 @@ verify.completions({ { name: "met", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "met(n: string): void {\n}", }, { name: "met2", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "met2(n: number): void {\n}", } ], @@ -61,22 +51,14 @@ verify.completions({ { name: "met", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, - insertText: "met(n: string): void;", + replacementSpan: test.ranges()[0], + insertText: "abstract met(n: string): void;", }, { name: "met2", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, - insertText: "met2(n: number): void;", + replacementSpan: test.ranges()[0], + insertText: "abstract met2(n: number): void;", } ], }); @@ -93,22 +75,14 @@ verify.completions({ { name: "met", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, - insertText: "met(n: string): void;", + replacementSpan: test.ranges()[1], + insertText: "abstract met(n: string): void;", }, { name: "met2", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, - insertText: "met2(n: number): void;", + replacementSpan: test.ranges()[1], + insertText: "abstract met2(n: number): void;", } ], }); diff --git a/tests/cases/fourslash/completionsOverridingMethod6.ts b/tests/cases/fourslash/completionsOverridingMethod6.ts index 9bdf1b969785a..4b97adba9bb95 100644 --- a/tests/cases/fourslash/completionsOverridingMethod6.ts +++ b/tests/cases/fourslash/completionsOverridingMethod6.ts @@ -10,11 +10,11 @@ ////} //// ////abstract class B extends A { -//// public abstract /*b*/ +//// [|public abstract|] /*b*/ ////} //// ////class C extends A { -//// public override m/*a*/ +//// [|public override m|]/*a*/ ////} //// ////interface D { @@ -23,7 +23,7 @@ ////} //// ////class E implements D { -//// public f/*c*/ +//// [|public f|]/*c*/ ////} verify.completions({ @@ -38,12 +38,8 @@ verify.completions({ { name: "method", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, - insertText: "method(): number {\n}", + replacementSpan: test.ranges()[1], + insertText: "public override method(): number {\n}", }, ], }); @@ -60,12 +56,8 @@ verify.completions({ { name: "method", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, - insertText: "method(): number;", + replacementSpan: test.ranges()[0], + insertText: "public abstract method(): number;", }, ], }); @@ -82,13 +74,9 @@ verify.completions({ { name: "fun", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, + replacementSpan: test.ranges()[2], insertText: -`fun(a: number): number; +`public fun(a: number): number; public fun(a: undefined, b: string): number; public fun(a: unknown, b?: unknown): number { }`, diff --git a/tests/cases/fourslash/completionsOverridingMethod7.ts b/tests/cases/fourslash/completionsOverridingMethod7.ts index 0fa6c30643ad7..051c518a0ada0 100644 --- a/tests/cases/fourslash/completionsOverridingMethod7.ts +++ b/tests/cases/fourslash/completionsOverridingMethod7.ts @@ -9,7 +9,7 @@ ////} //// ////abstract class Derived extends Base { -//// abstract /*a*/ +//// [|abstract|] /*a*/ ////} verify.completions({ @@ -24,13 +24,9 @@ verify.completions({ { name: "M", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, + replacementSpan: test.ranges()[0], insertText: -`M(t: T): void; +`abstract M(t: T): void; abstract M(t: T, x: number): void;`, }, ], diff --git a/tests/cases/fourslash/completionsOverridingMethod8.ts b/tests/cases/fourslash/completionsOverridingMethod8.ts index fced40bd21e2e..cd717aae5d411 100644 --- a/tests/cases/fourslash/completionsOverridingMethod8.ts +++ b/tests/cases/fourslash/completionsOverridingMethod8.ts @@ -27,11 +27,6 @@ verify.completions({ includes: [{ name: "method", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "method(p: I): void {\n}", hasAction: true, source: completion.CompletionSource.ClassMemberSnippet, diff --git a/tests/cases/fourslash/completionsOverridingMethod9.ts b/tests/cases/fourslash/completionsOverridingMethod9.ts index 76f91a68c2b7e..5ce63927083ba 100644 --- a/tests/cases/fourslash/completionsOverridingMethod9.ts +++ b/tests/cases/fourslash/completionsOverridingMethod9.ts @@ -23,21 +23,11 @@ verify.completions({ { name: "a", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "a?: number;" }, { name: "b", sortText: completion.SortText.LocationPriority, - replacementSpan: { - fileName: "", - pos: 0, - end: 0, - }, insertText: "b(x: number): void {\n}" }, ],