diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index ebfb7ff3e49a6..d54fa54317b6f 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -1560,6 +1560,10 @@ namespace ts { return emitJSDocTypeLiteral(node as JSDocTypeLiteral); case SyntaxKind.JSDocClassTag: case SyntaxKind.JSDocTag: + case SyntaxKind.JSDocOverrideTag: + case SyntaxKind.JSDocPublicTag: + case SyntaxKind.JSDocPrivateTag: + case SyntaxKind.JSDocProtectedTag: return emitJSDocSimpleTag(node as JSDocTag); case SyntaxKind.JSDocSeeTag: return emitJSDocSeeTag(node as JSDocSeeTag); diff --git a/src/services/codefixes/fixOverrideModifier.ts b/src/services/codefixes/fixOverrideModifier.ts index 498be1466e4af..7231f1152d7bd 100644 --- a/src/services/codefixes/fixOverrideModifier.ts +++ b/src/services/codefixes/fixOverrideModifier.ts @@ -20,23 +20,24 @@ namespace ts.codefix { Diagnostics.This_parameter_property_must_be_rewritten_as_a_property_declaration_with_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code ]; - const errorCodeFixIdMap: Record = { + const errorCodeFixIdMap: Record = { [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_override_modifier, + Diagnostics.Add_override_modifier, fixAddOverrideId, Diagnostics.Add_all_override_modifier, true ], [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_override_modifier + Diagnostics.Remove_override_modifier, fixRemoveOverrideId, Diagnostics.Remove_all_override_modifier, true ], [Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_implemented_an_abstract_method_that_declared_in_the_base_class_0.code]: [ - Diagnostics.Remove_override_modifier, fixRemoveOverrideId, Diagnostics.Remove_all_override_modifier + Diagnostics.Remove_override_modifier, fixRemoveOverrideId, Diagnostics.Remove_all_override_modifier, true ], [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_override_modifier + Diagnostics.Remove_override_modifier, fixRemoveOverrideId, Diagnostics.Remove_all_override_modifier, true ], [Diagnostics.This_parameter_property_must_be_rewritten_as_a_property_declaration_with_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code]: [ Diagnostics.Convert_to_property_declaration_and_add_override_modifier, fixConvertToPropertyDeclarationId, - Diagnostics.Convert_all_to_property_declaration_and_add_override_modifier + Diagnostics.Convert_all_to_property_declaration_and_add_override_modifier, + false ] }; @@ -48,8 +49,8 @@ namespace ts.codefix { const info = errorCodeFixIdMap[errorCode]; if (!info) return emptyArray; - const [ descriptions, fixId, fixAllDescriptions ] = info; - if (isSourceFileJS(sourceFile)) return emptyArray; + const [ descriptions, fixId, fixAllDescriptions, allowInjs ] = info; + if (!allowInjs && isSourceFileJS(sourceFile)) return emptyArray; const changes = textChanges.ChangeTracker.with(context, changes => dispatchChanges(changes, context, errorCode, span.start)); return [ @@ -61,7 +62,7 @@ namespace ts.codefix { codeFixAll(context, errorCodes, (changes, diag) => { const { code, start, file } = diag; const info = errorCodeFixIdMap[code]; - if (!info || info[1] !== context.fixId || isSourceFileJS(file)) { + if (!info || info[1] !== context.fixId || !info[3] && isSourceFileJS(file)) { return; } @@ -90,15 +91,24 @@ namespace ts.codefix { function doAddOverrideModifierChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number) { const classElement = findContainerClassElement(sourceFile, pos); - changeTracker.insertModifierBefore(sourceFile, SyntaxKind.OverrideKeyword, classElement); + if (isSourceFileJS(sourceFile)) { + changeTracker.addJSDocTags(sourceFile, classElement, [ factory.createJSDocOverrideTag(/*tagName*/ undefined) ]); + } + else { + changeTracker.insertModifierBefore(sourceFile, SyntaxKind.OverrideKeyword, classElement); + } } function doRemoveOverrideModifierChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number) { const classElement = findContainerClassElement(sourceFile, pos); - const overrideModifier = classElement.modifiers && find(classElement.modifiers, modifier => modifier.kind === SyntaxKind.OverrideKeyword); - Debug.assertIsDefined(overrideModifier); - - changeTracker.deleteModifier(sourceFile, overrideModifier); + if (isSourceFileJS(sourceFile)) { + changeTracker.filterJSDocTags(sourceFile, classElement, not(isJSDocOverrideTag)); + } + else { + const overrideModifier = classElement.modifiers && find(classElement.modifiers, modifier => modifier.kind === SyntaxKind.OverrideKeyword); + Debug.assertIsDefined(overrideModifier); + changeTracker.deleteModifier(sourceFile, overrideModifier); + } } function doConvertToPropertyDeclaration(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number) { diff --git a/src/services/codefixes/inferFromUsage.ts b/src/services/codefixes/inferFromUsage.ts index efad417248a64..00c846418dfe4 100644 --- a/src/services/codefixes/inferFromUsage.ts +++ b/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*/ ""); - 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)), ]); } @@ -308,7 +308,7 @@ namespace ts.codefix { } const typeExpression = factory.createJSDocTypeExpression(typeNode); const typeTag = isGetAccessorDeclaration(declaration) ? factory.createJSDocReturnTag(/*tagName*/ undefined, typeExpression, "") : factory.createJSDocTypeTag(/*tagName*/ undefined, typeExpression, ""); - 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); @@ -347,46 +347,7 @@ namespace ts.codefix { setEmitFlags(name, EmitFlags.NoComments | EmitFlags.NoNestedComments); return typeNode && factory.createJSDocParameterTag(/*tagName*/ undefined, name, /*isBracketed*/ !!inference.isOptional, factory.createJSDocTypeExpression(typeNode), /* isNameFirst */ false, ""); }); - addJSDocTags(changes, sourceFile, signature, paramTags); - } - - export function addJSDocTags(changes: textChanges.ChangeTracker, 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 = 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; - } - return signature.parent.parent; - } - - 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); } function getReferences(token: PropertyName | Token, program: Program, cancellationToken: CancellationToken): readonly Identifier[] { diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 64442de4d3693..0fcad5fdad19c 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -436,6 +436,56 @@ 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 = this.tryMergeJsdocTags(tag, newTag); + if (merged) oldTags[i] = merged; + return !!merged; + })); + const tag = factory.createJSDocComment(comments.join("\n"), factory.createNodeArray([...(oldTags || emptyArray), ...unmergedNewTags])); + const jsDocNode = parent.kind === SyntaxKind.ArrowFunction ? this.getJsDocNodeForArrowFunction(parent) : parent; + jsDocNode.jsDoc = parent.jsDoc; + jsDocNode.jsDocCache = parent.jsDocCache; + this.insertJsdocCommentBefore(sourceFile, jsDocNode, tag); + } + + public filterJSDocTags(sourceFile: SourceFile, parent: HasJSDoc, cb: (tag: JSDocTag) => boolean): void { + const comments = mapDefined(parent.jsDoc, j => j.comment); + const oldTags = filter(flatMapToMutable(parent.jsDoc, j => j.tags), cb); + const tag = factory.createJSDocComment(comments.join("\n"), factory.createNodeArray([...(oldTags || emptyArray)])); + const jsDocNode = parent.kind === SyntaxKind.ArrowFunction ? this.getJsDocNodeForArrowFunction(parent) : parent; + jsDocNode.jsDoc = parent.jsDoc; + jsDocNode.jsDocCache = parent.jsDocCache; + this.insertJsdocCommentBefore(sourceFile, jsDocNode, tag); + } + + public getJsDocNodeForArrowFunction(signature: ArrowFunction): HasJSDoc { + if (signature.parent.kind === SyntaxKind.PropertyDeclaration) { + return signature.parent; + } + return signature.parent.parent; + } + + public 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); + } + } + public replaceRangeWithText(sourceFile: SourceFile, range: TextRange, text: string): void { this.changes.push({ kind: ChangeKind.Text, sourceFile, range, text }); } diff --git a/tests/cases/fourslash/codeFixOverrideModifier_js1.ts b/tests/cases/fourslash/codeFixOverrideModifier_js1.ts index bc26610ae6d14..c995b8f8d2a64 100644 --- a/tests/cases/fourslash/codeFixOverrideModifier_js1.ts +++ b/tests/cases/fourslash/codeFixOverrideModifier_js1.ts @@ -9,14 +9,22 @@ //// foo (v) {} //// } //// class D extends B { +//// /** @public */ //// foo (v) {} -//// /**@override*/ -//// bar (v) {} -//// } -//// class C { -//// /**@override*/ -//// foo () {} //// } -verify.not.codeFixAvailable("Add 'override' modifier"); -verify.not.codeFixAvailable("Remove 'override' modifier"); +verify.codeFix({ + description: "Add 'override' modifier", + index: 0, + newFileContent: +`class B { + foo (v) {} +} +class D extends B { + /** + * @public + * @override + */ + foo (v) {} +}`, +}) diff --git a/tests/cases/fourslash/codeFixOverrideModifier_js2.ts b/tests/cases/fourslash/codeFixOverrideModifier_js2.ts new file mode 100644 index 0000000000000..a66af8ecf1885 --- /dev/null +++ b/tests/cases/fourslash/codeFixOverrideModifier_js2.ts @@ -0,0 +1,28 @@ +/// + +// @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) {} +}`, +})