Properly set hasAction for erasing modifiers in class member completions

src/compiler/diagnosticMessages.json

@@ -7019,6 +7019,10 @@
         "category": "Message",
         "code": 90060
     },
+    "Update modifiers of '{0}'": {
+        "category": "Message",
+        "code": 90061
+    },
 
     "Convert function to an ES2015 class": {
         "category": "Message",

src/services/completions.ts

@@ -1757,10 +1757,21 @@ function createCompletionEntry(
         isClassLikeMemberCompletion(symbol, location, sourceFile)
     ) {
         let importAdder;
-        const memberCompletionEntry = getEntryForMemberCompletion(host, program, options, preferences, name, symbol, location, position, contextToken, formatContext);
+        const memberCompletionEntry = getEntryForMemberCompletion(
+            host,
+            program,
+            options,
+            preferences,
+            name,
+            symbol,
+            location,
+            position,
+            contextToken,
+            formatContext,
+        );
         if (memberCompletionEntry) {
             ({ insertText, filterText, isSnippet, importAdder } = memberCompletionEntry);
-            if (importAdder?.hasFixes()) {
+            if (importAdder?.hasFixes() || memberCompletionEntry.eraseRange) {
                 hasAction = true;
                 source = CompletionSource.ClassMemberSnippet;
             }
@@ -2068,6 +2079,7 @@ function getPresentModifiers(
     let decorators: Decorator[] | undefined;
     let contextMod;
     const range: TextRange = { pos: position, end: position };
+
     /*
     Cases supported:
     In
@@ -2083,23 +2095,41 @@ function getPresentModifiers(
     }`
         `contextToken` is ``override`` (as a keyword),
     `contextToken.parent` is property declaration,
-    `location` is identifier ``m``,
-    `location.parent` is property declaration ``protected override m``,
-    `location.parent.parent` is class declaration ``class C { ... }``.
+    `location` is identifier ``m``.
     */
-    if (isPropertyDeclaration(contextToken.parent) && contextToken.parent.modifiers) {
-        modifiers |= modifiersToFlags(contextToken.parent.modifiers) & ModifierFlags.Modifier;
-        decorators = contextToken.parent.modifiers.filter(isDecorator) || [];
-        range.pos = Math.min(range.pos, contextToken.parent.modifiers.pos);
-    }
-    if (contextMod = isModifierLike(contextToken)) {
+    if (isPropertyDeclaration(contextToken.parent) && (contextMod = isModifierLike(contextToken))) {
+        if (contextToken.parent.modifiers) {
+            modifiers |= modifiersToFlags(contextToken.parent.modifiers) & ModifierFlags.Modifier;
+            decorators = contextToken.parent.modifiers.filter(isDecorator) || [];
+            range.pos = Math.min(...contextToken.parent.modifiers.map(n => n.getStart(sourceFile)));
+        }
         const contextModifierFlag = modifierToFlag(contextMod);
         if (!(modifiers & contextModifierFlag)) {
             modifiers |= contextModifierFlag;
-            range.pos = Math.min(range.pos, contextToken.pos);
+            range.pos = Math.min(range.pos, contextToken.getStart(sourceFile));
+        }
+        /*
+        We have two cases:
+        1.
+        `class C {
+            modifier |
+        }`
+        `contextToken` is `modifier`,
+        and the range should be `modifier |`, ending at `position`,
+        and
+        2.
+        `class C {
+            modifier otherToken|
+        }`
+        `contextToken` is `modifier`,
+        `contextToken.parent.name` is `otherToken`,
+        and the range should be `modifier `, ending at the start of `otherToken`.
+        */
+        if (contextToken.parent.name !== contextToken) {
+            range.end = contextToken.parent.name.getStart(sourceFile);
         }
     }
-    return { modifiers, decorators, range: range.pos !== position ? range : undefined };
+    return { modifiers, decorators, range: range.pos < range.end ? range : undefined };
 }
 
 function isModifierLike(node: Node): ModifierSyntaxKind | undefined {
@@ -2941,7 +2971,7 @@ function getCompletionEntryCodeActionsAndSourceDisplay(
             contextToken,
             formatContext,
         )!;
-        if (importAdder || eraseRange) {
+        if (importAdder?.hasFixes() || eraseRange) {
             const changes = textChanges.ChangeTracker.with(
                 { host, formatContext, preferences },
                 tracker => {
@@ -2957,7 +2987,9 @@ function getCompletionEntryCodeActionsAndSourceDisplay(
                 sourceDisplay: undefined,
                 codeActions: [{
                     changes,
-                    description: diagnosticToString([Diagnostics.Includes_imports_of_types_referenced_by_0, name]),
+                    description: importAdder?.hasFixes() ?
+                        diagnosticToString([Diagnostics.Includes_imports_of_types_referenced_by_0, name]) :
+                        diagnosticToString([Diagnostics.Update_modifiers_of_0, name]),
                 }],
             };
         }

tests/cases/fourslash/completionsOverridingMethod0.ts

@@ -267,6 +267,8 @@ verify.completions({
             sortText: completion.SortText.LocationPriority,
             insertText: "static met(n: number): number {\n}",
             filterText: "met",
+            hasAction: true,
+            source: completion.CompletionSource.ClassMemberSnippet,
         }
     ],
 });

tests/cases/fourslash/completionsOverridingMethod12.ts

@@ -31,6 +31,8 @@ verify.completions({
             sortText: completion.SortText.LocationPriority,
             insertText: "abstract get P(): string;",
             filterText: "P",
+            hasAction: true,
+            source: completion.CompletionSource.ClassMemberSnippet,
         },
     ],
 });
@@ -49,6 +51,33 @@ verify.completions({
             sortText: completion.SortText.LocationPriority,
             insertText: "abstract override get P(): string;",
             filterText: "P",
+            hasAction: true,
+            source: completion.CompletionSource.ClassMemberSnippet,
         },
     ],
+});
+
+verify.applyCodeActionFromCompletion("b", {
+    preferences: {
+      includeCompletionsWithInsertText: true,
+      includeCompletionsWithSnippetText: false,
+      includeCompletionsWithClassMemberSnippets: true,
+    },
+    name: "P",
+    source: completion.CompletionSource.ClassMemberSnippet,
+    description: "Update modifiers of 'P'",
+    newFileContent:
+`abstract class A {
+    public get P(): string {
+        return "";
+    }
+}
+
+abstract class B extends A {
+    abstract 
+}
+
+abstract class B1 extends A {
+
+}`
 });

tests/cases/fourslash/completionsOverridingMethod18.ts

@@ -0,0 +1,67 @@
+/// <reference path="fourslash.ts" />
+
+// @Filename: a.ts
+// @newline: LF
+
+//// declare function decorator(...args: any[]): any;
+
+//// class DecoratorBase {
+////     protected foo(a: string): string;
+////     protected foo(a: number): number;
+////     protected foo(a: any): any {
+////         return a;
+////     }
+//// }
+
+//// class DecoratorSub extends DecoratorBase {
+////     @decorator protected /**/
+//// }
+
+verify.completions({
+    marker: "",
+    isNewIdentifierLocation: true,
+    preferences: {
+        includeCompletionsWithInsertText: true,
+        includeCompletionsWithSnippetText: false,
+        includeCompletionsWithClassMemberSnippets: true,
+    },
+    includes: [
+        {
+            name: "foo",
+            sortText: completion.SortText.LocationPriority,
+            insertText:
+`protected foo(a: string): string;
+protected foo(a: number): number;
+@decorator
+protected foo(a: any) {
+}`,
+            filterText: "foo",
+            replacementSpan: undefined,
+            hasAction: true,
+            source: completion.CompletionSource.ClassMemberSnippet,
+        },
+    ]
+});
+
+verify.applyCodeActionFromCompletion("", {
+    preferences: {
+      includeCompletionsWithInsertText: true,
+      includeCompletionsWithSnippetText: false,
+      includeCompletionsWithClassMemberSnippets: true,
+    },
+    name: "foo",
+    source: completion.CompletionSource.ClassMemberSnippet,
+    description: "Update modifiers of 'foo'",
+    newFileContent:
+`declare function decorator(...args: any[]): any;
+class DecoratorBase {
+    protected foo(a: string): string;
+    protected foo(a: number): number;
+    protected foo(a: any): any {
+        return a;
+    }
+}
+class DecoratorSub extends DecoratorBase {
+
+}`
+});

tests/cases/fourslash/completionsOverridingMethod19.ts

@@ -0,0 +1,60 @@
+/// <reference path="fourslash.ts" />
+
+// @Filename: a.ts
+// @newline: LF
+//// class Base {
+////     method() {}
+////     protected prop = 1;
+//// }
+//// class E extends Base {
+////     protected notamodifier override /**/
+//// }
+
+verify.completions({
+    marker: "",
+    isNewIdentifierLocation: true,
+    preferences: {
+        includeCompletionsWithInsertText: true,
+        includeCompletionsWithSnippetText: false,
+        includeCompletionsWithClassMemberSnippets: true,
+    },
+    includes: [
+        {
+            name: "method",
+            sortText: completion.SortText.LocationPriority,
+            insertText: "override method(): void {\n}",
+            filterText: "method",
+            replacementSpan: undefined,
+            hasAction: true,
+            source: completion.CompletionSource.ClassMemberSnippet,
+        },
+        {
+            name: "prop",
+            sortText: completion.SortText.LocationPriority,
+            insertText: "protected override prop: number;",
+            filterText: "prop",
+            replacementSpan: undefined,
+            hasAction: true,
+            source: completion.CompletionSource.ClassMemberSnippet,
+        },
+    ]
+},);
+
+verify.applyCodeActionFromCompletion("", {
+    preferences: {
+      includeCompletionsWithInsertText: true,
+      includeCompletionsWithSnippetText: false,
+      includeCompletionsWithClassMemberSnippets: true,
+    },
+    name: "method",
+    source: completion.CompletionSource.ClassMemberSnippet,
+    description: "Update modifiers of 'method'",
+    newFileContent:
+`class Base {
+    method() {}
+    protected prop = 1;
+}
+class E extends Base {
+    protected notamodifier 
+}`
+});

tests/cases/fourslash/completionsOverridingMethod20.ts

@@ -0,0 +1,49 @@
+/// <reference path="fourslash.ts" />
+
+// @Filename: a.ts
+// @newline: LF
+//// abstract class AFoo {
+////     abstract bar(): Promise<void>;
+//// }
+//// class Foo extends AFoo {
+////     async b/*a*/
+//// }
+
+verify.completions({
+    marker: "a",
+    isNewIdentifierLocation: true,
+    preferences: {
+        includeCompletionsWithInsertText: true,
+        includeCompletionsWithSnippetText: false,
+        includeCompletionsWithClassMemberSnippets: true,
+    },
+    includes: [
+        {
+            name: "bar",
+            sortText: completion.SortText.LocationPriority,
+            insertText: "async bar(): Promise<void> {\n}",
+            filterText: "bar",
+            replacementSpan: undefined,
+            hasAction: true,
+            source: completion.CompletionSource.ClassMemberSnippet,
+        },
+    ]
+});
+
+verify.applyCodeActionFromCompletion("a", {
+    preferences: {
+      includeCompletionsWithInsertText: true,
+      includeCompletionsWithSnippetText: false,
+      includeCompletionsWithClassMemberSnippets: true,
+    },
+    name: "bar",
+    source: completion.CompletionSource.ClassMemberSnippet,
+    description: "Update modifiers of 'bar'",
+    newFileContent:
+`abstract class AFoo {
+    abstract bar(): Promise<void>;
+}
+class Foo extends AFoo {
+    b
+}`
+});

tests/cases/fourslash/completionsOverridingMethod21.ts

@@ -0,0 +1,48 @@
+/// <reference path="fourslash.ts" />
+
+// @Filename: a.ts
+// @newline: LF
+//// abstract class AFoo {
+////     abstract bar(): Promise<void>;
+//// }
+//// class BFoo extends AFoo {
+////     async /*b*/
+//// }
+
+verify.completions({
+    marker: "b",
+    isNewIdentifierLocation: true,
+    preferences: {
+        includeCompletionsWithInsertText: true,
+        includeCompletionsWithSnippetText: false,
+        includeCompletionsWithClassMemberSnippets: true,
+    },
+    includes: [
+        {
+            name: "bar",
+            sortText: completion.SortText.LocationPriority,
+            insertText: "async bar(): Promise<void> {\n}",
+            filterText: "bar",
+            replacementSpan: undefined,
+            hasAction: true,
+            source: completion.CompletionSource.ClassMemberSnippet,
+        },
+    ]
+});
+verify.applyCodeActionFromCompletion("b", {
+    preferences: {
+      includeCompletionsWithInsertText: true,
+      includeCompletionsWithSnippetText: false,
+      includeCompletionsWithClassMemberSnippets: true,
+    },
+    name: "bar",
+    source: completion.CompletionSource.ClassMemberSnippet,
+    description: "Update modifiers of 'bar'",
+    newFileContent:
+`abstract class AFoo {
+    abstract bar(): Promise<void>;
+}
+class BFoo extends AFoo {
+
+}`
+});