Skip to content

Commit 9b7ca24

Browse files
stkevintanKingwl
stkevintan
and
Kingwl
committed
feat: code fix for override in js files
Co-Authored-By: Wenlu Wang <[email protected]>
1 parent 339ad92 commit 9b7ca24

File tree

7 files changed

+117
-66
lines changed

7 files changed

+117
-66
lines changed

‎src/compiler/emitter.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,6 +1613,10 @@ namespace ts {
16131613
return emitJSDocSignature(node as JSDocSignature);
16141614
case SyntaxKind.JSDocTag:
16151615
case SyntaxKind.JSDocClassTag:
1616+
case SyntaxKind.JSDocPublicTag:
1617+
case SyntaxKind.JSDocPrivateTag:
1618+
case SyntaxKind.JSDocProtectedTag:
1619+
case SyntaxKind.JSDocOverrideTag:
16161620
return emitJSDocSimpleTag(node as JSDocTag);
16171621
case SyntaxKind.JSDocAugmentsTag:
16181622
case SyntaxKind.JSDocImplementsTag:
@@ -1621,11 +1625,7 @@ namespace ts {
16211625
case SyntaxKind.JSDocDeprecatedTag:
16221626
return;
16231627
// SyntaxKind.JSDocClassTag (see JSDocTag, above)
1624-
case SyntaxKind.JSDocPublicTag:
1625-
case SyntaxKind.JSDocPrivateTag:
1626-
case SyntaxKind.JSDocProtectedTag:
16271628
case SyntaxKind.JSDocReadonlyTag:
1628-
case SyntaxKind.JSDocOverrideTag:
16291629
return;
16301630
case SyntaxKind.JSDocCallbackTag:
16311631
return emitJSDocCallbackTag(node as JSDocCallbackTag);

‎src/compiler/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7401,7 +7401,7 @@ namespace ts {
74017401
updateJSDocUnknownTag(node: JSDocUnknownTag, tagName: Identifier, comment: string | NodeArray<JSDocComment> | undefined): JSDocUnknownTag;
74027402
createJSDocDeprecatedTag(tagName: Identifier, comment?: string | NodeArray<JSDocComment>): JSDocDeprecatedTag;
74037403
updateJSDocDeprecatedTag(node: JSDocDeprecatedTag, tagName: Identifier, comment?: string | NodeArray<JSDocComment>): JSDocDeprecatedTag;
7404-
createJSDocOverrideTag(tagName: Identifier, comment?: string | NodeArray<JSDocComment>): JSDocOverrideTag;
7404+
createJSDocOverrideTag(tagName: Identifier | undefined, comment?: string | NodeArray<JSDocComment>): JSDocOverrideTag;
74057405
updateJSDocOverrideTag(node: JSDocOverrideTag, tagName: Identifier, comment?: string | NodeArray<JSDocComment>): JSDocOverrideTag;
74067406
createJSDocText(text: string): JSDocText;
74077407
updateJSDocText(node: JSDocText, text: string): JSDocText;

‎src/services/codefixes/fixOverrideModifier.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,34 +20,33 @@ namespace ts.codefix {
2020
Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code
2121
];
2222

23-
const errorCodeFixIdMap: Record<number, [DiagnosticMessage, string | undefined, DiagnosticMessage | undefined]> = {
23+
const errorCodeFixIdMap: Record<number, [message: DiagnosticMessage, fixId: string | undefined, fixAllMessage: DiagnosticMessage | undefined]> = {
2424
[Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0.code]: [
2525
Diagnostics.Add_override_modifier, fixAddOverrideId, Diagnostics.Add_all_missing_override_modifiers,
2626
],
2727
[Diagnostics.This_member_cannot_have_an_override_modifier_because_its_containing_class_0_does_not_extend_another_class.code]: [
28-
Diagnostics.Remove_override_modifier, fixRemoveOverrideId, Diagnostics.Remove_all_unnecessary_override_modifiers
28+
Diagnostics.Remove_override_modifier, fixRemoveOverrideId, Diagnostics.Remove_all_unnecessary_override_modifiers,
2929
],
3030
[Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0.code]: [
3131
Diagnostics.Add_override_modifier, fixAddOverrideId, Diagnostics.Add_all_missing_override_modifiers,
3232
],
3333
[Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0.code]: [
34-
Diagnostics.Add_override_modifier, fixAddOverrideId, Diagnostics.Remove_all_unnecessary_override_modifiers
34+
Diagnostics.Add_override_modifier, fixAddOverrideId, Diagnostics.Remove_all_unnecessary_override_modifiers,
3535
],
3636
[Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0.code]: [
37-
Diagnostics.Remove_override_modifier, fixRemoveOverrideId, Diagnostics.Remove_all_unnecessary_override_modifiers
37+
Diagnostics.Remove_override_modifier, fixRemoveOverrideId, Diagnostics.Remove_all_unnecessary_override_modifiers,
3838
]
3939
};
4040

4141
registerCodeFix({
4242
errorCodes,
4343
getCodeActions: context => {
44-
const { errorCode, span, sourceFile } = context;
44+
const { errorCode, span } = context;
4545

4646
const info = errorCodeFixIdMap[errorCode];
4747
if (!info) return emptyArray;
4848

4949
const [ descriptions, fixId, fixAllDescriptions ] = info;
50-
if (isSourceFileJS(sourceFile)) return emptyArray;
5150
const changes = textChanges.ChangeTracker.with(context, changes => dispatchChanges(changes, context, errorCode, span.start));
5251

5352
return [
@@ -57,9 +56,9 @@ namespace ts.codefix {
5756
fixIds: [fixName, fixAddOverrideId, fixRemoveOverrideId],
5857
getAllCodeActions: context =>
5958
codeFixAll(context, errorCodes, (changes, diag) => {
60-
const { code, start, file } = diag;
59+
const { code, start } = diag;
6160
const info = errorCodeFixIdMap[code];
62-
if (!info || info[1] !== context.fixId || isSourceFileJS(file)) {
61+
if (!info || info[1] !== context.fixId) {
6362
return;
6463
}
6564

@@ -87,6 +86,10 @@ namespace ts.codefix {
8786

8887
function doAddOverrideModifierChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number) {
8988
const classElement = findContainerClassElementLike(sourceFile, pos);
89+
if (isSourceFileJS(sourceFile)) {
90+
changeTracker.addJSDocTags(sourceFile, classElement, [ factory.createJSDocOverrideTag(/*tagName*/ undefined)]);
91+
return;
92+
}
9093
const modifiers = classElement.modifiers || emptyArray;
9194
const staticModifier = find(modifiers, isStaticModifier);
9295
const abstractModifier = find(modifiers, isAbstractModifier);
@@ -101,6 +104,10 @@ namespace ts.codefix {
101104

102105
function doRemoveOverrideModifierChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number) {
103106
const classElement = findContainerClassElementLike(sourceFile, pos);
107+
if (isSourceFileJS(sourceFile)) {
108+
changeTracker.filterJSDocTags(sourceFile, classElement, not(isJSDocOverrideTag));
109+
return;
110+
}
104111
const overrideModifier = classElement.modifiers && find(classElement.modifiers, modifier => modifier.kind === SyntaxKind.OverrideKeyword);
105112
Debug.assertIsDefined(overrideModifier);
106113

‎src/services/codefixes/inferFromUsage.ts

Lines changed: 4 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ namespace ts.codefix {
129129
if (typeNode) {
130130
// Note that the codefix will never fire with an existing `@type` tag, so there is no need to merge tags
131131
const typeTag = factory.createJSDocTypeTag(/*tagName*/ undefined, factory.createJSDocTypeExpression(typeNode), /*comment*/ undefined);
132-
addJSDocTags(changes, sourceFile, cast(parent.parent.parent, isExpressionStatement), [typeTag]);
132+
changes.addJSDocTags(sourceFile, cast(parent.parent.parent, isExpressionStatement), [typeTag]);
133133
}
134134
importAdder.writeFixes(changes);
135135
return parent;
@@ -269,7 +269,7 @@ namespace ts.codefix {
269269
}
270270

271271
function annotateJSDocThis(changes: textChanges.ChangeTracker, sourceFile: SourceFile, containingFunction: SignatureDeclaration, typeNode: TypeNode) {
272-
addJSDocTags(changes, sourceFile, containingFunction, [
272+
changes.addJSDocTags(sourceFile, containingFunction, [
273273
factory.createJSDocThisTag(/*tagName*/ undefined, factory.createJSDocTypeExpression(typeNode)),
274274
]);
275275
}
@@ -309,7 +309,7 @@ namespace ts.codefix {
309309
}
310310
const typeExpression = factory.createJSDocTypeExpression(typeNode);
311311
const typeTag = isGetAccessorDeclaration(declaration) ? factory.createJSDocReturnTag(/*tagName*/ undefined, typeExpression, /*comment*/ undefined) : factory.createJSDocTypeTag(/*tagName*/ undefined, typeExpression, /*comment*/ undefined);
312-
addJSDocTags(changes, sourceFile, parent, [typeTag]);
312+
changes.addJSDocTags(sourceFile, parent, [typeTag]);
313313
}
314314
else if (!tryReplaceImportTypeNodeWithAutoImport(typeNode, declaration, sourceFile, changes, importAdder, getEmitScriptTarget(program.getCompilerOptions()))) {
315315
changes.tryInsertTypeAnnotation(sourceFile, declaration, typeNode);
@@ -376,46 +376,7 @@ namespace ts.codefix {
376376
else {
377377
const paramTags = map(inferences, ({ name, typeNode, isOptional }) =>
378378
factory.createJSDocParameterTag(/*tagName*/ undefined, name, /*isBracketed*/ !!isOptional, factory.createJSDocTypeExpression(typeNode), /* isNameFirst */ false, /*comment*/ undefined));
379-
addJSDocTags(changes, sourceFile, signature, paramTags);
380-
}
381-
}
382-
383-
export function addJSDocTags(changes: textChanges.ChangeTracker, sourceFile: SourceFile, parent: HasJSDoc, newTags: readonly JSDocTag[]): void {
384-
const comments = flatMap(parent.jsDoc, j => typeof j.comment === "string" ? factory.createJSDocText(j.comment) : j.comment) as JSDocComment[];
385-
const oldTags = flatMapToMutable(parent.jsDoc, j => j.tags);
386-
const unmergedNewTags = newTags.filter(newTag => !oldTags || !oldTags.some((tag, i) => {
387-
const merged = tryMergeJsdocTags(tag, newTag);
388-
if (merged) oldTags[i] = merged;
389-
return !!merged;
390-
}));
391-
const tag = factory.createJSDocComment(factory.createNodeArray(intersperse(comments, factory.createJSDocText("\n"))), factory.createNodeArray([...(oldTags || emptyArray), ...unmergedNewTags]));
392-
const jsDocNode = parent.kind === SyntaxKind.ArrowFunction ? getJsDocNodeForArrowFunction(parent) : parent;
393-
jsDocNode.jsDoc = parent.jsDoc;
394-
jsDocNode.jsDocCache = parent.jsDocCache;
395-
changes.insertJsdocCommentBefore(sourceFile, jsDocNode, tag);
396-
}
397-
398-
function getJsDocNodeForArrowFunction(signature: ArrowFunction): HasJSDoc {
399-
if (signature.parent.kind === SyntaxKind.PropertyDeclaration) {
400-
return signature.parent as HasJSDoc;
401-
}
402-
return signature.parent.parent as HasJSDoc;
403-
}
404-
405-
function tryMergeJsdocTags(oldTag: JSDocTag, newTag: JSDocTag): JSDocTag | undefined {
406-
if (oldTag.kind !== newTag.kind) {
407-
return undefined;
408-
}
409-
switch (oldTag.kind) {
410-
case SyntaxKind.JSDocParameterTag: {
411-
const oldParam = oldTag as JSDocParameterTag;
412-
const newParam = newTag as JSDocParameterTag;
413-
return isIdentifier(oldParam.name) && isIdentifier(newParam.name) && oldParam.name.escapedText === newParam.name.escapedText
414-
? factory.createJSDocParameterTag(/*tagName*/ undefined, newParam.name, /*isBracketed*/ false, newParam.typeExpression, newParam.isNameFirst, oldParam.comment)
415-
: undefined;
416-
}
417-
case SyntaxKind.JSDocReturnTag:
418-
return factory.createJSDocReturnTag(/*tagName*/ undefined, (newTag as JSDocReturnTag).typeExpression, oldTag.comment);
379+
changes.addJSDocTags(sourceFile, signature, paramTags);
419380
}
420381
}
421382

‎src/services/textChanges.ts

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,55 @@ namespace ts.textChanges {
494494
this.insertNodeAt(sourceFile, fnStart, tag, { preserveLeadingWhitespace: false, suffix: this.newLineCharacter + indent });
495495
}
496496

497+
public addJSDocTags(sourceFile: SourceFile, parent: HasJSDoc, newTags: readonly JSDocTag[]): void {
498+
const comments = mapDefined(parent.jsDoc, j => j.comment);
499+
const oldTags = flatMapToMutable(parent.jsDoc, j => j.tags);
500+
const unmergedNewTags = newTags.filter(newTag => !oldTags || !oldTags.some((tag, i) => {
501+
const merged = this.tryMergeJsdocTags(tag, newTag);
502+
if (merged) oldTags[i] = merged;
503+
return !!merged;
504+
}));
505+
const tag = factory.createJSDocComment(comments.join("\n"), factory.createNodeArray([...(oldTags || emptyArray), ...unmergedNewTags]));
506+
const jsDocNode = parent.kind === SyntaxKind.ArrowFunction ? this.getJsDocNodeForArrowFunction(parent) : parent;
507+
jsDocNode.jsDoc = parent.jsDoc;
508+
jsDocNode.jsDocCache = parent.jsDocCache;
509+
this.insertJsdocCommentBefore(sourceFile, jsDocNode, tag);
510+
}
511+
512+
public filterJSDocTags(sourceFile: SourceFile, parent: HasJSDoc, predicate: (tag: JSDocTag) => boolean): void {
513+
const comments = mapDefined(parent.jsDoc, j => j.comment);
514+
const oldTags = filter(flatMapToMutable(parent.jsDoc, j => j.tags), predicate);
515+
const tag = factory.createJSDocComment(comments.join("\n"), factory.createNodeArray([...(oldTags || emptyArray)]));
516+
const jsDocNode = parent.kind === SyntaxKind.ArrowFunction ? this.getJsDocNodeForArrowFunction(parent) : parent;
517+
jsDocNode.jsDoc = parent.jsDoc;
518+
jsDocNode.jsDocCache = parent.jsDocCache;
519+
this.insertJsdocCommentBefore(sourceFile, jsDocNode, tag);
520+
}
521+
522+
public getJsDocNodeForArrowFunction(signature: ArrowFunction): HasJSDoc {
523+
if (signature.parent.kind === SyntaxKind.PropertyDeclaration) {
524+
return signature.parent as HasJSDoc;
525+
}
526+
return signature.parent.parent as HasJSDoc;
527+
}
528+
529+
public tryMergeJsdocTags(oldTag: JSDocTag, newTag: JSDocTag): JSDocTag | undefined {
530+
if (oldTag.kind !== newTag.kind) {
531+
return undefined;
532+
}
533+
switch (oldTag.kind) {
534+
case SyntaxKind.JSDocParameterTag: {
535+
const oldParam = oldTag as JSDocParameterTag;
536+
const newParam = newTag as JSDocParameterTag;
537+
return isIdentifier(oldParam.name) && isIdentifier(newParam.name) && oldParam.name.escapedText === newParam.name.escapedText
538+
? factory.createJSDocParameterTag(/*tagName*/ undefined, newParam.name, /*isBracketed*/ false, newParam.typeExpression, newParam.isNameFirst, oldParam.comment)
539+
: undefined;
540+
}
541+
case SyntaxKind.JSDocReturnTag:
542+
return factory.createJSDocReturnTag(/*tagName*/ undefined, (newTag as JSDocReturnTag).typeExpression, oldTag.comment);
543+
}
544+
}
545+
497546
public replaceRangeWithText(sourceFile: SourceFile, range: TextRange, text: string): void {
498547
this.changes.push({ kind: ChangeKind.Text, sourceFile, range, text });
499548
}
@@ -769,7 +818,6 @@ namespace ts.textChanges {
769818
public insertNodeInListAfter(sourceFile: SourceFile, after: Node, newNode: Node, containingList = formatting.SmartIndenter.getContainingList(after, sourceFile)): void {
770819
if (!containingList) {
771820
Debug.fail("node is not a list element");
772-
return;
773821
}
774822
const index = indexOfNode(containingList, after);
775823
if (index < 0) {

‎tests/cases/fourslash/codeFixOverrideModifier_js1.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
/// <reference path='fourslash.ts' />
2-
32
// @allowJs: true
43
// @checkJs: true
54
// @noEmit: true
@@ -9,14 +8,22 @@
98
//// foo (v) {}
109
//// }
1110
//// class D extends B {
11+
//// /** @public */
1212
//// foo (v) {}
13-
//// /**@override*/
14-
//// bar (v) {}
15-
//// }
16-
//// class C {
17-
//// /**@override*/
18-
//// foo () {}
1913
//// }
2014

21-
verify.not.codeFixAvailable("fixAddOverrideModifier");
22-
verify.not.codeFixAvailable("fixRemoveOverrideModifier");
15+
verify.codeFix({
16+
description: "Add 'override' modifier",
17+
index: 0,
18+
newFileContent:
19+
`class B {
20+
foo (v) {}
21+
}
22+
class D extends B {
23+
/**
24+
* @public
25+
* @override
26+
*/
27+
foo (v) {}
28+
}`,
29+
})

‎tests/cases/fourslash/codeFixOverrideModifier_js2.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @allowJs: true
4+
// @checkJs: true
5+
// @noEmit: true
6+
// @noImplicitOverride: true
7+
// @filename: a.js
8+
//// class B { }
9+
//// class D extends B {
10+
//// /**
11+
//// * @public
12+
//// * @override
13+
//// */
14+
//// foo (v) {}
15+
//// }
16+
17+
verify.codeFix({
18+
description: "Remove 'override' modifier",
19+
index: 0,
20+
newFileContent:
21+
`class B { }
22+
class D extends B {
23+
/**
24+
* @public
25+
*/
26+
foo (v) {}
27+
}`,
28+
})

0 commit comments

Comments
 (0)