implement code fix for override of js files

src/compiler/emitter.ts

@@ -1613,6 +1613,10 @@ namespace ts {
                         return emitJSDocSignature(node as JSDocSignature);
                     case SyntaxKind.JSDocTag:
                     case SyntaxKind.JSDocClassTag:
+                    case SyntaxKind.JSDocPublicTag:
+                    case SyntaxKind.JSDocPrivateTag:
+                    case SyntaxKind.JSDocProtectedTag:
+                    case SyntaxKind.JSDocOverrideTag:
                         return emitJSDocSimpleTag(node as JSDocTag);
                     case SyntaxKind.JSDocAugmentsTag:
                     case SyntaxKind.JSDocImplementsTag:
@@ -1621,11 +1625,7 @@ namespace ts {
                     case SyntaxKind.JSDocDeprecatedTag:
                         return;
                     // SyntaxKind.JSDocClassTag (see JSDocTag, above)
-                    case SyntaxKind.JSDocPublicTag:
-                    case SyntaxKind.JSDocPrivateTag:
-                    case SyntaxKind.JSDocProtectedTag:
                     case SyntaxKind.JSDocReadonlyTag:
-                    case SyntaxKind.JSDocOverrideTag:
                         return;
                     case SyntaxKind.JSDocCallbackTag:
                         return emitJSDocCallbackTag(node as JSDocCallbackTag);

src/compiler/types.ts

@@ -7401,7 +7401,7 @@ namespace ts {
         updateJSDocUnknownTag(node: JSDocUnknownTag, tagName: Identifier, comment: string | NodeArray<JSDocComment> | undefined): JSDocUnknownTag;
         createJSDocDeprecatedTag(tagName: Identifier, comment?: string | NodeArray<JSDocComment>): JSDocDeprecatedTag;
         updateJSDocDeprecatedTag(node: JSDocDeprecatedTag, tagName: Identifier, comment?: string | NodeArray<JSDocComment>): JSDocDeprecatedTag;
-        createJSDocOverrideTag(tagName: Identifier, comment?: string | NodeArray<JSDocComment>): JSDocOverrideTag;
+        createJSDocOverrideTag(tagName: Identifier | undefined, comment?: string | NodeArray<JSDocComment>): JSDocOverrideTag;
         updateJSDocOverrideTag(node: JSDocOverrideTag, tagName: Identifier, comment?: string | NodeArray<JSDocComment>): JSDocOverrideTag;
         createJSDocText(text: string): JSDocText;
         updateJSDocText(node: JSDocText, text: string): JSDocText;

src/services/codefixes/fixOverrideModifier.ts

@@ -20,34 +20,49 @@ namespace ts.codefix {
         Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code
     ];
 
-    const errorCodeFixIdMap: Record<number, [DiagnosticMessage, string | undefined, DiagnosticMessage | undefined]> = {
-        [Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0.code]: [
-            Diagnostics.Add_override_modifier, fixAddOverrideId, Diagnostics.Add_all_missing_override_modifiers,
-        ],
-        [Diagnostics.This_member_cannot_have_an_override_modifier_because_its_containing_class_0_does_not_extend_another_class.code]: [
-            Diagnostics.Remove_override_modifier, fixRemoveOverrideId, Diagnostics.Remove_all_unnecessary_override_modifiers
-        ],
-        [Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code]: [
-            Diagnostics.Add_override_modifier, fixAddOverrideId, Diagnostics.Add_all_missing_override_modifiers,
-        ],
-        [Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0.code]: [
-            Diagnostics.Add_override_modifier, fixAddOverrideId, Diagnostics.Remove_all_unnecessary_override_modifiers
-        ],
-        [Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0.code]: [
-            Diagnostics.Remove_override_modifier, fixRemoveOverrideId, Diagnostics.Remove_all_unnecessary_override_modifiers
-        ]
+    interface ErrorCodeFixInfo {
+        descriptions: DiagnosticMessage;
+        fixId?: string | undefined;
+        fixAllDescriptions?: DiagnosticMessage | undefined;
+    }
+
+    const errorCodeFixIdMap: Record<number, ErrorCodeFixInfo> = {
+        [Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0.code]: {
+            descriptions: Diagnostics.Add_override_modifier,
+            fixId: fixAddOverrideId,
+            fixAllDescriptions: Diagnostics.Add_all_missing_override_modifiers,
+        },
+        [Diagnostics.This_member_cannot_have_an_override_modifier_because_its_containing_class_0_does_not_extend_another_class.code]: {
+            descriptions: Diagnostics.Remove_override_modifier,
+            fixId: fixRemoveOverrideId,
+            fixAllDescriptions: Diagnostics.Remove_all_unnecessary_override_modifiers,
+        },
+        [Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code]: {
+            descriptions: Diagnostics.Add_override_modifier,
+            fixId: fixAddOverrideId,
+            fixAllDescriptions: Diagnostics.Add_all_missing_override_modifiers,
+        },
+        [Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0.code]: {
+            descriptions: Diagnostics.Add_override_modifier,
+            fixId: fixAddOverrideId,
+            fixAllDescriptions: Diagnostics.Remove_all_unnecessary_override_modifiers,
+        },
+        [Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0.code]: {
+            descriptions: Diagnostics.Remove_override_modifier,
+            fixId: fixRemoveOverrideId,
+            fixAllDescriptions: Diagnostics.Remove_all_unnecessary_override_modifiers,
+        }
     };
 
     registerCodeFix({
         errorCodes,
         getCodeActions: context => {
-            const { errorCode, span, sourceFile } = context;
+            const { errorCode, span } = context;
 
             const info = errorCodeFixIdMap[errorCode];
             if (!info) return emptyArray;
 
-            const [ descriptions, fixId, fixAllDescriptions ] = info;
-            if (isSourceFileJS(sourceFile)) return emptyArray;
+            const { descriptions, fixId, fixAllDescriptions } = info;
             const changes = textChanges.ChangeTracker.with(context, changes => dispatchChanges(changes, context, errorCode, span.start));
 
             return [
@@ -57,9 +72,9 @@ namespace ts.codefix {
         fixIds: [fixName, fixAddOverrideId, fixRemoveOverrideId],
         getAllCodeActions: context =>
             codeFixAll(context, errorCodes, (changes, diag) => {
-                const { code, start, file } = diag;
+                const { code, start } = diag;
                 const info = errorCodeFixIdMap[code];
-                if (!info || info[1] !== context.fixId || isSourceFileJS(file)) {
+                if (!info || info.fixId !== context.fixId) {
                     return;
                 }
 
@@ -87,6 +102,10 @@ namespace ts.codefix {
 
     function doAddOverrideModifierChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number) {
         const classElement = findContainerClassElementLike(sourceFile, pos);
+        if (isSourceFileJS(sourceFile)) {
+            changeTracker.addJSDocTags(sourceFile, classElement, [ factory.createJSDocOverrideTag(/*tagName*/ undefined)]);
+            return;
+        }
         const modifiers = classElement.modifiers || emptyArray;
         const staticModifier = find(modifiers, isStaticModifier);
         const abstractModifier = find(modifiers, isAbstractModifier);
@@ -101,6 +120,10 @@ namespace ts.codefix {
 
     function doRemoveOverrideModifierChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number) {
         const classElement = findContainerClassElementLike(sourceFile, pos);
+        if (isSourceFileJS(sourceFile)) {
+            changeTracker.filterJSDocTags(sourceFile, classElement, not(isJSDocOverrideTag));
+            return;
+        }
         const overrideModifier = classElement.modifiers && find(classElement.modifiers, modifier => modifier.kind === SyntaxKind.OverrideKeyword);
         Debug.assertIsDefined(overrideModifier);
 

src/services/codefixes/inferFromUsage.ts

@@ -129,7 +129,7 @@ namespace ts.codefix {
                     if (typeNode) {
                         // Note that the codefix will never fire with an existing `@type` tag, so there is no need to merge tags
                         const typeTag = factory.createJSDocTypeTag(/*tagName*/ undefined, factory.createJSDocTypeExpression(typeNode), /*comment*/ undefined);
-                        addJSDocTags(changes, sourceFile, cast(parent.parent.parent, isExpressionStatement), [typeTag]);
+                        changes.addJSDocTags(sourceFile, cast(parent.parent.parent, isExpressionStatement), [typeTag]);
                     }
                     importAdder.writeFixes(changes);
                     return parent;
@@ -269,7 +269,7 @@ namespace ts.codefix {
     }
 
     function annotateJSDocThis(changes: textChanges.ChangeTracker, sourceFile: SourceFile, containingFunction: SignatureDeclaration, typeNode: TypeNode) {
-        addJSDocTags(changes, sourceFile, containingFunction, [
+        changes.addJSDocTags(sourceFile, containingFunction, [
             factory.createJSDocThisTag(/*tagName*/ undefined, factory.createJSDocTypeExpression(typeNode)),
         ]);
     }
@@ -309,7 +309,7 @@ namespace ts.codefix {
                 }
                 const typeExpression = factory.createJSDocTypeExpression(typeNode);
                 const typeTag = isGetAccessorDeclaration(declaration) ? factory.createJSDocReturnTag(/*tagName*/ undefined, typeExpression, /*comment*/ undefined) : factory.createJSDocTypeTag(/*tagName*/ undefined, typeExpression, /*comment*/ undefined);
-                addJSDocTags(changes, sourceFile, parent, [typeTag]);
+                changes.addJSDocTags(sourceFile, parent, [typeTag]);
             }
             else if (!tryReplaceImportTypeNodeWithAutoImport(typeNode, declaration, sourceFile, changes, importAdder, getEmitScriptTarget(program.getCompilerOptions()))) {
                 changes.tryInsertTypeAnnotation(sourceFile, declaration, typeNode);
@@ -376,46 +376,7 @@ namespace ts.codefix {
         else {
             const paramTags = map(inferences, ({ name, typeNode, isOptional }) =>
                 factory.createJSDocParameterTag(/*tagName*/ undefined, name, /*isBracketed*/ !!isOptional, factory.createJSDocTypeExpression(typeNode), /* isNameFirst */ false, /*comment*/ undefined));
-            addJSDocTags(changes, sourceFile, signature, paramTags);
-        }
-    }
-
-    export function addJSDocTags(changes: textChanges.ChangeTracker, sourceFile: SourceFile, parent: HasJSDoc, newTags: readonly JSDocTag[]): void {
-        const comments = flatMap(parent.jsDoc, j => typeof j.comment === "string" ? factory.createJSDocText(j.comment) : j.comment) as JSDocComment[];
-        const oldTags = flatMapToMutable(parent.jsDoc, j => j.tags);
-        const unmergedNewTags = newTags.filter(newTag => !oldTags || !oldTags.some((tag, i) => {
-            const merged = tryMergeJsdocTags(tag, newTag);
-            if (merged) oldTags[i] = merged;
-            return !!merged;
-        }));
-        const tag = factory.createJSDocComment(factory.createNodeArray(intersperse(comments, factory.createJSDocText("\n"))), factory.createNodeArray([...(oldTags || emptyArray), ...unmergedNewTags]));
-        const jsDocNode = parent.kind === SyntaxKind.ArrowFunction ? getJsDocNodeForArrowFunction(parent) : parent;
-        jsDocNode.jsDoc = parent.jsDoc;
-        jsDocNode.jsDocCache = parent.jsDocCache;
-        changes.insertJsdocCommentBefore(sourceFile, jsDocNode, tag);
-    }
-
-    function getJsDocNodeForArrowFunction(signature: ArrowFunction): HasJSDoc {
-        if (signature.parent.kind === SyntaxKind.PropertyDeclaration) {
-            return signature.parent as HasJSDoc;
-        }
-        return signature.parent.parent as HasJSDoc;
-    }
-
-    function tryMergeJsdocTags(oldTag: JSDocTag, newTag: JSDocTag): JSDocTag | undefined {
-        if (oldTag.kind !== newTag.kind) {
-            return undefined;
-        }
-        switch (oldTag.kind) {
-            case SyntaxKind.JSDocParameterTag: {
-                const oldParam = oldTag as JSDocParameterTag;
-                const newParam = newTag as JSDocParameterTag;
-                return isIdentifier(oldParam.name) && isIdentifier(newParam.name) && oldParam.name.escapedText === newParam.name.escapedText
-                    ? factory.createJSDocParameterTag(/*tagName*/ undefined, newParam.name, /*isBracketed*/ false, newParam.typeExpression, newParam.isNameFirst, oldParam.comment)
-                    : undefined;
-            }
-            case SyntaxKind.JSDocReturnTag:
-                return factory.createJSDocReturnTag(/*tagName*/ undefined, (newTag as JSDocReturnTag).typeExpression, oldTag.comment);
+            changes.addJSDocTags(sourceFile, signature, paramTags);
         }
     }
 

src/services/textChanges.ts

@@ -494,6 +494,27 @@ namespace ts.textChanges {
             this.insertNodeAt(sourceFile, fnStart, tag, { preserveLeadingWhitespace: false, suffix: this.newLineCharacter + indent });
         }
 
+        public addJSDocTags(sourceFile: SourceFile, parent: HasJSDoc, newTags: readonly JSDocTag[]): void {
+            const comments = mapDefined(parent.jsDoc, j => j.comment);
+            const oldTags = flatMapToMutable(parent.jsDoc, j => j.tags);
+            const unmergedNewTags = newTags.filter(newTag => !oldTags || !oldTags.some((tag, i) => {
+                const merged = tryMergeJsdocTags(tag, newTag);
+                if (merged) oldTags[i] = merged;
+                return !!merged;
+            }));
+            const tag = factory.createJSDocComment(comments.join("\n"), factory.createNodeArray([...(oldTags || emptyArray), ...unmergedNewTags]));
+            const jsDocNode = getJsDocNode(parent);
+            this.insertJsdocCommentBefore(sourceFile, jsDocNode, tag);
+        }
+
+        public filterJSDocTags(sourceFile: SourceFile, parent: HasJSDoc, predicate: (tag: JSDocTag) => boolean): void {
+            const comments = mapDefined(parent.jsDoc, j => j.comment);
+            const oldTags = filter(flatMapToMutable(parent.jsDoc, j => j.tags), predicate);
+            const tag = factory.createJSDocComment(comments.join("\n"), factory.createNodeArray([...(oldTags || emptyArray)]));
+            const jsDocNode = getJsDocNode(parent);
+            this.insertJsdocCommentBefore(sourceFile, jsDocNode, tag);
+        }
+
         public replaceRangeWithText(sourceFile: SourceFile, range: TextRange, text: string): void {
             this.changes.push({ kind: ChangeKind.Text, sourceFile, range, text });
         }
@@ -769,7 +790,6 @@ namespace ts.textChanges {
         public insertNodeInListAfter(sourceFile: SourceFile, after: Node, newNode: Node, containingList = formatting.SmartIndenter.getContainingList(after, sourceFile)): void {
             if (!containingList) {
                 Debug.fail("node is not a list element");
-                return;
             }
             const index = indexOfNode(containingList, after);
             if (index < 0) {
@@ -920,6 +940,34 @@ namespace ts.textChanges {
         }
     }
 
+    function getJsDocNode(parent: HasJSDoc): HasJSDoc {
+        const jsDocNode = parent.kind === SyntaxKind.ArrowFunction ?
+            parent.parent.kind === SyntaxKind.PropertyDeclaration ?
+                parent.parent as HasJSDoc :
+                parent.parent.parent as HasJSDoc :
+            parent;
+        jsDocNode.jsDoc = parent.jsDoc;
+        jsDocNode.jsDocCache = parent.jsDocCache;
+        return jsDocNode;
+    }
+
+    function tryMergeJsdocTags(oldTag: JSDocTag, newTag: JSDocTag): JSDocTag | undefined {
+        if (oldTag.kind !== newTag.kind) {
+            return undefined;
+        }
+        switch (oldTag.kind) {
+            case SyntaxKind.JSDocParameterTag: {
+                const oldParam = oldTag as JSDocParameterTag;
+                const newParam = newTag as JSDocParameterTag;
+                return isIdentifier(oldParam.name) && isIdentifier(newParam.name) && oldParam.name.escapedText === newParam.name.escapedText
+                    ? factory.createJSDocParameterTag(/*tagName*/ undefined, newParam.name, /*isBracketed*/ false, newParam.typeExpression, newParam.isNameFirst, oldParam.comment)
+                    : undefined;
+            }
+            case SyntaxKind.JSDocReturnTag:
+                return factory.createJSDocReturnTag(/*tagName*/ undefined, (newTag as JSDocReturnTag).typeExpression, oldTag.comment);
+        }
+    }
+
     // find first non-whitespace position in the leading trivia of the node
     function startPositionToDeleteNodeInList(sourceFile: SourceFile, node: Node): number {
         return skipTrivia(sourceFile.text, getAdjustedStartPosition(sourceFile, node, { leadingTriviaOption: LeadingTriviaOption.IncludeAll }), /*stopAfterLineBreak*/ false, /*stopAtComments*/ true);

tests/cases/fourslash/codeFixOverrideModifier_js1.ts

@@ -1,5 +1,4 @@
 /// <reference path='fourslash.ts' />
-
 // @allowJs: true
 // @checkJs: true
 // @noEmit: true
@@ -9,14 +8,22 @@
 ////     foo (v) {}
 //// }
 //// class D extends B {
+////     /** @public */
 ////     foo (v) {}
-////     /**@override*/
-////     bar (v) {}
-//// }
-//// class C {
-////     /**@override*/
-////     foo () {}
 //// }
 
-verify.not.codeFixAvailable("fixAddOverrideModifier");
-verify.not.codeFixAvailable("fixRemoveOverrideModifier");
+verify.codeFix({
+    description: "Add 'override' modifier",
+    index: 0,
+    newFileContent:
+`class B {
+    foo (v) {}
+}
+class D extends B {
+    /**
+     * @public
+     * @override
+     */
+    foo (v) {}
+}`,
+})

tests/cases/fourslash/codeFixOverrideModifier_js2.ts

@@ -0,0 +1,28 @@
+/// <reference path='fourslash.ts' />
+
+// @allowJs: true
+// @checkJs: true
+// @noEmit: true
+// @noImplicitOverride: true
+// @filename: a.js
+//// class B { }
+//// class D extends B {
+////     /**
+////      * @public
+////      * @override
+////      */
+////     foo (v) {}
+//// }
+
+verify.codeFix({
+    description: "Remove 'override' modifier",
+    index: 0,
+    newFileContent:
+`class B { }
+class D extends B {
+    /**
+     * @public
+     */
+    foo (v) {}
+}`,
+})