annotateWithTypeFromJSDoc: Use changes.insertTypeAnnotation instead of replaceNode (#22404)

Author: Andy

src/compiler/utilities.ts

@@ -2957,49 +2957,38 @@ namespace ts {
      * Gets the effective type annotation of a variable, parameter, or property. If the node was
      * parsed in a JavaScript file, gets the type annotation from JSDoc.
      */
-    export function getEffectiveTypeAnnotationNode(node: Node, checkJSDoc?: boolean): TypeNode | undefined {
-        if (hasType(node)) {
-            return node.type;
-        }
-        if (checkJSDoc || isInJavaScriptFile(node)) {
-            return getJSDocType(node);
-        }
+    export function getEffectiveTypeAnnotationNode(node: Node): TypeNode | undefined {
+        return (node as HasType).type || (isInJavaScriptFile(node) ? getJSDocType(node) : undefined);
     }
 
     /**
      * Gets the effective return type annotation of a signature. If the node was parsed in a
      * JavaScript file, gets the return type annotation from JSDoc.
      */
-    export function getEffectiveReturnTypeNode(node: SignatureDeclaration, checkJSDoc?: boolean): TypeNode | undefined {
-        if (node.type) {
-            return node.type;
-        }
-        if (checkJSDoc || isInJavaScriptFile(node)) {
-            return getJSDocReturnType(node);
-        }
+    export function getEffectiveReturnTypeNode(node: SignatureDeclaration): TypeNode | undefined {
+        return node.type || (isInJavaScriptFile(node) ? getJSDocReturnType(node) : undefined);
     }
 
     /**
      * Gets the effective type parameters. If the node was parsed in a
      * JavaScript file, gets the type parameters from the `@template` tag from JSDoc.
      */
-    export function getEffectiveTypeParameterDeclarations(node: DeclarationWithTypeParameters, checkJSDoc?: boolean): ReadonlyArray<TypeParameterDeclaration> {
-        if (node.typeParameters) {
-            return node.typeParameters;
-        }
-        if (checkJSDoc || isInJavaScriptFile(node)) {
-            const templateTag = getJSDocTemplateTag(node);
-            return templateTag && templateTag.typeParameters;
-        }
+    export function getEffectiveTypeParameterDeclarations(node: DeclarationWithTypeParameters): ReadonlyArray<TypeParameterDeclaration> | undefined {
+        return node.typeParameters || (isInJavaScriptFile(node) ? getJSDocTypeParameterDeclarations(node) : undefined);
+    }
+
+    export function getJSDocTypeParameterDeclarations(node: DeclarationWithTypeParameters): ReadonlyArray<TypeParameterDeclaration> {
+        const templateTag = getJSDocTemplateTag(node);
+        return templateTag && templateTag.typeParameters;
     }
 
     /**
      * Gets the effective type annotation of the value parameter of a set accessor. If the node
      * was parsed in a JavaScript file, gets the type annotation from JSDoc.
      */
-    export function getEffectiveSetAccessorTypeAnnotationNode(node: SetAccessorDeclaration, checkJSDoc?: boolean): TypeNode {
+    export function getEffectiveSetAccessorTypeAnnotationNode(node: SetAccessorDeclaration): TypeNode {
         const parameter = getSetAccessorValueParameter(node);
-        return parameter && getEffectiveTypeAnnotationNode(parameter, checkJSDoc);
+        return parameter && getEffectiveTypeAnnotationNode(parameter);
     }
 
     export function emitNewLineBeforeLeadingComments(lineMap: ReadonlyArray<number>, writer: EmitTextWriter, node: TextRange, leadingComments: ReadonlyArray<CommentRange>) {

src/services/codefixes/annotateWithTypeFromJSDoc.ts

@@ -42,19 +42,34 @@ namespace ts.codefix {
 
     function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, decl: DeclarationWithType): void {
         if (isFunctionLikeDeclaration(decl) && (getJSDocReturnType(decl) || decl.parameters.some(p => !!getJSDocType(p)))) {
-            findAncestor(decl, isFunctionLike);
-            const fn = findAncestor(decl, isFunctionLikeDeclaration);
-            const functionWithType = addTypesToFunctionLike(fn);
-            suppressLeadingAndTrailingTrivia(functionWithType);
-            changes.replaceNode(sourceFile, fn, functionWithType, textChanges.useNonAdjustedPositions);
-            return;
+            const typeParameters = getJSDocTypeParameterDeclarations(decl);
+            const returnType = getJSDocReturnType(decl);
+            const returnTypeNode = returnType && transformJSDocType(returnType);
+
+            if (isArrowFunction(decl) && !findChildOfKind(decl, SyntaxKind.OpenParenToken, sourceFile)) {
+                const params = decl.parameters.map(p => {
+                    const paramType = getJSDocType(p);
+                    return paramType && !p.type ? updateParameter(p, p.decorators, p.modifiers, p.dotDotDotToken, p.name, p.questionToken, transformJSDocType(paramType), p.initializer) : p;
+                });
+                changes.replaceNode(sourceFile, decl, updateArrowFunction(decl, decl.modifiers, decl.typeParameters || typeParameters, params, decl.type || returnTypeNode, decl.equalsGreaterThanToken, decl.body));
+            }
+            else {
+                if (typeParameters && !decl.typeParameters) {
+                    changes.insertTypeParameters(sourceFile, decl, typeParameters);
+                }
+                for (const param of decl.parameters) {
+                    if (!param.type) {
+                        const paramType = getJSDocType(param);
+                        if (paramType) changes.insertTypeAnnotation(sourceFile, param, transformJSDocType(paramType));
+                    }
+                }
+                if (returnTypeNode && !decl.type) changes.insertTypeAnnotation(sourceFile, decl, returnTypeNode);
+            }
         }
         else {
             const jsdocType = Debug.assertDefined(getJSDocType(decl)); // If not defined, shouldn't have been an error to fix
             Debug.assert(!decl.type); // If defined, shouldn't have been an error to fix.
-            const declarationWithType = addType(decl, transformJSDocType(jsdocType) as TypeNode);
-            suppressLeadingAndTrailingTrivia(declarationWithType);
-            changes.replaceNode(sourceFile, decl, declarationWithType, textChanges.useNonAdjustedPositions);
+            changes.insertTypeAnnotation(sourceFile, decl, transformJSDocType(jsdocType));
         }
     }
 
@@ -65,48 +80,7 @@ namespace ts.codefix {
             node.kind === SyntaxKind.PropertyDeclaration;
     }
 
-    function addTypesToFunctionLike(decl: FunctionLikeDeclaration) {
-        const typeParameters = getEffectiveTypeParameterDeclarations(decl, /*checkJSDoc*/ true);
-        const parameters = decl.parameters.map(
-            p => createParameter(p.decorators, p.modifiers, p.dotDotDotToken, p.name, p.questionToken, transformJSDocType(getEffectiveTypeAnnotationNode(p, /*checkJSDoc*/ true)) as TypeNode, p.initializer));
-        const returnType = transformJSDocType(getEffectiveReturnTypeNode(decl, /*checkJSDoc*/ true)) as TypeNode;
-        switch (decl.kind) {
-            case SyntaxKind.FunctionDeclaration:
-                return createFunctionDeclaration(decl.decorators, decl.modifiers, decl.asteriskToken, decl.name, typeParameters, parameters, returnType, decl.body);
-            case SyntaxKind.Constructor:
-                return createConstructor(decl.decorators, decl.modifiers, parameters, decl.body);
-            case SyntaxKind.FunctionExpression:
-                return createFunctionExpression(decl.modifiers, decl.asteriskToken, decl.name, typeParameters, parameters, returnType, decl.body);
-            case SyntaxKind.ArrowFunction:
-                return createArrowFunction(decl.modifiers, typeParameters, parameters, returnType, decl.equalsGreaterThanToken, decl.body);
-            case SyntaxKind.MethodDeclaration:
-                return createMethod(decl.decorators, decl.modifiers, decl.asteriskToken, decl.name, decl.questionToken, typeParameters, parameters, returnType, decl.body);
-            case SyntaxKind.GetAccessor:
-                return createGetAccessor(decl.decorators, decl.modifiers, decl.name, decl.parameters, returnType, decl.body);
-            case SyntaxKind.SetAccessor:
-                return createSetAccessor(decl.decorators, decl.modifiers, decl.name, parameters, decl.body);
-            default:
-                return Debug.assertNever(decl, `Unexpected SyntaxKind: ${(decl as any).kind}`);
-        }
-    }
-
-    function addType(decl: DeclarationWithType, jsdocType: TypeNode) {
-        switch (decl.kind) {
-            case SyntaxKind.VariableDeclaration:
-                return createVariableDeclaration(decl.name, jsdocType, decl.initializer);
-            case SyntaxKind.PropertySignature:
-                return createPropertySignature(decl.modifiers, decl.name, decl.questionToken, jsdocType, decl.initializer);
-            case SyntaxKind.PropertyDeclaration:
-                return createProperty(decl.decorators, decl.modifiers, decl.name, decl.questionToken, jsdocType, decl.initializer);
-            default:
-                return Debug.fail(`Unexpected SyntaxKind: ${decl.kind}`);
-        }
-    }
-
-    function transformJSDocType(node: Node): Node | undefined {
-        if (node === undefined) {
-            return undefined;
-        }
+    function transformJSDocType(node: TypeNode): TypeNode | undefined {
         switch (node.kind) {
             case SyntaxKind.JSDocAllType:
             case SyntaxKind.JSDocUnknownType:
@@ -121,12 +95,10 @@ namespace ts.codefix {
                 return transformJSDocVariadicType(node as JSDocVariadicType);
             case SyntaxKind.JSDocFunctionType:
                 return transformJSDocFunctionType(node as JSDocFunctionType);
-            case SyntaxKind.Parameter:
-                return transformJSDocParameter(node as ParameterDeclaration);
             case SyntaxKind.TypeReference:
                 return transformJSDocTypeReference(node as TypeReferenceNode);
             default:
-                const visited = visitEachChild(node, transformJSDocType, /*context*/ undefined) as TypeNode;
+                const visited = visitEachChild(node, transformJSDocType, /*context*/ undefined);
                 setEmitFlags(visited, EmitFlags.SingleLine);
                 return visited;
         }
@@ -145,8 +117,7 @@ namespace ts.codefix {
     }
 
     function transformJSDocFunctionType(node: JSDocFunctionType) {
-        const parameters = node.parameters && node.parameters.map(transformJSDocType);
-        return createFunctionTypeNode(emptyArray, parameters as ParameterDeclaration[], node.type);
+        return createFunctionTypeNode(emptyArray, node.parameters.map(transformJSDocParameter), node.type);
     }
 
     function transformJSDocParameter(node: ParameterDeclaration) {

src/services/textChanges.ts

@@ -327,6 +327,10 @@ namespace ts.textChanges {
             return this;
         }
 
+        private insertNodesAt(sourceFile: SourceFile, pos: number, newNodes: ReadonlyArray<Node>, options: InsertNodeOptions = {}): void {
+            this.changes.push({ kind: ChangeKind.ReplaceWithMultipleNodes, sourceFile, options, nodes: newNodes, range: { pos, end: pos } });
+        }
+
         public insertNodeAtTopOfFile(sourceFile: SourceFile, newNode: Statement, blankLineBetween: boolean): void {
             const pos = getInsertionPositionAtSourceFileTop(sourceFile);
             this.insertNodeAt(sourceFile, pos, newNode, {
@@ -353,6 +357,11 @@ namespace ts.textChanges {
             this.insertNodeAt(sourceFile, end, type, { prefix: ": " });
         }
 
+        public insertTypeParameters(sourceFile: SourceFile, node: SignatureDeclaration, typeParameters: ReadonlyArray<TypeParameterDeclaration>): void {
+            const lparen = findChildOfKind(node, SyntaxKind.OpenParenToken, sourceFile)!.pos;
+            this.insertNodesAt(sourceFile, lparen, typeParameters, { prefix: "<", suffix: ">" });
+        }
+
         private getOptionsForInsertNodeBefore(before: Node, doubleNewlines: boolean): ChangeNodeOptions {
             if (isStatement(before) || isClassElement(before)) {
                 return { suffix: doubleNewlines ? this.newLineCharacter + this.newLineCharacter : this.newLineCharacter };

tests/cases/fourslash/annotateWithTypeFromJSDoc13.ts

@@ -9,6 +9,6 @@ verify.codeFix({
     newFileContent:
 `class C {
     /** @return {number} */
-    get c(): number { return 12; }
+    get c(): number { return 12 }
 }`,
 });

tests/cases/fourslash/annotateWithTypeFromJSDoc18.ts

@@ -9,6 +9,6 @@ verify.codeFix({
     newFileContent:
 `class C {
     /** @param {number} value */
-    set c(value: number) { return 12; }
+    set c(value: number) { return 12 }
 }`,
 });

tests/cases/fourslash/annotateWithTypeFromJSDoc5.ts

@@ -10,6 +10,6 @@ verify.codeFix({
     newFileContent:
 `class C {
     /** @type {number | null} */
-    p: number | null = null;
+    p: number | null = null
 }`,
 });

tests/cases/fourslash/annotateWithTypeFromJSDoc8.ts

@@ -14,6 +14,6 @@ verify.codeFix({
  * @param {number} x
  * @returns {number}
  */
-var f = function(x: number): number {
+var f = function (x: number): number {
 }`,
 });