From 9b7ca24ebc2e12d5408983a65d83b544948a7690 Mon Sep 17 00:00:00 2001 From: stkevintan Date: Fri, 1 Oct 2021 16:19:05 +0800 Subject: [PATCH 1/7] feat: code fix for override in js files Co-Authored-By: Wenlu Wang --- src/compiler/emitter.ts | 8 +-- src/compiler/types.ts | 2 +- src/services/codefixes/fixOverrideModifier.ts | 23 ++++++--- src/services/codefixes/inferFromUsage.ts | 47 ++--------------- src/services/textChanges.ts | 50 ++++++++++++++++++- .../fourslash/codeFixOverrideModifier_js1.ts | 25 ++++++---- .../fourslash/codeFixOverrideModifier_js2.ts | 28 +++++++++++ 7 files changed, 117 insertions(+), 66 deletions(-) create mode 100644 tests/cases/fourslash/codeFixOverrideModifier_js2.ts diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 5667e792b4606..556de9345ebc0 100644 --- a/src/compiler/emitter.ts +++ b/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); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 6194f41a20d22..2eac7f8061464 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -7401,7 +7401,7 @@ namespace ts { updateJSDocUnknownTag(node: JSDocUnknownTag, tagName: Identifier, comment: string | NodeArray | undefined): JSDocUnknownTag; createJSDocDeprecatedTag(tagName: Identifier, comment?: string | NodeArray): JSDocDeprecatedTag; updateJSDocDeprecatedTag(node: JSDocDeprecatedTag, tagName: Identifier, comment?: string | NodeArray): JSDocDeprecatedTag; - createJSDocOverrideTag(tagName: Identifier, comment?: string | NodeArray): JSDocOverrideTag; + createJSDocOverrideTag(tagName: Identifier | undefined, comment?: string | NodeArray): JSDocOverrideTag; updateJSDocOverrideTag(node: JSDocOverrideTag, tagName: Identifier, comment?: string | NodeArray): JSDocOverrideTag; createJSDocText(text: string): JSDocText; updateJSDocText(node: JSDocText, text: string): JSDocText; diff --git a/src/services/codefixes/fixOverrideModifier.ts b/src/services/codefixes/fixOverrideModifier.ts index d267dececcf70..dca4678e948e0 100644 --- a/src/services/codefixes/fixOverrideModifier.ts +++ b/src/services/codefixes/fixOverrideModifier.ts @@ -20,34 +20,33 @@ 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 = { + 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.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.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 + Diagnostics.Remove_override_modifier, fixRemoveOverrideId, 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 changes = textChanges.ChangeTracker.with(context, changes => dispatchChanges(changes, context, errorCode, span.start)); return [ @@ -57,9 +56,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[1] !== context.fixId) { return; } @@ -87,6 +86,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 +104,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 94456fec7b172..e982ec395d0e0 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*/ 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); } } diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 1c82d66afdb27..5173d64b336fe 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -494,6 +494,55 @@ 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, 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 = 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 as HasJSDoc; + } + return signature.parent.parent as HasJSDoc; + } + + 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 }); } @@ -769,7 +818,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) { diff --git a/tests/cases/fourslash/codeFixOverrideModifier_js1.ts b/tests/cases/fourslash/codeFixOverrideModifier_js1.ts index dd5675266572a..d5bcdd818df2b 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 { + /** + * @public + * @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..37b6356d7aa9d --- /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) {} +}`, +}) \ No newline at end of file From 65e4235303e293ee159269531267950bb3d98ee1 Mon Sep 17 00:00:00 2001 From: stkevintan Date: Fri, 1 Oct 2021 17:50:50 +0800 Subject: [PATCH 2/7] fix comments Co-Authored-By: Wenlu Wang --- src/services/codefixes/fixOverrideModifier.ts | 52 ++++++++++++------- src/services/textChanges.ts | 44 ++++++++++------ 2 files changed, 62 insertions(+), 34 deletions(-) diff --git a/src/services/codefixes/fixOverrideModifier.ts b/src/services/codefixes/fixOverrideModifier.ts index dca4678e948e0..ededecc05cd3b 100644 --- a/src/services/codefixes/fixOverrideModifier.ts +++ b/src/services/codefixes/fixOverrideModifier.ts @@ -20,22 +20,38 @@ 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 = { - [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 = { + [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({ @@ -46,7 +62,7 @@ namespace ts.codefix { const info = errorCodeFixIdMap[errorCode]; if (!info) return emptyArray; - const [ descriptions, fixId, fixAllDescriptions ] = info; + const { descriptions, fixId, fixAllDescriptions } = info; const changes = textChanges.ChangeTracker.with(context, changes => dispatchChanges(changes, context, errorCode, span.start)); return [ @@ -58,7 +74,7 @@ namespace ts.codefix { codeFixAll(context, errorCodes, (changes, diag) => { const { code, start } = diag; const info = errorCodeFixIdMap[code]; - if (!info || info[1] !== context.fixId) { + if (!info || info.fixId !== context.fixId) { return; } diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 5173d64b336fe..eac57425beabe 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -497,35 +497,47 @@ namespace ts.textChanges { 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 unmergedNewTags = newTags.filter(newTag => !oldTags || !oldTags.some((oldTag, i) => { + if (oldTag.kind !== newTag.kind) { + return false; + } + switch (oldTag.kind) { + case SyntaxKind.JSDocParameterTag: { + const oldParam = oldTag as JSDocParameterTag; + const newParam = newTag as JSDocParameterTag; + if (isIdentifier(oldParam.name) && isIdentifier(newParam.name) && oldParam.name.escapedText === newParam.name.escapedText) { + oldTags[i] = factory.createJSDocParameterTag(/*tagName*/ undefined, newParam.name, /*isBracketed*/ false, newParam.typeExpression, newParam.isNameFirst, oldParam.comment); + return true; + } + return false; + } + case SyntaxKind.JSDocReturnTag: + oldTags[i] = factory.createJSDocReturnTag(/*tagName*/ undefined, (newTag as JSDocReturnTag).typeExpression, oldTag.comment); + return true; + } })); 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); + this.setJSDocTags(sourceFile, parent, 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 = parent.kind === SyntaxKind.ArrowFunction ? this.getJsDocNodeForArrowFunction(parent) : parent; + this.setJSDocTags(sourceFile, parent, tag); + } + + public setJSDocTags(sourceFile: SourceFile, parent: HasJSDoc, tag: JSDoc): void { + 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; this.insertJsdocCommentBefore(sourceFile, jsDocNode, tag); } - public getJsDocNodeForArrowFunction(signature: ArrowFunction): HasJSDoc { - if (signature.parent.kind === SyntaxKind.PropertyDeclaration) { - return signature.parent as HasJSDoc; - } - return signature.parent.parent as HasJSDoc; - } - public tryMergeJsdocTags(oldTag: JSDocTag, newTag: JSDocTag): JSDocTag | undefined { if (oldTag.kind !== newTag.kind) { return undefined; From dd96d0612c2c65444cf0ae870f683d650344dfbd Mon Sep 17 00:00:00 2001 From: stkevintan Date: Fri, 1 Oct 2021 18:00:00 +0800 Subject: [PATCH 3/7] remove tryMergeJsdocTags --- src/services/textChanges.ts | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index eac57425beabe..2d7f80ef143ea 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -538,23 +538,6 @@ namespace ts.textChanges { this.insertJsdocCommentBefore(sourceFile, jsDocNode, tag); } - 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 }); } From ff9b66a28ee0283196f36e86f922262ab45cafba Mon Sep 17 00:00:00 2001 From: stkevintan Date: Mon, 4 Oct 2021 20:28:22 +0800 Subject: [PATCH 4/7] fix: bring the two methods back as functions --- src/services/textChanges.ts | 65 ++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 2d7f80ef143ea..59de23f8b2dbe 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -497,44 +497,21 @@ namespace ts.textChanges { 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((oldTag, i) => { - if (oldTag.kind !== newTag.kind) { - return false; - } - switch (oldTag.kind) { - case SyntaxKind.JSDocParameterTag: { - const oldParam = oldTag as JSDocParameterTag; - const newParam = newTag as JSDocParameterTag; - if (isIdentifier(oldParam.name) && isIdentifier(newParam.name) && oldParam.name.escapedText === newParam.name.escapedText) { - oldTags[i] = factory.createJSDocParameterTag(/*tagName*/ undefined, newParam.name, /*isBracketed*/ false, newParam.typeExpression, newParam.isNameFirst, oldParam.comment); - return true; - } - return false; - } - case SyntaxKind.JSDocReturnTag: - oldTags[i] = factory.createJSDocReturnTag(/*tagName*/ undefined, (newTag as JSDocReturnTag).typeExpression, oldTag.comment); - return true; - } + 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])); - this.setJSDocTags(sourceFile, parent, tag); + 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)])); - this.setJSDocTags(sourceFile, parent, tag); - } - - public setJSDocTags(sourceFile: SourceFile, parent: HasJSDoc, tag: JSDoc): void { - 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; + const jsDocNode = getJsDocNode(parent); this.insertJsdocCommentBefore(sourceFile, jsDocNode, tag); } @@ -963,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); From 182e205b3ff7317cfa1cd5e8bc2e99a8f2578286 Mon Sep 17 00:00:00 2001 From: stkevintan Date: Thu, 14 Oct 2021 16:38:07 +0800 Subject: [PATCH 5/7] revert emitter changes --- src/compiler/emitter.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 556de9345ebc0..5667e792b4606 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -1613,10 +1613,6 @@ 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: @@ -1625,7 +1621,11 @@ 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); From 6ff5846a177b566fd01a71854b06426fee2c00f5 Mon Sep 17 00:00:00 2001 From: stkevintan Date: Tue, 26 Oct 2021 16:28:58 +0800 Subject: [PATCH 6/7] fix comments --- src/compiler/types.ts | 2 +- src/services/codefixes/fixOverrideModifier.ts | 2 +- src/services/textChanges.ts | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 2eac7f8061464..6194f41a20d22 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -7401,7 +7401,7 @@ namespace ts { updateJSDocUnknownTag(node: JSDocUnknownTag, tagName: Identifier, comment: string | NodeArray | undefined): JSDocUnknownTag; createJSDocDeprecatedTag(tagName: Identifier, comment?: string | NodeArray): JSDocDeprecatedTag; updateJSDocDeprecatedTag(node: JSDocDeprecatedTag, tagName: Identifier, comment?: string | NodeArray): JSDocDeprecatedTag; - createJSDocOverrideTag(tagName: Identifier | undefined, comment?: string | NodeArray): JSDocOverrideTag; + createJSDocOverrideTag(tagName: Identifier, comment?: string | NodeArray): JSDocOverrideTag; updateJSDocOverrideTag(node: JSDocOverrideTag, tagName: Identifier, comment?: string | NodeArray): JSDocOverrideTag; createJSDocText(text: string): JSDocText; updateJSDocText(node: JSDocText, text: string): JSDocText; diff --git a/src/services/codefixes/fixOverrideModifier.ts b/src/services/codefixes/fixOverrideModifier.ts index ededecc05cd3b..851222b5aaec5 100644 --- a/src/services/codefixes/fixOverrideModifier.ts +++ b/src/services/codefixes/fixOverrideModifier.ts @@ -103,7 +103,7 @@ 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)]); + changeTracker.addJSDocTags(sourceFile, classElement, [factory.createJSDocOverrideTag(factory.createIdentifier("override"))]); return; } const modifiers = classElement.modifiers || emptyArray; diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 59de23f8b2dbe..bdb90f65dee92 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -503,16 +503,16 @@ namespace ts.textChanges { return !!merged; })); const tag = factory.createJSDocComment(comments.join("\n"), factory.createNodeArray([...(oldTags || emptyArray), ...unmergedNewTags])); - const jsDocNode = getJsDocNode(parent); - this.insertJsdocCommentBefore(sourceFile, jsDocNode, tag); + const host = updateJSDocHost(parent); + this.insertJsdocCommentBefore(sourceFile, host, 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); + const host = updateJSDocHost(parent); + this.insertJsdocCommentBefore(sourceFile, host, tag); } public replaceRangeWithText(sourceFile: SourceFile, range: TextRange, text: string): void { @@ -940,7 +940,7 @@ namespace ts.textChanges { } } - function getJsDocNode(parent: HasJSDoc): HasJSDoc { + function updateJSDocHost(parent: HasJSDoc): HasJSDoc { const jsDocNode = parent.kind === SyntaxKind.ArrowFunction ? parent.parent.kind === SyntaxKind.PropertyDeclaration ? parent.parent as HasJSDoc : From c02478d13ce2fb5516c6479767631bb3c0e4c2e2 Mon Sep 17 00:00:00 2001 From: stkevintan Date: Wed, 10 Nov 2021 21:40:24 +0800 Subject: [PATCH 7/7] fix: test failures --- src/compiler/emitter.ts | 2 +- src/services/codefixes/fixOverrideModifier.ts | 35 ++++++++++++++++++- src/services/textChanges.ts | 24 +++++++------ .../fourslash/codeFixOverrideModifier_js1.ts | 2 +- .../fourslash/codeFixOverrideModifier_js2.ts | 2 +- 5 files changed, 50 insertions(+), 15 deletions(-) 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 851222b5aaec5..4b554d1c10dd8 100644 --- a/src/services/codefixes/fixOverrideModifier.ts +++ b/src/services/codefixes/fixOverrideModifier.ts @@ -17,7 +17,11 @@ 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, ]; interface ErrorCodeFixInfo { @@ -27,30 +31,55 @@ namespace ts.codefix { } 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, } }; @@ -89,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); diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 6ad6d8e937aec..44ac36ab2046d 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -495,22 +495,22 @@ namespace ts.textChanges { } public addJSDocTags(sourceFile: SourceFile, parent: HasJSDoc, newTags: readonly JSDocTag[]): void { - const comments = mapDefined(parent.jsDoc, j => j.comment); + 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 unmergedNewTags = newTags.filter(newTag => !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 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 = 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 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); } @@ -790,6 +790,7 @@ 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) { @@ -941,11 +942,12 @@ namespace ts.textChanges { } function updateJSDocHost(parent: HasJSDoc): HasJSDoc { - const jsDocNode = parent.kind === SyntaxKind.ArrowFunction ? - parent.parent.kind === SyntaxKind.PropertyDeclaration ? - parent.parent as HasJSDoc : - parent.parent.parent as HasJSDoc : - parent; + 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; diff --git a/tests/cases/fourslash/codeFixOverrideModifier_js1.ts b/tests/cases/fourslash/codeFixOverrideModifier_js1.ts index d5bcdd818df2b..8f3584021cb92 100644 --- a/tests/cases/fourslash/codeFixOverrideModifier_js1.ts +++ b/tests/cases/fourslash/codeFixOverrideModifier_js1.ts @@ -21,7 +21,7 @@ verify.codeFix({ } class D extends B { /** - * @public + * * @override */ foo (v) {} diff --git a/tests/cases/fourslash/codeFixOverrideModifier_js2.ts b/tests/cases/fourslash/codeFixOverrideModifier_js2.ts index 37b6356d7aa9d..b2091a65498e5 100644 --- a/tests/cases/fourslash/codeFixOverrideModifier_js2.ts +++ b/tests/cases/fourslash/codeFixOverrideModifier_js2.ts @@ -21,7 +21,7 @@ verify.codeFix({ `class B { } class D extends B { /** - * @public + * */ foo (v) {} }`,