inferFromUsage codefix now emits JSDoc in JS files

src/compiler/transformers/declarations/diagnostics.ts

@@ -466,4 +466,4 @@ namespace ts {
             };
         }
     }
-}
+}

src/harness/fourslash.ts

@@ -2516,10 +2516,10 @@ Actual: ${stringify(fullActual)}`);
          * @param expectedContents The contents of the file after the fixes are applied.
          * @param fileName The file to check. If not supplied, the current open file is used.
          */
-        public verifyFileAfterCodeFix(expectedContents: string, fileName?: string) {
+        public verifyFileAfterCodeFix(expectedContents: string, fileName?: string, index?: number) {
             fileName = fileName ? fileName : this.activeFile.fileName;
 
-            this.applyCodeActions(this.getCodeFixes(fileName));
+            this.applyCodeActions(this.getCodeFixes(fileName), index);
 
             const actualContents: string = this.getFileContent(fileName);
             if (this.removeWhitespace(actualContents) !== this.removeWhitespace(expectedContents)) {
@@ -4388,6 +4388,10 @@ namespace FourSlashInterface {
             this.state.verifyRangeAfterCodeFix(expectedText, includeWhiteSpace, errorCode, index);
         }
 
+        public fileAfterCodeFix(expectedContents: string, fileName?: string, index?: number) {
+            this.state.verifyFileAfterCodeFix(expectedContents, fileName, index);
+        }
+
         public codeFixAll(options: VerifyCodeFixAllOptions): void {
             this.state.verifyCodeFixAll(options);
         }

src/services/codefixes/inferFromUsage.ts

@@ -26,9 +26,6 @@ namespace ts.codefix {
         errorCodes,
         getCodeActions(context) {
             const { sourceFile, program, span: { start }, errorCode, cancellationToken } = context;
-            if (isSourceFileJS(sourceFile)) {
-                return undefined; // TODO: GH#20113
-            }
 
             const token = getTokenAtPosition(sourceFile, start);
             let declaration!: Declaration | undefined;
@@ -50,7 +47,7 @@ namespace ts.codefix {
     function getDiagnostic(errorCode: number, token: Node): DiagnosticMessage {
         switch (errorCode) {
             case Diagnostics.Parameter_0_implicitly_has_an_1_type.code:
-                return isSetAccessor(getContainingFunction(token)!) ? Diagnostics.Infer_type_of_0_from_usage : Diagnostics.Infer_parameter_types_from_usage; // TODO: GH#18217
+                return isSetAccessorDeclaration(getContainingFunction(token)!) ? Diagnostics.Infer_type_of_0_from_usage : Diagnostics.Infer_parameter_types_from_usage; // TODO: GH#18217
             case Diagnostics.Rest_parameter_0_implicitly_has_an_any_type.code:
                 return Diagnostics.Infer_parameter_types_from_usage;
             default:
@@ -59,7 +56,7 @@ namespace ts.codefix {
     }
 
     function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, token: Node, errorCode: number, program: Program, cancellationToken: CancellationToken, markSeen: NodeSeenTracker): Declaration | undefined {
-        if (!isParameterPropertyModifier(token.kind) && token.kind !== SyntaxKind.Identifier && token.kind !== SyntaxKind.DotDotDotToken) {
+        if (!isParameterPropertyModifier(token.kind) && token.kind !== SyntaxKind.Identifier && token.kind !== SyntaxKind.DotDotDotToken && token.kind !== SyntaxKind.ThisKeyword) {
             return undefined;
         }
 
@@ -72,6 +69,14 @@ namespace ts.codefix {
                     annotateVariableDeclaration(changes, sourceFile, parent, program, cancellationToken);
                     return parent;
                 }
+                if (isPropertyAccessExpression(parent)) {
+                    const type = inferTypeForVariableFromUsage(parent.name, program, cancellationToken);
+                    const typeNode = type && getTypeNodeIfAccessible(type, parent, program.getTypeChecker());
+                    if (typeNode) {
+                        changes.tryInsertJSDocType(sourceFile, parent, typeNode);
+                    }
+                    return parent;
+                }
                 return undefined;
 
             case Diagnostics.Variable_0_implicitly_has_an_1_type.code: {
@@ -92,7 +97,7 @@ namespace ts.codefix {
         switch (errorCode) {
             // Parameter declarations
             case Diagnostics.Parameter_0_implicitly_has_an_1_type.code:
-                if (isSetAccessor(containingFunction)) {
+                if (isSetAccessorDeclaration(containingFunction)) {
                     annotateSetAccessor(changes, sourceFile, containingFunction, program, cancellationToken);
                     return containingFunction;
                 }
@@ -108,15 +113,15 @@ namespace ts.codefix {
             // Get Accessor declarations
             case Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation.code:
             case Diagnostics._0_which_lacks_return_type_annotation_implicitly_has_an_1_return_type.code:
-                if (isGetAccessor(containingFunction) && isIdentifier(containingFunction.name)) {
+                if (isGetAccessorDeclaration(containingFunction) && isIdentifier(containingFunction.name)) {
                     annotate(changes, sourceFile, containingFunction, inferTypeForVariableFromUsage(containingFunction.name, program, cancellationToken), program);
                     return containingFunction;
                 }
                 return undefined;
 
             // Set Accessor declarations
             case Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation.code:
-                if (isSetAccessor(containingFunction)) {
+                if (isSetAccessorDeclaration(containingFunction)) {
                     annotateSetAccessor(changes, sourceFile, containingFunction, program, cancellationToken);
                     return containingFunction;
                 }
@@ -150,35 +155,67 @@ namespace ts.codefix {
             return;
         }
 
-        const types = inferTypeForParametersFromUsage(containingFunction, sourceFile, program, cancellationToken) ||
-            containingFunction.parameters.map(p => isIdentifier(p.name) ? inferTypeForVariableFromUsage(p.name, program, cancellationToken) : undefined);
+        const symbols = inferTypeForParametersFromUsage(containingFunction, sourceFile, program, cancellationToken) ||
+            containingFunction.parameters.map(p => ({
+                declaration: p,
+                type: isIdentifier(p.name) ? inferTypeForVariableFromUsage(p.name, program, cancellationToken) : undefined
+            } as ParameterInference));
         // We didn't actually find a set of type inference positions matching each parameter position
-        if (!types || containingFunction.parameters.length !== types.length) {
+        if (containingFunction.parameters.length !== symbols.length) {
             return;
         }
 
-        zipWith(containingFunction.parameters, types, (parameter, type) => {
-            if (!parameter.type && !parameter.initializer) {
-                annotate(changes, sourceFile, parameter, type, program);
+        if (isInJSFile(containingFunction)) {
+            annotateJSDocParameters(changes, sourceFile, symbols, program);
+        }
+        else {
+            for (const { declaration, type } of symbols) {
+                if (declaration && !declaration.type && !declaration.initializer) {
+                    annotate(changes, sourceFile, declaration, type, program);
+                }
             }
-        });
+        }
     }
 
     function annotateSetAccessor(changes: textChanges.ChangeTracker, sourceFile: SourceFile, setAccessorDeclaration: SetAccessorDeclaration, program: Program, cancellationToken: CancellationToken): void {
         const param = firstOrUndefined(setAccessorDeclaration.parameters);
         if (param && isIdentifier(setAccessorDeclaration.name) && isIdentifier(param.name)) {
             const type = inferTypeForVariableFromUsage(setAccessorDeclaration.name, program, cancellationToken) ||
                 inferTypeForVariableFromUsage(param.name, program, cancellationToken);
-            annotate(changes, sourceFile, param, type, program);
+            if (isInJSFile(setAccessorDeclaration)) {
+                annotateJSDocParameters(changes, sourceFile, [{ declaration: param, type }], program);
+            }
+            else {
+                annotate(changes, sourceFile, param, type, program);
+            }
         }
     }
 
     function annotate(changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: textChanges.TypeAnnotatable, type: Type | undefined, program: Program): void {
         const typeNode = type && getTypeNodeIfAccessible(type, declaration, program.getTypeChecker());
-        if (typeNode) changes.tryInsertTypeAnnotation(sourceFile, declaration, typeNode);
+        if (typeNode) {
+            if (isInJSFile(sourceFile) && declaration.kind !== SyntaxKind.PropertySignature) {
+                changes.tryInsertJSDocType(sourceFile, declaration, typeNode);
+            }
+            else {
+                changes.tryInsertTypeAnnotation(sourceFile, declaration, typeNode);
+            }
+        }
+    }
+
+    function annotateJSDocParameters(changes: textChanges.ChangeTracker, sourceFile: SourceFile, symbols: ParameterInference[], program: Program): void {
+        const result = [];
+        for (const symbol of symbols) {
+            const param = symbol.declaration;
+            const typeNode = symbol.type && getTypeNodeIfAccessible(symbol.type, param, program.getTypeChecker());
+            if (typeNode && !param.initializer && !getJSDocType(param)) {
+                result.push({ ...symbol, typeNode });
+            }
+        }
+        changes.tryInsertJSDocParameters(sourceFile, result);
     }
 
-    function getTypeNodeIfAccessible(type: Type, enclosingScope: Node, checker: TypeChecker): TypeNode | undefined {
+   function getTypeNodeIfAccessible(type: Type, enclosingScope: Node, checker: TypeChecker): TypeNode | undefined {
         let typeIsAccessible = true;
         const notAccessible = () => { typeIsAccessible = false; };
         const res = checker.typeToTypeNode(type, enclosingScope, /*flags*/ undefined, {
@@ -203,7 +240,7 @@ namespace ts.codefix {
         return InferFromReference.inferTypeFromReferences(getReferences(token, program, cancellationToken), program.getTypeChecker(), cancellationToken);
     }
 
-    function inferTypeForParametersFromUsage(containingFunction: FunctionLikeDeclaration, sourceFile: SourceFile, program: Program, cancellationToken: CancellationToken): (Type | undefined)[] | undefined {
+    function inferTypeForParametersFromUsage(containingFunction: FunctionLikeDeclaration, sourceFile: SourceFile, program: Program, cancellationToken: CancellationToken): ParameterInference[] | undefined {
         switch (containingFunction.kind) {
             case SyntaxKind.Constructor:
             case SyntaxKind.FunctionExpression:
@@ -219,6 +256,13 @@ namespace ts.codefix {
         }
     }
 
+    export interface ParameterInference {
+        declaration: ParameterDeclaration;
+        type?: Type;
+        typeNode?: TypeNode;
+        isOptional?: boolean;
+    }
+
     namespace InferFromReference {
         interface CallContext {
             argumentTypes: Type[];
@@ -246,7 +290,7 @@ namespace ts.codefix {
             return getTypeFromUsageContext(usageContext, checker);
         }
 
-        export function inferTypeForParametersFromReferences(references: ReadonlyArray<Identifier>, declaration: FunctionLikeDeclaration, checker: TypeChecker, cancellationToken: CancellationToken): (Type | undefined)[] | undefined {
+        export function inferTypeForParametersFromReferences(references: ReadonlyArray<Identifier>, declaration: FunctionLikeDeclaration, checker: TypeChecker, cancellationToken: CancellationToken): ParameterInference[] | undefined {
             if (references.length === 0) {
                 return undefined;
             }
@@ -262,11 +306,13 @@ namespace ts.codefix {
             }
             const isConstructor = declaration.kind === SyntaxKind.Constructor;
             const callContexts = isConstructor ? usageContext.constructContexts : usageContext.callContexts;
+            let isOptional = false;
             return callContexts && declaration.parameters.map((parameter, parameterIndex) => {
                 const types: Type[] = [];
                 const isRest = isRestParameter(parameter);
                 for (const callContext of callContexts) {
                     if (callContext.argumentTypes.length <= parameterIndex) {
+                        isOptional = isInJSFile(declaration);
                         continue;
                     }
 
@@ -280,10 +326,14 @@ namespace ts.codefix {
                     }
                 }
                 if (!types.length) {
-                    return undefined;
+                    return { declaration: parameter };
                 }
                 const type = checker.getWidenedType(checker.getUnionType(types, UnionReduction.Subtype));
-                return isRest ? checker.createArrayType(type) : type;
+                return {
+                    type: isRest ? checker.createArrayType(type) : type,
+                    isOptional: isOptional && !isRest,
+                    declaration: parameter
+                };
             });
         }
 

src/services/textChanges.ts

@@ -339,6 +339,12 @@ namespace ts.textChanges {
             this.insertText(sourceFile, token.getStart(sourceFile), text);
         }
 
+        public insertCommentThenNewline(sourceFile: SourceFile, character: number, position: number, commentText: string): void {
+            const token = getTouchingToken(sourceFile, position);
+            const text = "/**" + commentText + "*/" + this.newLineCharacter + this.repeatString(" ", character);
+            this.insertText(sourceFile, token.getStart(sourceFile), text);
+        }
+
         public replaceRangeWithText(sourceFile: SourceFile, range: TextRange, text: string) {
             this.changes.push({ kind: ChangeKind.Text, sourceFile, range, text });
         }
@@ -347,6 +353,23 @@ namespace ts.textChanges {
             this.replaceRangeWithText(sourceFile, createRange(pos), text);
         }
 
+        public tryInsertJSDocParameters(sourceFile: SourceFile, parameters: codefix.ParameterInference[]) {
+            if (parameters.length === 0) {
+                return;
+            }
+            const parent = parameters[0].declaration.parent;
+            const indent = getLineAndCharacterOfPosition(sourceFile, parent.getStart()).character;
+            let commentText = "\n";
+            for (const { declaration, typeNode, isOptional } of parameters) {
+                if (isIdentifier(declaration.name)) {
+                    const printed = createPrinter().printNode(EmitHint.Unspecified, typeNode!, sourceFile);
+                    commentText += this.printJSDocParameter(indent, printed, declaration.name, isOptional);
+                }
+            }
+            commentText += this.repeatString(" ", indent + 1);
+            this.insertCommentThenNewline(sourceFile, indent, parent.getStart(), commentText);
+        }
+
         /** Prefer this over replacing a node with another that has a type annotation, as it avoids reformatting the other parts of the node. */
         public tryInsertTypeAnnotation(sourceFile: SourceFile, node: TypeAnnotatable, type: TypeNode): void {
             let endNode: Node | undefined;
@@ -365,6 +388,36 @@ namespace ts.textChanges {
             this.insertNodeAt(sourceFile, endNode.end, type, { prefix: ": " });
         }
 
+        public tryInsertJSDocType(sourceFile: SourceFile, node: Node, type: TypeNode): void {
+            const printed = createPrinter().printNode(EmitHint.Unspecified, type, sourceFile);
+            let commentText;
+            if (isGetAccessorDeclaration(node)) {
+                commentText = ` @return {${printed}} `;
+            }
+            else {
+                commentText = ` @type {${printed}} `;
+                node = node.parent;
+            }
+            this.insertCommentThenNewline(sourceFile, getLineAndCharacterOfPosition(sourceFile, node.getStart()).character, node.getStart(), commentText);
+        }
+
+        /** Should only be used to repeat strings a small number of times */
+        private repeatString(s: string, n: number) {
+            let sum = "";
+            for (let i = 0; i < n; i++) {
+                sum += s;
+            }
+            return sum;
+        }
+
+        private printJSDocParameter(indent: number, printed: string, name: Identifier, isOptionalParameter: boolean | undefined) {
+            let printName = unescapeLeadingUnderscores(name.escapedText);
+            if (isOptionalParameter) {
+                printName = `[${printName}]`;
+            }
+            return this.repeatString(" ", indent) + ` * @param {${printed}} ${printName}\n`;
+        }
+
         public insertTypeParameters(sourceFile: SourceFile, node: SignatureDeclaration, typeParameters: ReadonlyArray<TypeParameterDeclaration>): void {
             // If no `(`, is an arrow function `x => x`, so use the pos of the first parameter
             const start = (findChildOfKind(node, SyntaxKind.OpenParenToken, sourceFile) || first(node.parameters)).getStart(sourceFile);

tests/cases/fourslash/codeFixInferFromUsageCallJS.ts

@@ -0,0 +1,20 @@
+/// <reference path='fourslash.ts' />
+
+// @allowJs: true
+// @checkJs: true
+// @noImplicitAny: true
+// @Filename: test.js
+////function wat(b) {
+////    b();
+////}
+
+verify.codeFixAll({
+    fixId: "inferFromUsage",
+    fixAllDescription: "Infer all types from usage",
+    newFileContent:
+`/**
+ * @param {() => void} b
+ */
+function wat(b) {
+    b();
+}`});

tests/cases/fourslash/codeFixInferFromUsageJS.ts

@@ -0,0 +1,12 @@
+/// <reference path='fourslash.ts' />
+
+// @allowJs: true
+// @checkJs: true
+// @noImplicitAny: true
+// @Filename: test.js
+////[|var foo;|]
+////function f() {
+////    foo += 2;
+////}
+
+verify.rangeAfterCodeFix("/** @type {number} */\nvar foo;",/*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, 2);

tests/cases/fourslash/codeFixInferFromUsageMember2JS.ts

@@ -0,0 +1,14 @@
+/// <reference path='fourslash.ts' />
+
+// @allowJs: true
+// @checkJs: true
+// @noImplicitAny: true
+// @Filename: important.js
+
+/////** @typedef {{ [|p |]}} I */
+/////** @type {I} */
+////var i;
+////i.p = 0;
+
+
+verify.rangeAfterCodeFix("p: number", undefined, undefined, 1);

tests/cases/fourslash/codeFixInferFromUsageMemberJS.ts

@@ -0,0 +1,30 @@
+/// <reference path='fourslash.ts' />
+
+// @allowJs: true
+// @checkJs: true
+// @noImplicitAny: true
+// @strictNullChecks: true
+// @Filename: important.js
+////class C {
+////    constructor() {
+////        [|this.p|] = undefined;
+////    }
+////    method() {
+////        this.p.push(1)
+////    }
+////}
+
+
+// Note: Should be number[] | undefined, but inference currently privileges assignments
+// over usage (even when the only result is undefined) and infers only undefined.
+verify.fileAfterCodeFix(
+`class C {
+    constructor() {
+        /** @type {undefined} */
+        this.p = undefined;
+    }
+    method() {
+        this.p.push(1)
+    }
+}
+`, undefined, 2);