Added override keyword to codefix implemented abstract methods (#51033)

* Added override keyword to codefixed implemented abstract methods

* Only when noImplicitOverrides is true, and always check abstract modifier

* Added test on abstract/override already being there

* Added back a few test cases

* Check declaration modifier, not class modifier

Author: JoshuaKGoldberg

src/compiler/core.ts

@@ -1137,6 +1137,8 @@ export function append<T>(to: T[], value: T | undefined): T[] | undefined {
  *
  * @internal
  */
+export function combine<T>(xs: T[] | undefined, ys: T[] | undefined): T[] | undefined;
+/** @internal */
 export function combine<T>(xs: T | readonly T[] | undefined, ys: T | readonly T[] | undefined): T | readonly T[] | undefined;
 /** @internal */
 export function combine<T>(xs: T | T[] | undefined, ys: T | T[] | undefined): T | T[] | undefined;

src/services/codefixes/helpers.ts

@@ -8,6 +8,7 @@ import {
     CharacterCodes,
     ClassLikeDeclaration,
     CodeFixContextBase,
+    combine,
     Debug,
     Diagnostics,
     emptyArray,
@@ -31,6 +32,7 @@ import {
     getSynthesizedDeepClone,
     getTokenAtPosition,
     getTsConfigObjectLiteralExpression,
+    hasAbstractModifier,
     Identifier,
     idText,
     IntersectionType,
@@ -197,7 +199,7 @@ export function addNewNodeForMemberSymbol(
     if (declaration && isAutoAccessorPropertyDeclaration(declaration)) {
         modifierFlags |= ModifierFlags.Accessor;
     }
-    const modifiers = modifierFlags ? factory.createNodeArray(factory.createModifiersFromModifierFlags(modifierFlags)) : undefined;
+    const modifiers = createModifiers();
     const type = checker.getWidenedType(checker.getTypeOfSymbolAtLocation(symbol, enclosingDeclaration));
     const optional = !!(symbol.flags & SymbolFlags.Optional);
     const ambient = !!(enclosingDeclaration.flags & NodeFlags.Ambient) || isAmbient;
@@ -304,6 +306,25 @@ export function addNewNodeForMemberSymbol(
         if (method) addClassElement(method);
     }
 
+
+    function createModifiers(): NodeArray<Modifier> | undefined {
+        let modifiers: Modifier[] | undefined;
+
+        if (modifierFlags) {
+            modifiers = combine(modifiers, factory.createModifiersFromModifierFlags(modifierFlags));
+        }
+
+        if (shouldAddOverrideKeyword()) {
+            modifiers = append(modifiers, factory.createToken(SyntaxKind.OverrideKeyword));
+        }
+
+        return modifiers && factory.createNodeArray(modifiers);
+    }
+
+    function shouldAddOverrideKeyword(): boolean {
+        return !!(context.program.getCompilerOptions().noImplicitOverride && declaration && hasAbstractModifier(declaration));
+    }
+
     function createName(node: PropertyName) {
         if (isIdentifier(node) && node.escapedText === "constructor") {
             return factory.createComputedPropertyName(factory.createStringLiteral(idText(node), quotePreference === QuotePreference.Single));

tests/cases/fourslash/codeFixAmbientClassExtendAbstractMethod.ts

@@ -1,5 +1,6 @@
 /// <reference path='fourslash.ts' />
 
+// @noImplicitOverride: true
 ////abstract class A {
 ////    abstract f(a: number, b: string): boolean;
 ////    abstract f(a: number, b: string): this;
@@ -30,12 +31,12 @@ verify.codeFix({
 }
 
 declare class C extends A {
-    f(a: number, b: string): boolean;
-    f(a: number, b: string): this;
-    f(a: string, b: number): Function;
-    f(a: string): Function;
-    f1(this: A): number;
-    f2(this: A, a: number, b: string): number;
-    foo(): number;
+    override f(a: number, b: string): boolean;
+    override f(a: number, b: string): this;
+    override f(a: string, b: number): Function;
+    override f(a: string): Function;
+    override f1(this: A): number;
+    override f2(this: A, a: number, b: string): number;
+    override foo(): number;
 }`
 });

tests/cases/fourslash/codeFixClassExtendAbstractGetterSetter.ts

@@ -1,5 +1,6 @@
 /// <reference path='fourslash.ts' />
 
+// @noImplicitOverride: true
 ////abstract class A {
 ////    abstract get a(): number | string;
 ////    abstract get b(): this;
@@ -38,28 +39,28 @@ verify.codeFix({
 abstract class B extends A {}
 
 class C extends A {
-    get a(): string | number {
+    override get a(): string | number {
         throw new Error("Method not implemented.");
     }
-    get b(): this {
+    override get b(): this {
         throw new Error("Method not implemented.");
     }
-    get c(): A {
+    override get c(): A {
         throw new Error("Method not implemented.");
     }
-    set d(arg: string | number) {
+    override set d(arg: string | number) {
         throw new Error("Method not implemented.");
     }
-    set e(arg: this) {
+    override set e(arg: this) {
         throw new Error("Method not implemented.");
     }
-    set f(arg: A) {
+    override set f(arg: A) {
         throw new Error("Method not implemented.");
     }
-    get g(): string {
+    override get g(): string {
         throw new Error("Method not implemented.");
     }
-    set g(newName: string) {
+    override set g(newName: string) {
         throw new Error("Method not implemented.");
     }
 }`

tests/cases/fourslash/codeFixClassExtendAbstractMethod.ts

@@ -1,5 +1,6 @@
 /// <reference path='fourslash.ts' />
 
+// @noImplicitOverride: true
 ////abstract class A {
 ////    abstract f(a: number, b: string): boolean;
 ////    abstract f(a: number, b: string): this;
@@ -22,14 +23,14 @@ verify.codeFix({
 }
 
 class C extends A {
-    f(a: number, b: string): boolean;
-    f(a: number, b: string): this;
-    f(a: string, b: number): Function;
-    f(a: string): Function;
-    f(a: unknown, b?: unknown): boolean | Function | this {
+    override f(a: number, b: string): boolean;
+    override f(a: number, b: string): this;
+    override f(a: string, b: number): Function;
+    override f(a: string): Function;
+    override f(a: unknown, b?: unknown): boolean | Function | this {
         throw new Error("Method not implemented.");
     }
-    foo(): number {
+    override foo(): number {
         throw new Error("Method not implemented.");
     }
 }`

tests/cases/fourslash/codeFixClassExtendAbstractMethodModifiersAlreadyIncludeOverride.ts

@@ -0,0 +1,29 @@
+/// <reference path='fourslash.ts' />
+
+// @noImplicitOverride: true
+//// abstract class A {
+////   abstract foo(n: number | string): number | string;
+//// }
+////
+//// abstract class B extends A {
+////   abstract override foo(n: number): number;
+//// }
+////
+//// class C extends B { }
+
+verify.codeFix({
+  description: "Implement inherited abstract class",
+  newFileContent: `abstract class A {
+  abstract foo(n: number | string): number | string;
+}
+
+abstract class B extends A {
+  abstract override foo(n: number): number;
+}
+
+class C extends B {
+    override foo(n: number): number {
+        throw new Error("Method not implemented.");
+    }
+}`,
+});

tests/cases/fourslash/codeFixClassExtendAbstractMethod_all.ts

@@ -1,5 +1,6 @@
 /// <reference path='fourslash.ts' />
 
+// @noImplicitOverride: true
 ////abstract class A {
 ////    abstract m(): void;
 ////    abstract n(): void;
@@ -16,18 +17,18 @@ verify.codeFixAll({
     abstract n(): void;
 }
 class B extends A {
-    m(): void {
+    override m(): void {
         throw new Error("Method not implemented.");
     }
-    n(): void {
+    override n(): void {
         throw new Error("Method not implemented.");
     }
 }
 class C extends A {
-    m(): void {
+    override m(): void {
         throw new Error("Method not implemented.");
     }
-    n(): void {
+    override n(): void {
         throw new Error("Method not implemented.");
     }
 }`,

tests/cases/fourslash/codeFixClassExtendAbstractMethod_comment.ts

@@ -1,5 +1,6 @@
 /// <reference path='fourslash.ts' />
 
+// @noImplicitOverride: true
 ////abstract class A {
 ////    abstract m() : void;
 ////}
@@ -16,7 +17,7 @@ verify.codeFix({
 }
 
 class B extends A {
-    m(): void {
+    override m(): void {
         throw new Error("Method not implemented.");
     }
     // comment

tests/cases/fourslash/codeFixClassExtendAbstractPrivateProperty.ts

@@ -1,5 +1,6 @@
 /// <reference path='fourslash.ts' />
 
+// @noImplicitOverride: true
 //// abstract class A {
 ////    private abstract x: number;
 ////    m() { this.x; } // Avoid unused private

tests/cases/fourslash/codeFixClassExtendAbstractProperty.ts

@@ -1,5 +1,6 @@
 /// <reference path='fourslash.ts' />
 
+// @noImplicitOverride: true
 ////abstract class A {
 ////    abstract x: number;
 ////    abstract y: this;
@@ -18,8 +19,8 @@ verify.codeFix({
 }
 
 class C extends A {
-    x: number;
-    y: this;
-    z: A;
+    override x: number;
+    override y: this;
+    override z: A;
 }`
 });