diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index d4eff1e0ace5e..5a2fc96f58b68 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -1604,6 +1604,7 @@ namespace ts { return emitJSDocSignature(node as JSDocSignature); case SyntaxKind.JSDocTag: case SyntaxKind.JSDocClassTag: + case SyntaxKind.JSDocOverrideTag: return emitJSDocSimpleTag(node as JSDocTag); case SyntaxKind.JSDocAugmentsTag: case SyntaxKind.JSDocImplementsTag: @@ -1616,7 +1617,6 @@ namespace ts { case SyntaxKind.JSDocPrivateTag: case SyntaxKind.JSDocProtectedTag: case SyntaxKind.JSDocReadonlyTag: - case SyntaxKind.JSDocOverrideTag: return; case SyntaxKind.JSDocCallbackTag: return emitJSDocCallbackTag(node as JSDocCallbackTag); diff --git a/src/services/codefixes/fixOverrideModifier.ts b/src/services/codefixes/fixOverrideModifier.ts index d267dececcf70..4b554d1c10dd8 100644 --- a/src/services/codefixes/fixOverrideModifier.ts +++ b/src/services/codefixes/fixOverrideModifier.ts @@ -17,37 +17,81 @@ namespace ts.codefix { Diagnostics.This_member_cannot_have_an_override_modifier_because_its_containing_class_0_does_not_extend_another_class.code, Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0.code, Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0.code, - Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code + Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code, + Diagnostics.This_member_must_have_a_JSDoc_comment_with_an_override_tag_because_it_overrides_a_member_in_the_base_class_0.code, + Diagnostics.This_member_cannot_have_a_JSDoc_comment_with_an_override_tag_because_its_containing_class_0_does_not_extend_another_class.code, + Diagnostics.This_parameter_property_must_have_a_JSDoc_comment_with_an_override_tag_because_it_overrides_a_member_in_the_base_class_0.code, + Diagnostics.This_member_cannot_have_a_JSDoc_comment_with_an_override_tag_because_it_is_not_declared_in_the_base_class_0.code, ]; - 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_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 = { + // case #1: + [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_must_have_a_JSDoc_comment_with_an_override_tag_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 + }, + // case #2: + [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_member_cannot_have_a_JSDoc_comment_with_an_override_tag_because_its_containing_class_0_does_not_extend_another_class.code]: { + descriptions: Diagnostics.Remove_override_modifier, + fixId: fixRemoveOverrideId, + fixAllDescriptions: Diagnostics.Remove_override_modifier + }, + // case #3: + [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_parameter_property_must_have_a_JSDoc_comment_with_an_override_tag_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, + }, + // case #4: + [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, + }, + // case #5: + [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, + }, + [Diagnostics.This_member_cannot_have_a_JSDoc_comment_with_an_override_tag_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 +101,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; } @@ -74,11 +118,15 @@ namespace ts.codefix { pos: number) { switch (errorCode) { case Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0.code: + case Diagnostics.This_member_must_have_a_JSDoc_comment_with_an_override_tag_because_it_overrides_a_member_in_the_base_class_0.code: case Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0.code: case Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code: + case Diagnostics.This_parameter_property_must_have_a_JSDoc_comment_with_an_override_tag_because_it_overrides_a_member_in_the_base_class_0.code: return doAddOverrideModifierChange(changeTracker, context.sourceFile, pos); case Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0.code: + case Diagnostics.This_member_cannot_have_a_JSDoc_comment_with_an_override_tag_because_it_is_not_declared_in_the_base_class_0.code: case Diagnostics.This_member_cannot_have_an_override_modifier_because_its_containing_class_0_does_not_extend_another_class.code: + case Diagnostics.This_member_cannot_have_a_JSDoc_comment_with_an_override_tag_because_its_containing_class_0_does_not_extend_another_class.code: return doRemoveOverrideModifierChange(changeTracker, context.sourceFile, pos); default: Debug.fail("Unexpected error code: " + errorCode); @@ -87,6 +135,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(factory.createIdentifier("override"))]); + return; + } const modifiers = classElement.modifiers || emptyArray; const staticModifier = find(modifiers, isStaticModifier); const abstractModifier = find(modifiers, isAbstractModifier); @@ -101,6 +153,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); diff --git a/src/services/codefixes/inferFromUsage.ts b/src/services/codefixes/inferFromUsage.ts index e680d4307ec22..46926e2252b86 100644 --- a/src/services/codefixes/inferFromUsage.ts +++ b/src/services/codefixes/inferFromUsage.ts @@ -131,7 +131,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; @@ -271,7 +271,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)), ]); } @@ -311,7 +311,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); @@ -378,46 +378,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); } } diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index d204bb0bc07ac..44ac36ab2046d 100644 --- a/src/services/textChanges.ts +++ b/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 = 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.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, ...unmergedNewTags])); + const host = updateJSDocHost(parent); + this.insertJsdocCommentBefore(sourceFile, host, tag); + } + + public filterJSDocTags(sourceFile: SourceFile, parent: HasJSDoc, predicate: (tag: JSDocTag) => boolean): 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 tag = factory.createJSDocComment(factory.createNodeArray(intersperse(comments, factory.createJSDocText("\n"))), factory.createNodeArray([...(filter(oldTags, predicate) || emptyArray)])); + const host = updateJSDocHost(parent); + this.insertJsdocCommentBefore(sourceFile, host, tag); + } + public replaceRangeWithText(sourceFile: SourceFile, range: TextRange, text: string): void { this.changes.push({ kind: ChangeKind.Text, sourceFile, range, text }); } @@ -920,6 +941,35 @@ namespace ts.textChanges { } } + function updateJSDocHost(parent: HasJSDoc): HasJSDoc { + if (parent.kind !== SyntaxKind.ArrowFunction) { + return parent; + } + const jsDocNode = parent.parent.kind === SyntaxKind.PropertyDeclaration ? + parent.parent as HasJSDoc : + parent.parent.parent as HasJSDoc; + 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); diff --git a/tests/cases/fourslash/codeFixOverrideModifier_js1.ts b/tests/cases/fourslash/codeFixOverrideModifier_js1.ts index dd5675266572a..8f3584021cb92 100644 --- a/tests/cases/fourslash/codeFixOverrideModifier_js1.ts +++ b/tests/cases/fourslash/codeFixOverrideModifier_js1.ts @@ -1,5 +1,4 @@ /// - // @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 { + /** + * + * @override + */ + foo (v) {} +}`, +}) \ No newline at end of file diff --git a/tests/cases/fourslash/codeFixOverrideModifier_js2.ts b/tests/cases/fourslash/codeFixOverrideModifier_js2.ts new file mode 100644 index 0000000000000..b2091a65498e5 --- /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 { + /** + * + */ + foo (v) {} +}`, +}) \ No newline at end of file