fix(47417): allow undefined type to be added to JSDoc types (microsoft#47449)

a-tarasyuk · web-flow · commit febfd442cdba · 2022-01-16T13:44:07.000-08:00
diff --git a/src/services/codefixes/fixStrictClassInitialization.ts b/src/services/codefixes/fixStrictClassInitialization.ts
@@ -8,37 +8,33 @@ namespace ts.codefix {
     registerCodeFix({
         errorCodes,
         getCodeActions: function getCodeActionsForStrictClassInitializationErrors(context) {
-            const propertyDeclaration = getPropertyDeclaration(context.sourceFile, context.span.start);
-            if (!propertyDeclaration) return;
-
-            const result = [
-                getActionForAddMissingUndefinedType(context, propertyDeclaration),
-                getActionForAddMissingDefiniteAssignmentAssertion(context, propertyDeclaration)
-            ];
-
-            append(result, getActionForAddMissingInitializer(context, propertyDeclaration));
+            const info = getInfo(context.sourceFile, context.span.start);
+            if (!info) return;
 
+            const result: CodeFixAction[] = [];
+            append(result, getActionForAddMissingUndefinedType(context, info));
+            append(result, getActionForAddMissingDefiniteAssignmentAssertion(context, info));
+            append(result, getActionForAddMissingInitializer(context, info));
             return result;
         },
         fixIds: [fixIdAddDefiniteAssignmentAssertions, fixIdAddUndefinedType, fixIdAddInitializer],
         getAllCodeActions: context => {
             return codeFixAll(context, errorCodes, (changes, diag) => {
-                const propertyDeclaration = getPropertyDeclaration(diag.file, diag.start);
-                if (!propertyDeclaration) return;
+                const info = getInfo(diag.file, diag.start);
+                if (!info) return;
 
                 switch (context.fixId) {
                     case fixIdAddDefiniteAssignmentAssertions:
-                        addDefiniteAssignmentAssertion(changes, diag.file, propertyDeclaration);
+                        addDefiniteAssignmentAssertion(changes, diag.file, info.prop);
                         break;
                     case fixIdAddUndefinedType:
-                        addUndefinedType(changes, diag.file, propertyDeclaration);
+                        addUndefinedType(changes, diag.file, info);
                         break;
                     case fixIdAddInitializer:
                         const checker = context.program.getTypeChecker();
-                        const initializer = getInitializer(checker, propertyDeclaration);
+                        const initializer = getInitializer(checker, info.prop);
                         if (!initializer) return;
-
-                        addInitializer(changes, diag.file, propertyDeclaration, initializer);
+                        addInitializer(changes, diag.file, info.prop, initializer);
                         break;
                     default:
                         Debug.fail(JSON.stringify(context.fixId));
@@ -47,14 +43,27 @@ namespace ts.codefix {
         },
     });
 
-    function getPropertyDeclaration(sourceFile: SourceFile, pos: number): PropertyDeclaration | undefined {
+    interface Info {
+        prop: PropertyDeclaration;
+        type: TypeNode;
+        isJs: boolean;
+    }
+
+    function getInfo(sourceFile: SourceFile, pos: number): Info | undefined {
         const token = getTokenAtPosition(sourceFile, pos);
-        return isIdentifier(token) ? cast(token.parent, isPropertyDeclaration) : undefined;
+        if (isIdentifier(token) && isPropertyDeclaration(token.parent)) {
+            const type = getEffectiveTypeAnnotationNode(token.parent);
+            if (type) {
+                return { type, prop: token.parent, isJs: isInJSFile(token.parent) };
+            }
+        }
+        return undefined;
     }
 
-    function getActionForAddMissingDefiniteAssignmentAssertion(context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction {
-        const changes = textChanges.ChangeTracker.with(context, t => addDefiniteAssignmentAssertion(t, context.sourceFile, propertyDeclaration));
-        return createCodeFixAction(fixName, changes, [Diagnostics.Add_definite_assignment_assertion_to_property_0, propertyDeclaration.getText()], fixIdAddDefiniteAssignmentAssertions, Diagnostics.Add_definite_assignment_assertions_to_all_uninitialized_properties);
+    function getActionForAddMissingDefiniteAssignmentAssertion(context: CodeFixContext, info: Info): CodeFixAction | undefined {
+        if (info.isJs) return undefined;
+        const changes = textChanges.ChangeTracker.with(context, t => addDefiniteAssignmentAssertion(t, context.sourceFile, info.prop));
+        return createCodeFixAction(fixName, changes, [Diagnostics.Add_definite_assignment_assertion_to_property_0, info.prop.getText()], fixIdAddDefiniteAssignmentAssertions, Diagnostics.Add_definite_assignment_assertions_to_all_uninitialized_properties);
     }
 
     function addDefiniteAssignmentAssertion(changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration): void {
@@ -70,25 +79,32 @@ namespace ts.codefix {
         changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration, property);
     }
 
-    function getActionForAddMissingUndefinedType(context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction {
-        const changes = textChanges.ChangeTracker.with(context, t => addUndefinedType(t, context.sourceFile, propertyDeclaration));
-        return createCodeFixAction(fixName, changes, [Diagnostics.Add_undefined_type_to_property_0, propertyDeclaration.name.getText()], fixIdAddUndefinedType, Diagnostics.Add_undefined_type_to_all_uninitialized_properties);
+    function getActionForAddMissingUndefinedType(context: CodeFixContext, info: Info): CodeFixAction {
+        const changes = textChanges.ChangeTracker.with(context, t => addUndefinedType(t, context.sourceFile, info));
+        return createCodeFixAction(fixName, changes, [Diagnostics.Add_undefined_type_to_property_0, info.prop.name.getText()], fixIdAddUndefinedType, Diagnostics.Add_undefined_type_to_all_uninitialized_properties);
     }
 
-    function addUndefinedType(changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration): void {
+    function addUndefinedType(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, info: Info): void {
         const undefinedTypeNode = factory.createKeywordTypeNode(SyntaxKind.UndefinedKeyword);
-        const type = propertyDeclaration.type!; // TODO: GH#18217
-        const types = isUnionTypeNode(type) ? type.types.concat(undefinedTypeNode) : [type, undefinedTypeNode];
-        changeTracker.replaceNode(propertyDeclarationSourceFile, type, factory.createUnionTypeNode(types));
+        const types = isUnionTypeNode(info.type) ? info.type.types.concat(undefinedTypeNode) : [info.type, undefinedTypeNode];
+        const unionTypeNode = factory.createUnionTypeNode(types);
+        if (info.isJs) {
+            changeTracker.addJSDocTags(sourceFile, info.prop, [factory.createJSDocTypeTag(/*tagName*/ undefined, factory.createJSDocTypeExpression(unionTypeNode))]);
+        }
+        else {
+            changeTracker.replaceNode(sourceFile, info.type, unionTypeNode);
+        }
     }
 
-    function getActionForAddMissingInitializer(context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction | undefined {
+    function getActionForAddMissingInitializer(context: CodeFixContext, info: Info): CodeFixAction | undefined {
+        if (info.isJs) return undefined;
+
         const checker = context.program.getTypeChecker();
-        const initializer = getInitializer(checker, propertyDeclaration);
+        const initializer = getInitializer(checker, info.prop);
         if (!initializer) return undefined;
 
-        const changes = textChanges.ChangeTracker.with(context, t => addInitializer(t, context.sourceFile, propertyDeclaration, initializer));
-        return createCodeFixAction(fixName, changes, [Diagnostics.Add_initializer_to_property_0, propertyDeclaration.name.getText()], fixIdAddInitializer, Diagnostics.Add_initializers_to_all_uninitialized_properties);
+        const changes = textChanges.ChangeTracker.with(context, t => addInitializer(t, context.sourceFile, info.prop, initializer));
+        return createCodeFixAction(fixName, changes, [Diagnostics.Add_initializer_to_property_0, info.prop.name.getText()], fixIdAddInitializer, Diagnostics.Add_initializers_to_all_uninitialized_properties);
     }
 
     function addInitializer(changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration, initializer: Expression): void {
diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts
@@ -502,7 +502,11 @@ namespace ts.textChanges {
                 if (merged) oldTags[i] = merged;
                 return !!merged;
             }));
-            const tag = factory.createJSDocComment(factory.createNodeArray(intersperse(comments, factory.createJSDocText("\n"))), factory.createNodeArray([...oldTags, ...unmergedNewTags]));
+            const tags = [...oldTags, ...unmergedNewTags];
+            const jsDoc = singleOrUndefined(parent.jsDoc);
+            const comment = jsDoc && positionsAreOnSameLine(jsDoc.pos, jsDoc.end, sourceFile) && !length(comments) ? undefined :
+                factory.createNodeArray(intersperse(comments, factory.createJSDocText("\n")));
+            const tag = factory.createJSDocComment(comment, factory.createNodeArray(tags));
             const host = updateJSDocHost(parent);
             this.insertJsdocCommentBefore(sourceFile, host, tag);
         }
@@ -967,6 +971,8 @@ namespace ts.textChanges {
             }
             case SyntaxKind.JSDocReturnTag:
                 return factory.createJSDocReturnTag(/*tagName*/ undefined, (newTag as JSDocReturnTag).typeExpression, oldTag.comment);
+            case SyntaxKind.JSDocTypeTag:
+                return factory.createJSDocTypeTag(/*tagName*/ undefined, (newTag as JSDocTypeTag).typeExpression, oldTag.comment);
         }
     }
 
diff --git a/tests/cases/fourslash/codeFixClassPropertyInitialization14.ts b/tests/cases/fourslash/codeFixClassPropertyInitialization14.ts
@@ -0,0 +1,21 @@
+/// <reference path='fourslash.ts' />
+
+// @strict: true
+// @checkJs: true
+// @allowJs: true
+// @filename: a.js
+
+////class Foo {
+////    /** @type {string} */
+////    a;
+////}
+
+verify.codeFix({
+    description: `Add 'undefined' type to property 'a'`,
+    newFileContent:
+`class Foo {
+    /** @type {string | undefined} */
+    a;
+}`,
+    index: 2
+})
diff --git a/tests/cases/fourslash/codeFixClassPropertyInitialization15.ts b/tests/cases/fourslash/codeFixClassPropertyInitialization15.ts
@@ -0,0 +1,26 @@
+/// <reference path='fourslash.ts' />
+
+// @strict: true
+// @checkJs: true
+// @allowJs: true
+// @filename: a.js
+////class Foo {
+////    /**
+////     * comment
+////     * @type {string}
+////     */
+////    a;
+////}
+
+verify.codeFix({
+    description: `Add 'undefined' type to property 'a'`,
+    newFileContent:
+`class Foo {
+    /**
+     * comment
+     * @type {string | undefined}
+     */
+    a;
+}`,
+    index: 2
+})
diff --git a/tests/cases/fourslash/codeFixClassPropertyInitialization16.ts b/tests/cases/fourslash/codeFixClassPropertyInitialization16.ts
@@ -0,0 +1,24 @@
+/// <reference path='fourslash.ts' />
+
+// @strict: true
+// @checkJs: true
+// @allowJs: true
+// @filename: a.js
+////class Foo {
+////    /**
+////     * @type {string}
+////     */
+////    a;
+////}
+
+verify.codeFix({
+    description: `Add 'undefined' type to property 'a'`,
+    newFileContent:
+`class Foo {
+    /**
+     * @type {string | undefined}
+     */
+    a;
+}`,
+    index: 2
+})
diff --git a/tests/cases/fourslash/codeFixClassPropertyInitialization_all_4.ts b/tests/cases/fourslash/codeFixClassPropertyInitialization_all_4.ts
@@ -0,0 +1,46 @@
+/// <reference path='fourslash.ts' />
+
+// @strict: true
+// @checkJs: true
+// @allowJs: true
+// @filename: a.js
+////class A {
+////    /**
+////     * comment
+////     * @type {string}
+////     */
+////    a;
+////}
+////class B {
+////    /** @type {string} */
+////    a;
+////}
+////class C {
+////    /**
+////     * @type {string}
+////     */
+////    a;
+////}
+
+verify.codeFixAll({
+    fixId: 'addMissingPropertyUndefinedType',
+    fixAllDescription: "Add undefined type to all uninitialized properties",
+    newFileContent:
+`class A {
+    /**
+     * comment
+     * @type {string | undefined}
+     */
+    a;
+}
+class B {
+    /** @type {string | undefined} */
+    a;
+}
+class C {
+    /**
+     * @type {string | undefined}
+     */
+    a;
+}`
+});

Original file line number	Diff line number	Diff line change
`@@ -502,7 +502,11 @@ namespace ts.textChanges {`
`502`	`502`	`if (merged) oldTags[i] = merged;`
`503`	`503`	`return !!merged;`
`504`	`504`	`}));`
`505`		`- const tag = factory.createJSDocComment(factory.createNodeArray(intersperse(comments, factory.createJSDocText("\n"))), factory.createNodeArray([...oldTags, ...unmergedNewTags]));`
	`505`	`+ const tags = [...oldTags, ...unmergedNewTags];`
	`506`	`+ const jsDoc = singleOrUndefined(parent.jsDoc);`
	`507`	`+ const comment = jsDoc && positionsAreOnSameLine(jsDoc.pos, jsDoc.end, sourceFile) && !length(comments) ? undefined :`
	`508`	`+ factory.createNodeArray(intersperse(comments, factory.createJSDocText("\n")));`
	`509`	`+ const tag = factory.createJSDocComment(comment, factory.createNodeArray(tags));`
`506`	`510`	`const host = updateJSDocHost(parent);`
`507`	`511`	`this.insertJsdocCommentBefore(sourceFile, host, tag);`
`508`	`512`	`}`
`@@ -967,6 +971,8 @@ namespace ts.textChanges {`
`967`	`971`	`}`
`968`	`972`	`case SyntaxKind.JSDocReturnTag:`
`969`	`973`	`return factory.createJSDocReturnTag(/tagName/ undefined, (newTag as JSDocReturnTag).typeExpression, oldTag.comment);`
	`974`	`+ case SyntaxKind.JSDocTypeTag:`
	`975`	`+ return factory.createJSDocTypeTag(/tagName/ undefined, (newTag as JSDocTypeTag).typeExpression, oldTag.comment);`
`970`	`976`	`}`
`971`	`977`	`}`
`972`	`978`