Convert annotateWithTypeFromJSDoc refactor to a codefix

src/compiler/diagnosticMessages.json

@@ -3830,6 +3830,10 @@
         "category": "Suggestion",
         "code": 80003
     },
+    "JSDoc types may be moved to TypeScript types.": {
+        "category": "Suggestion",
+        "code": 80004
+    },
 
     "Add missing 'super()' call": {
         "category": "Message",

src/services/refactors/annotateWithTypeFromJSDoc.ts → src/services/codefixes/annotateWithTypeFromJSDoc.ts

@@ -1,108 +1,66 @@
 /* @internal */
-namespace ts.refactor.annotateWithTypeFromJSDoc {
-    const refactorName = "Annotate with type from JSDoc";
-    const actionName = "annotate";
-    const description = Diagnostics.Annotate_with_type_from_JSDoc.message;
-    registerRefactor(refactorName, { getEditsForAction, getAvailableActions });
+namespace ts.codefix {
+    const fixId = "annotateWithTypeFromJSDoc";
+    const errorCodes = [Diagnostics.JSDoc_types_may_be_moved_to_TypeScript_types.code];
+    registerCodeFix({
+        errorCodes,
+        getCodeActions(context) {
+            const decl = getDeclaration(context.sourceFile, context.span.start);
+            if (!decl) return;
+            const description = getLocaleSpecificMessage(Diagnostics.Annotate_with_type_from_JSDoc);
+            const changes = textChanges.ChangeTracker.with(context, t => doChange(t, context.sourceFile, decl));
+            return [{ description, changes, fixId }];
+        },
+        fixIds: [fixId],
+        getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => {
+            const decl = getDeclaration(diag.file!, diag.start!);
+            if (decl) doChange(changes, diag.file!, decl);
+        }),
+    });
+
+    function getDeclaration(file: SourceFile, pos: number): DeclarationWithType | undefined {
+        const name = getTokenAtPosition(file, pos, /*includeJsDocComment*/ false);
+        // For an arrow function with no name, 'name' lands on the first parameter.
+        return tryCast(isParameter(name.parent) ? name.parent.parent : name.parent, parameterShouldGetTypeFromJSDoc);
+    }
 
     type DeclarationWithType =
         | FunctionLikeDeclaration
         | VariableDeclaration
-        | ParameterDeclaration
         | PropertySignature
         | PropertyDeclaration;
 
-    function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined {
-        if (isInJavaScriptFile(context.file)) {
-            return undefined;
-        }
-
-        const node = getTokenAtPosition(context.file, context.startPosition, /*includeJsDocComment*/ false);
-        if (hasUsableJSDoc(findAncestor(node, isDeclarationWithType))) {
-            return [{
-                name: refactorName,
-                description,
-                actions: [
-                    {
-                        description,
-                        name: actionName
-                    }
-                ]
-            }];
-        }
+    export function parameterShouldGetTypeFromJSDoc(node: Node): node is DeclarationWithType {
+        return isDeclarationWithType(node) && hasUsableJSDoc(node);
     }
 
-    function hasUsableJSDoc(decl: DeclarationWithType): boolean {
-        if (!decl) {
-            return false;
-        }
-        if (isFunctionLikeDeclaration(decl)) {
-            return decl.parameters.some(hasUsableJSDoc) || (!decl.type && !!getJSDocReturnType(decl));
-        }
-        return !decl.type && !!getJSDocType(decl);
+    function hasUsableJSDoc(decl: DeclarationWithType | ParameterDeclaration): boolean {
+        return isFunctionLikeDeclaration(decl)
+            ? decl.parameters.some(hasUsableJSDoc) || (!decl.type && !!getJSDocReturnType(decl))
+            : !decl.type && !!getJSDocType(decl);
     }
 
-    function getEditsForAction(context: RefactorContext, action: string): RefactorEditInfo | undefined {
-        if (actionName !== action) {
-            return Debug.fail(`actionName !== action: ${actionName} !== ${action}`);
-        }
-        const node = getTokenAtPosition(context.file, context.startPosition, /*includeJsDocComment*/ false);
-        const decl = findAncestor(node, isDeclarationWithType);
-        if (!decl || decl.type) {
-            return undefined;
-        }
-        const jsdocType = getJSDocType(decl);
-        const isFunctionWithJSDoc = isFunctionLikeDeclaration(decl) && (getJSDocReturnType(decl) || decl.parameters.some(p => !!getJSDocType(p)));
-        if (isFunctionWithJSDoc || jsdocType && decl.kind === SyntaxKind.Parameter) {
-            return getEditsForFunctionAnnotation(context);
-        }
-        else if (jsdocType) {
-            return getEditsForAnnotation(context);
+    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;
         }
         else {
-            Debug.assert(!!refactor, "No applicable refactor found.");
-        }
-    }
-
-    function getEditsForAnnotation(context: RefactorContext): RefactorEditInfo | undefined {
-        const sourceFile = context.file;
-        const token = getTokenAtPosition(sourceFile, context.startPosition, /*includeJsDocComment*/ false);
-        const decl = findAncestor(token, isDeclarationWithType);
-        const jsdocType = getJSDocType(decl);
-        if (!decl || !jsdocType || decl.type) {
-            return Debug.fail(`!decl || !jsdocType || decl.type: !${decl} || !${jsdocType} || ${decl.type}`);
+            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);
         }
-
-        const changeTracker = textChanges.ChangeTracker.fromContext(context);
-        const declarationWithType = addType(decl, transformJSDocType(jsdocType) as TypeNode);
-        suppressLeadingAndTrailingTrivia(declarationWithType);
-        changeTracker.replaceNode(sourceFile, decl, declarationWithType, textChanges.useNonAdjustedPositions);
-        return {
-            edits: changeTracker.getChanges(),
-            renameFilename: undefined,
-            renameLocation: undefined
-        };
-    }
-
-    function getEditsForFunctionAnnotation(context: RefactorContext): RefactorEditInfo | undefined {
-        const sourceFile = context.file;
-        const token = getTokenAtPosition(sourceFile, context.startPosition, /*includeJsDocComment*/ false);
-        const decl = findAncestor(token, isFunctionLikeDeclaration);
-        const changeTracker = textChanges.ChangeTracker.fromContext(context);
-        const functionWithType = addTypesToFunctionLike(decl);
-        suppressLeadingAndTrailingTrivia(functionWithType);
-        changeTracker.replaceNode(sourceFile, decl, functionWithType, textChanges.useNonAdjustedPositions);
-        return {
-            edits: changeTracker.getChanges(),
-            renameFilename: undefined,
-            renameLocation: undefined
-        };
     }
 
     function isDeclarationWithType(node: Node): node is DeclarationWithType {
         return isFunctionLikeDeclaration(node) ||
             node.kind === SyntaxKind.VariableDeclaration ||
-            node.kind === SyntaxKind.Parameter ||
             node.kind === SyntaxKind.PropertySignature ||
             node.kind === SyntaxKind.PropertyDeclaration;
     }

src/services/codefixes/fixes.ts

@@ -1,4 +1,5 @@
 /// <reference path="addMissingInvocationForDecorator.ts" />
+/// <reference path="annotateWithTypeFromJSDoc.ts" />
 /// <reference path="convertFunctionToEs6Class.ts" />
 /// <reference path="convertToEs6Module.ts" />
 /// <reference path="correctQualifiedNameToIndexedAccessType.ts" />

src/services/refactors/refactors.ts

@@ -1,2 +1 @@
-/// <reference path="annotateWithTypeFromJSDoc.ts" />
 /// <reference path="extractSymbol.ts" />

src/services/suggestionDiagnostics.ts

@@ -9,21 +9,28 @@ namespace ts {
             diags.push(createDiagnosticForNode(sourceFile.commonJsModuleIndicator, Diagnostics.File_is_a_CommonJS_module_it_may_be_converted_to_an_ES6_module));
         }
 
+        const isJsFile = isSourceFileJavaScript(sourceFile);
+
         function check(node: Node) {
             switch (node.kind) {
                 case SyntaxKind.FunctionDeclaration:
                 case SyntaxKind.FunctionExpression:
-                    const symbol = node.symbol;
-                    if (symbol.members && (symbol.members.size > 0)) {
-                        diags.push(createDiagnosticForNode(isVariableDeclaration(node.parent) ? node.parent.name : node, Diagnostics.This_constructor_function_may_be_converted_to_a_class_declaration));
+                    if (isJsFile) {
+                        const symbol = node.symbol;
+                        if (symbol.members && (symbol.members.size > 0)) {
+                            diags.push(createDiagnosticForNode(isVariableDeclaration(node.parent) ? node.parent.name : node, Diagnostics.This_constructor_function_may_be_converted_to_a_class_declaration));
+                        }
                     }
                     break;
             }
+
+            if (!isJsFile && codefix.parameterShouldGetTypeFromJSDoc(node)) {
+                diags.push(createDiagnosticForNode(node.name || node, Diagnostics.JSDoc_types_may_be_moved_to_TypeScript_types));
+            }
+
             node.forEachChild(check);
         }
-        if (isInJavaScriptFile(sourceFile)) {
-            check(sourceFile);
-        }
+        check(sourceFile);
 
         if (getAllowSyntheticDefaultImports(program.getCompilerOptions())) {
             for (const importNode of sourceFile.imports) {

tests/cases/fourslash/annotateWithTypeFromJSDoc1.ts

@@ -2,9 +2,16 @@
 
 // @Filename: test123.ts
 /////** @type {number} */
-////var /*1*/x;
+////var [|x|];
 
-verify.applicableRefactorAvailableAtMarker('1');
-verify.fileAfterApplyingRefactorAtMarker('1',
+verify.getSuggestionDiagnostics([{
+    message: "JSDoc types may be moved to TypeScript types.",
+    code: 80004,
+}]);
+
+verify.codeFix({
+    description: "Annotate with type from JSDoc",
+    newFileContent:
 `/** @type {number} */
-var x: number;`, 'Annotate with type from JSDoc', 'annotate');
+var x: number;`,
+});

tests/cases/fourslash/annotateWithTypeFromJSDoc10.ts

@@ -4,12 +4,14 @@
 //// * @param {?} x
 //// * @returns {number}
 //// */
-////var f = /*1*/(/*2*/x) => x
+////var f = (x) => x
 
-verify.applicableRefactorAvailableAtMarker('1');
-verify.fileAfterApplyingRefactorAtMarker('1',
+verify.codeFix({
+    description: "Annotate with type from JSDoc",
+    newFileContent:
 `/**
  * @param {?} x
  * @returns {number}
  */
-var f = (x: any): number => x`, 'Annotate with type from JSDoc', 'annotate');
+var f = (x: any): number => x`,
+});

tests/cases/fourslash/annotateWithTypeFromJSDoc11.ts

@@ -4,12 +4,14 @@
 //// * @param {?} x
 //// * @returns {number}
 //// */
-////var f = /*1*/(/*2*/x) => x
+////var f = (x) => x
 
-verify.applicableRefactorAvailableAtMarker('2');
-verify.fileAfterApplyingRefactorAtMarker('2',
+verify.codeFix({
+    description: "Annotate with type from JSDoc",
+    newFileContent:
 `/**
  * @param {?} x
  * @returns {number}
  */
-var f = (x: any): number => x`, 'Annotate with type from JSDoc', 'annotate');
+var f = (x: any): number => x`,
+});

tests/cases/fourslash/annotateWithTypeFromJSDoc12.ts

@@ -4,15 +4,18 @@
 ////    /**
 ////     * @return {...*}
 ////     */
-////    /*1*/m(x) {
+////    m(x) {
 ////    }
 ////}
-verify.applicableRefactorAvailableAtMarker('1');
-verify.fileAfterApplyingRefactorAtMarker('1',
+
+verify.codeFix({
+    description: "Annotate with type from JSDoc",
+    newFileContent:
 `class C {
     /**
      * @return {...*}
      */
     m(x): any[] {
     }
-}`, 'Annotate with type from JSDoc', 'annotate');
+}`,
+});

tests/cases/fourslash/annotateWithTypeFromJSDoc13.ts

@@ -1,11 +1,14 @@
 /// <reference path='fourslash.ts' />
 ////class C {
 ////    /** @return {number} */
-////    get /*1*/c() { return 12 }
+////    get c() { return 12 }
 ////}
-verify.applicableRefactorAvailableAtMarker('1');
-verify.fileAfterApplyingRefactorAtMarker('1',
+
+verify.codeFix({
+    description: "Annotate with type from JSDoc",
+    newFileContent:
 `class C {
     /** @return {number} */
     get c(): number { return 12; }
-}`, 'Annotate with type from JSDoc', 'annotate');
+}`,
+});

tests/cases/fourslash/annotateWithTypeFromJSDoc14.ts

@@ -1,11 +1,14 @@
 /// <reference path='fourslash.ts' />
 /////** @return {number} */
 ////function f() {
-////    /*1*/return 12;
+////    return 12;
 ////}
-verify.applicableRefactorAvailableAtMarker('1');
-verify.fileAfterApplyingRefactorAtMarker('1',
+
+verify.codeFix({
+    description: "Annotate with type from JSDoc",
+    newFileContent:
 `/** @return {number} */
 function f(): number {
     return 12;
-}`, 'Annotate with type from JSDoc', 'annotate');
+}`,
+});

tests/cases/fourslash/annotateWithTypeFromJSDoc15.ts

@@ -11,10 +11,12 @@
 //// * @param {Array<number>} epsilon
 //// * @param {promise<String>} zeta
 //// */
-////function f(/*1*/x, /*2*/y, /*3*/z, /*4*/alpha, /*5*/beta, /*6*/gamma, /*7*/delta, /*8*/epsilon, /*9*/zeta) {
+////function f(x, y, z, alpha, beta, gamma, delta, epsilon, zeta) {
 ////}
-verify.applicableRefactorAvailableAtMarker('9');
-verify.fileAfterApplyingRefactorAtMarker('9',
+
+verify.codeFix({
+    description: "Annotate with type from JSDoc",
+    newFileContent:
 `/**
  * @param {Boolean} x
  * @param {String} y
@@ -27,4 +29,5 @@ verify.fileAfterApplyingRefactorAtMarker('9',
  * @param {promise<String>} zeta
  */
 function f(x: boolean, y: string, z: number, alpha: object, beta: Date, gamma: Promise<any>, delta: Array<any>, epsilon: Array<number>, zeta: Promise<string>) {
-}`, 'Annotate with type from JSDoc', 'annotate');
+}`,
+});

tests/cases/fourslash/annotateWithTypeFromJSDoc16.ts

@@ -1,9 +1,11 @@
 /// <reference path='fourslash.ts' />
 // @strict: true
 /////** @type {function(*, ...number, ...boolean): void} */
-////var /*1*/x = (x, ys, ...zs) => { };
+////var x = (x, ys, ...zs) => { };
 
-verify.applicableRefactorAvailableAtMarker('1');
-verify.fileAfterApplyingRefactorAtMarker('1',
+verify.codeFix({
+    description: "Annotate with type from JSDoc",
+    newFileContent:
 `/** @type {function(*, ...number, ...boolean): void} */
-var x: (arg0: any, arg1: number[], ...rest: boolean[]) => void = (x, ys, ...zs) => { };`, 'Annotate with type from JSDoc', 'annotate');
+var x: (arg0: any, arg1: number[], ...rest: boolean[]) => void = (x, ys, ...zs) => { };`,
+});

tests/cases/fourslash/annotateWithTypeFromJSDoc17.ts

@@ -3,16 +3,18 @@
 ////    /**
 ////     * @param {number} x - the first parameter
 ////     */
-////    constructor(/*1*/x) {
+////    constructor(x) {
 ////    }
 ////}
-verify.applicableRefactorAvailableAtMarker('1');
-verify.fileAfterApplyingRefactorAtMarker('1',
+
+verify.codeFix({
+    description: "Annotate with type from JSDoc",
+    newFileContent:
 `class C {
     /**
      * @param {number} x - the first parameter
      */
     constructor(x: number) {
     }
-}`, 'Annotate with type from JSDoc', 'annotate');
-
+}`,
+});