inferFromUsage codefix now emits JSDoc in JS files (#27610)

* Now adding @type to variable declarations, at least

Probably everything else, but not as well.

* Improve @param output and add test

It's still bad, but at least it's not wrong.

* Add some js/inferFromUsage tests and fixes

Also, remove redundant is(Set|Get)Accessor functions.

* Fix @typedef refactor

* Emit JSDoc optional parameters

By surrounding the parameter name with brackets. It is super, super ugly
right now.

* Get rest of existing tests working

* Correct location of comments

* Handle @param blocks

1. Format multiple params nicely in a single-multiline block.
2. Generate only params that haven't already been documented. Existing
documentation is not touched.

* Re-add isGet/SetAccessor -- it is part of the API

* Move isSet/GetAccessor back to the original location

* Oh no I missed a newline and a space

* Switch to an object type

* A lot of cleanup

More to come, though. annotate is only called in
annotateVariableDeclaration where we don't know whether we're in JS or
not.

* Move and delegate to annotateJSDocParameters

* Address PR comments

* Lint: newline problems!!!!

* Switch another call to getNonformattedText

* Update baseline missed after merge

Author: sandersn

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, host } = 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, host: LanguageServiceHost): 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, host, cancellationToken);
                     return parent;
                 }
+                if (isPropertyAccessExpression(parent)) {
+                    const type = inferTypeForVariableFromUsage(parent.name, program, cancellationToken);
+                    const typeNode = type && getTypeNodeIfAccessible(type, parent, program, host);
+                    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, host, 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, host);
                     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, host, cancellationToken);
                     return containingFunction;
                 }
@@ -150,35 +155,61 @@ namespace ts.codefix {
             return;
         }
 
-        const types = inferTypeForParametersFromUsage(containingFunction, sourceFile, program, cancellationToken) ||
-            containingFunction.parameters.map(p => isIdentifier(p.name) ? inferTypeForVariableFromUsage(p.name, program, cancellationToken) : undefined);
-        // We didn't actually find a set of type inference positions matching each parameter position
-        if (!types || containingFunction.parameters.length !== types.length) {
-            return;
-        }
+        const parameterInferences = inferTypeForParametersFromUsage(containingFunction, sourceFile, program, cancellationToken) ||
+            containingFunction.parameters.map<ParameterInference>(p => ({
+                declaration: p,
+                type: isIdentifier(p.name) ? inferTypeForVariableFromUsage(p.name, program, cancellationToken) : undefined
+            }));
+        Debug.assert(containingFunction.parameters.length === parameterInferences.length);
 
-        zipWith(containingFunction.parameters, types, (parameter, type) => {
-            if (!parameter.type && !parameter.initializer) {
-                annotate(changes, sourceFile, parameter, type, program, host);
+        if (isInJSFile(containingFunction)) {
+            annotateJSDocParameters(changes, sourceFile, parameterInferences, program, host);
+        }
+        else {
+            for (const { declaration, type } of parameterInferences) {
+                if (declaration && !declaration.type && !declaration.initializer) {
+                    annotate(changes, sourceFile, declaration, type, program, host);
+                }
             }
-        });
+        }
     }
 
     function annotateSetAccessor(changes: textChanges.ChangeTracker, sourceFile: SourceFile, setAccessorDeclaration: SetAccessorDeclaration, program: Program, host: LanguageServiceHost, 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, host);
+            if (isInJSFile(setAccessorDeclaration)) {
+                annotateJSDocParameters(changes, sourceFile, [{ declaration: param, type }], program, host);
+            }
+            else {
+                annotate(changes, sourceFile, param, type, program, host);
+            }
         }
     }
 
     function annotate(changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: textChanges.TypeAnnotatable, type: Type | undefined, program: Program, host: LanguageServiceHost): void {
         const typeNode = type && getTypeNodeIfAccessible(type, declaration, program, host);
-        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 getTypeNodeIfAccessible(type: Type, enclosingScope: Node, program: Program, host: LanguageServiceHost): TypeNode | undefined {
+    function annotateJSDocParameters(changes: textChanges.ChangeTracker, sourceFile: SourceFile, parameterInferences: ParameterInference[], program: Program, host: LanguageServiceHost): void {
+        const result = mapDefined(parameterInferences, inference => {
+            const param = inference.declaration;
+            const typeNode = inference.type && getTypeNodeIfAccessible(inference.type, param, program, host);
+            return typeNode && !param.initializer && !getJSDocType(param) ? { ...inference, typeNode } : undefined;
+        });
+        changes.tryInsertJSDocParameters(sourceFile, result);
+    }
+
+   function getTypeNodeIfAccessible(type: Type, enclosingScope: Node, program: Program, host: LanguageServiceHost): TypeNode | undefined {
         const checker = program.getTypeChecker();
         let typeIsAccessible = true;
         const notAccessible = () => { typeIsAccessible = false; };
@@ -212,7 +243,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:
@@ -228,6 +259,13 @@ namespace ts.codefix {
         }
     }
 
+    interface ParameterInference {
+        declaration: ParameterDeclaration;
+        type?: Type;
+        typeNode?: TypeNode;
+        isOptional?: boolean;
+    }
+
     namespace InferFromReference {
         interface CallContext {
             argumentTypes: Type[];
@@ -255,7 +293,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;
             }
@@ -274,8 +312,10 @@ namespace ts.codefix {
             return callContexts && declaration.parameters.map((parameter, parameterIndex) => {
                 const types: Type[] = [];
                 const isRest = isRestParameter(parameter);
+                let isOptional = false;
                 for (const callContext of callContexts) {
                     if (callContext.argumentTypes.length <= parameterIndex) {
+                        isOptional = isInJSFile(declaration);
                         continue;
                     }
 
@@ -289,10 +329,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

@@ -209,6 +209,12 @@ namespace ts.textChanges {
 
     export type TypeAnnotatable = SignatureDeclaration | VariableDeclaration | ParameterDeclaration | PropertyDeclaration | PropertySignature;
 
+    interface JSDocParameter {
+        declaration: ParameterDeclaration;
+        typeNode: TypeNode;
+        isOptional?: boolean;
+    }
+
     export class ChangeTracker {
         private readonly changes: Change[] = [];
         private readonly newFiles: { readonly oldFile: SourceFile | undefined, readonly fileName: string, readonly statements: ReadonlyArray<Statement> }[] = [];
@@ -339,6 +345,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 + 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 +359,23 @@ namespace ts.textChanges {
             this.replaceRangeWithText(sourceFile, createRange(pos), text);
         }
 
+        public tryInsertJSDocParameters(sourceFile: SourceFile, parameters: JSDocParameter[]) {
+            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 = changesToText.getNonformattedText(typeNode, sourceFile, this.newLineCharacter).text;
+                    commentText += this.printJSDocParameter(indent, printed, declaration.name, isOptional);
+                }
+            }
+            commentText += 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 +394,27 @@ namespace ts.textChanges {
             this.insertNodeAt(sourceFile, endNode.end, type, { prefix: ": " });
         }
 
+        public tryInsertJSDocType(sourceFile: SourceFile, node: Node, type: TypeNode): void {
+            const printed = changesToText.getNonformattedText(type, sourceFile, this.newLineCharacter).text;
+            let commentText;
+            if (isGetAccessorDeclaration(node)) {
+                commentText = ` @return {${printed}} `;
+            }
+            else {
+                commentText = ` @type {${printed}} `;
+                node = node.parent;
+            }
+            this.insertCommentThenNewline(sourceFile, getLineAndCharacterOfPosition(sourceFile, node.getStart(sourceFile)).character, node.getStart(sourceFile), commentText);
+        }
+
+        private printJSDocParameter(indent: number, printed: string, name: Identifier, isOptionalParameter: boolean | undefined) {
+            let printName = unescapeLeadingUnderscores(name.escapedText);
+            if (isOptionalParameter) {
+                printName = `[${printName}]`;
+            }
+            return 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);
@@ -806,7 +856,7 @@ namespace ts.textChanges {
         }
 
         /** Note: output node may be mutated input node. */
-        function getNonformattedText(node: Node, sourceFile: SourceFile | undefined, newLineCharacter: string): { text: string, node: Node } {
+        export function getNonformattedText(node: Node, sourceFile: SourceFile | undefined, newLineCharacter: string): { text: string, node: Node } {
             const writer = new Writer(newLineCharacter);
             const newLine = newLineCharacter === "\n" ? NewLineKind.LineFeed : NewLineKind.CarriageReturnLineFeed;
             createPrinter({ newLine, neverAsciiEscape: true }, writer).writeNode(EmitHint.Unspecified, node, sourceFile, writer);

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);