feat(11378): check param names in JSDoc

src/compiler/checker.ts

@@ -34413,6 +34413,7 @@ namespace ts {
             }
 
             checkTypeParameters(getEffectiveTypeParameterDeclarations(node));
+            checkUnmatchedJSDocParameters(node);
 
             forEach(node.parameters, checkParameter);
 
@@ -36178,40 +36179,7 @@ namespace ts {
 
         function checkJSDocParameterTag(node: JSDocParameterTag) {
             checkSourceElement(node.typeExpression);
-            if (!getParameterSymbolFromJSDoc(node)) {
-                const decl = getHostSignatureFromJSDoc(node);
-                // don't issue an error for invalid hosts -- just functions --
-                // and give a better error message when the host function mentions `arguments`
-                // but the tag doesn't have an array type
-                if (decl) {
-                    const i = getJSDocTags(decl).filter(isJSDocParameterTag).indexOf(node);
-                    if (i > -1 && i < decl.parameters.length && isBindingPattern(decl.parameters[i].name)) {
-                        return;
-                    }
-                    if (!containsArgumentsReference(decl)) {
-                        if (isQualifiedName(node.name)) {
-                            error(node.name,
-                                Diagnostics.Qualified_name_0_is_not_allowed_without_a_leading_param_object_1,
-                                entityNameToString(node.name),
-                                entityNameToString(node.name.left));
-                        }
-                        else {
-                            error(node.name,
-                                Diagnostics.JSDoc_param_tag_has_name_0_but_there_is_no_parameter_with_that_name,
-                                idText(node.name));
-                        }
-                    }
-                    else if (findLast(getJSDocTags(decl), isJSDocParameterTag) === node &&
-                        node.typeExpression && node.typeExpression.type &&
-                        !isArrayType(getTypeFromTypeNode(node.typeExpression.type))) {
-                        error(node.name,
-                            Diagnostics.JSDoc_param_tag_has_name_0_but_there_is_no_parameter_with_that_name_It_would_match_arguments_if_it_had_an_array_type,
-                            idText(node.name.kind === SyntaxKind.QualifiedName ? node.name.right : node.name));
-                    }
-                }
-            }
         }
-
         function checkJSDocPropertyTag(node: JSDocPropertyTag) {
             checkSourceElement(node.typeExpression);
         }
@@ -38506,6 +38474,45 @@ namespace ts {
             }
         }
 
+        function checkUnmatchedJSDocParameters(node: SignatureDeclaration) {
+            const jsdocParameters = filter(getJSDocTags(node), isJSDocParameterTag);
+            if (length(jsdocParameters) === 0) return;
+
+            const isJs = isInJSFile(node);
+            const parameters = new Set<__String>();
+            const excludedParameters = new Set<number>();
+            forEach(node.parameters, ({ name }, index) => {
+                if (isIdentifier(name)) {
+                    parameters.add(name.escapedText);
+                }
+                if (isBindingPattern(name)) {
+                    excludedParameters.add(index);
+                }
+            });
+
+            const containsArguments = containsArgumentsReference(node);
+            if (containsArguments) {
+                const lastJSDocParam = lastOrUndefined(jsdocParameters);
+                if (lastJSDocParam && isIdentifier(lastJSDocParam.name) && lastJSDocParam.typeExpression &&
+                    lastJSDocParam.typeExpression.type && !parameters.has(lastJSDocParam.name.escapedText) && !isArrayType(getTypeFromTypeNode(lastJSDocParam.typeExpression.type))) {
+                    errorOrSuggestion(isJs, lastJSDocParam.name, Diagnostics.JSDoc_param_tag_has_name_0_but_there_is_no_parameter_with_that_name_It_would_match_arguments_if_it_had_an_array_type, idText(lastJSDocParam.name));
+                }
+            }
+            else {
+                forEach(jsdocParameters, ({ name }, index) => {
+                    if (excludedParameters.has(index) || isIdentifier(name) && parameters.has(name.escapedText)) {
+                        return;
+                    }
+                    if (isQualifiedName(name)) {
+                        errorOrSuggestion(isJs, name, Diagnostics.Qualified_name_0_is_not_allowed_without_a_leading_param_object_1, entityNameToString(name), entityNameToString(name.left));
+                    }
+                    else {
+                        errorOrSuggestion(isJs, name, Diagnostics.JSDoc_param_tag_has_name_0_but_there_is_no_parameter_with_that_name, idText(name));
+                    }
+                });
+            }
+        }
+
         /**
          * Check each type parameter and check that type parameters have no duplicate type parameter declarations
          */

src/compiler/diagnosticMessages.json

@@ -7135,6 +7135,18 @@
         "category": "Message",
         "code": 95169
     },
+    "Remove unused parameter '{0}'": {
+        "category": "Message",
+        "code": 95170
+    },
+    "Remove all unused parameters": {
+        "category": "Message",
+        "code": 95171
+    },
+    "Rename parameter '{0}' to '{1}'": {
+        "category": "Message",
+        "code": 95172
+    },
 
     "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
         "category": "Error",

src/services/codefixes/fixUnmatchedParameter.ts

@@ -0,0 +1,89 @@
+/* @internal */
+namespace ts.codefix {
+    const deleteUnmatchedParameter = "deleteUnmatchedParameter";
+    const renameUnmatchedParameter = "renameUnmatchedParameter";
+
+    const errorCodes = [
+        Diagnostics.JSDoc_param_tag_has_name_0_but_there_is_no_parameter_with_that_name.code,
+    ];
+
+    registerCodeFix({
+        fixIds: [deleteUnmatchedParameter, renameUnmatchedParameter],
+        errorCodes,
+        getCodeActions: function getCodeActionsToFixUnmatchedParameter(context) {
+            const { sourceFile, span } = context;
+            const actions: CodeFixAction[] = [];
+            const info = getInfo(sourceFile, span.start);
+            if (info) {
+                append(actions, getDeleteAction(context, info));
+                append(actions, getRenameAction(context, info));
+                return actions;
+            }
+            return undefined;
+        },
+        getAllCodeActions: function getAllCodeActionsToFixUnmatchedParameter(context) {
+            const tagsToSignature = new Map<SignatureDeclaration, JSDocTag[]>();
+            return createCombinedCodeActions(textChanges.ChangeTracker.with(context, changes => {
+                eachDiagnostic(context, errorCodes, ({ file, start }) => {
+                    const info = getInfo(file, start);
+                    if (info) {
+                        tagsToSignature.set(info.signature, append(tagsToSignature.get(info.signature), info.jsDocParameterTag));
+                    }
+                });
+
+                tagsToSignature.forEach((tags, signature) => {
+                    const tagsSet = new Set(tags);
+                    if (context.fixId === deleteUnmatchedParameter) {
+                        changes.filterJSDocTags(signature.getSourceFile(), signature, t => !tagsSet.has(t));
+                    }
+                });
+            }));
+        }
+    });
+
+    function getDeleteAction(context: CodeFixContext, { name, signature, jsDocParameterTag }: Info) {
+        const changes = textChanges.ChangeTracker.with(context, changeTracker =>
+            changeTracker.filterJSDocTags(context.sourceFile, signature, t => t !== jsDocParameterTag));
+        return createCodeFixAction(deleteUnmatchedParameter, changes, [Diagnostics.Remove_unused_parameter_0, name.getText()], deleteUnmatchedParameter, Diagnostics.Remove_all_unused_parameters);
+    }
+
+    function getRenameAction(context: CodeFixContext, { name, signature, jsDocParameterTag }: Info) {
+        const sourceFile = context.sourceFile;
+        if (length(signature.parameters)) {
+            const tags = getJSDocTags(signature);
+            const names = new Set<__String>();
+            for (const tag of tags) {
+                if (isJSDocParameterTag(tag) && isIdentifier(tag.name)) {
+                    names.add(tag.name.escapedText);
+                }
+            }
+            const parameterName = firstDefined(signature.parameters, p =>
+                isIdentifier(p.name) && !names.has(p.name.escapedText) ? p.name.getText(sourceFile) : undefined);
+            if (parameterName === undefined) {
+                return undefined;
+            }
+            const newJSDocParameterTag = factory.updateJSDocParameterTag(jsDocParameterTag, jsDocParameterTag.tagName, factory.createIdentifier(parameterName), jsDocParameterTag.isBracketed, jsDocParameterTag.typeExpression, jsDocParameterTag.isNameFirst, jsDocParameterTag.comment);
+            const changes = textChanges.ChangeTracker.with(context, changeTracker =>
+                changeTracker.replaceJSDocComment(context.sourceFile, signature, map(tags, t => t === jsDocParameterTag ? newJSDocParameterTag : t)));
+            return createCodeFixActionWithoutFixAll(renameUnmatchedParameter, changes, [Diagnostics.Rename_parameter_0_to_1, name.getText(sourceFile), parameterName]);
+        }
+    }
+
+    interface Info {
+        readonly signature: SignatureDeclaration;
+        readonly jsDocParameterTag: JSDocParameterTag;
+        readonly name: Identifier;
+    }
+
+    function getInfo(sourceFile: SourceFile, pos: number): Info | undefined {
+        const token = getTokenAtPosition(sourceFile, pos);
+        if (token.parent && isJSDocParameterTag(token.parent) && isIdentifier(token.parent.name)) {
+            const jsDocParameterTag = token.parent;
+            const signature = getHostSignatureFromJSDoc(jsDocParameterTag);
+            if (signature) {
+                return { signature, name: token.parent.name, jsDocParameterTag };
+            }
+        }
+        return undefined;
+    }
+}

src/services/textChanges.ts

@@ -495,29 +495,30 @@ namespace ts.textChanges {
             this.insertNodeAt(sourceFile, fnStart, tag, { preserveLeadingWhitespace: false, suffix: this.newLineCharacter + indent });
         }
 
+        private createJSDocText(sourceFile: SourceFile, node: HasJSDoc) {
+            const comments = flatMap(node.jsDoc, jsDoc =>
+                isString(jsDoc.comment) ? factory.createJSDocText(jsDoc.comment) : jsDoc.comment) as JSDocComment[];
+            const jsDoc = singleOrUndefined(node.jsDoc);
+            return jsDoc && positionsAreOnSameLine(jsDoc.pos, jsDoc.end, sourceFile) && length(comments) === 0 ? undefined :
+                factory.createNodeArray(intersperse(comments, factory.createJSDocText("\n")));
+        }
+
+        public replaceJSDocComment(sourceFile: SourceFile, node: HasJSDoc, tags: readonly JSDocTag[]) {
+            this.insertJsdocCommentBefore(sourceFile, updateJSDocHost(node), factory.createJSDocComment(this.createJSDocText(sourceFile, node), factory.createNodeArray(tags)));
+        }
+
         public addJSDocTags(sourceFile: SourceFile, parent: HasJSDoc, newTags: readonly JSDocTag[]): void {
-            const comments = flatMap(parent.jsDoc, j => typeof j.comment === "string" ? factory.createJSDocText(j.comment) : j.comment) as JSDocComment[];
             const oldTags = flatMapToMutable(parent.jsDoc, j => j.tags);
             const unmergedNewTags = newTags.filter(newTag => !oldTags.some((tag, i) => {
                 const merged = tryMergeJsdocTags(tag, newTag);
                 if (merged) oldTags[i] = merged;
                 return !!merged;
             }));
-            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);
+            this.replaceJSDocComment(sourceFile, parent, [...oldTags, ...unmergedNewTags]);
         }
 
         public filterJSDocTags(sourceFile: SourceFile, parent: HasJSDoc, predicate: (tag: JSDocTag) => boolean): void {
-            const comments = flatMap(parent.jsDoc, j => typeof j.comment === "string" ? factory.createJSDocText(j.comment) : j.comment) as JSDocComment[];
-            const oldTags = flatMapToMutable(parent.jsDoc, j => j.tags);
-            const tag = factory.createJSDocComment(factory.createNodeArray(intersperse(comments, factory.createJSDocText("\n"))), factory.createNodeArray([...(filter(oldTags, predicate) || emptyArray)]));
-            const host = updateJSDocHost(parent);
-            this.insertJsdocCommentBefore(sourceFile, host, tag);
+            this.replaceJSDocComment(sourceFile, parent, filter(flatMapToMutable(parent.jsDoc, j => j.tags), predicate));
         }
 
         public replaceRangeWithText(sourceFile: SourceFile, range: TextRange, text: string): void {

src/services/tsconfig.json

@@ -87,6 +87,7 @@
         "codefixes/fixExtendsInterfaceBecomesImplements.ts",
         "codefixes/fixForgottenThisPropertyAccess.ts",
         "codefixes/fixInvalidJsxCharacters.ts",
+        "codefixes/fixUnmatchedParameter.ts",
         "codefixes/fixUnusedIdentifier.ts",
         "codefixes/fixUnreachableCode.ts",
         "codefixes/fixUnusedLabel.ts",

tests/baselines/reference/checkJsdocParamOnVariableDeclaredFunctionExpression.errors.txt

@@ -0,0 +1,23 @@
+tests/cases/conformance/jsdoc/0.js(14,20): error TS8024: JSDoc '@param' tag has name 's', but there is no parameter with that name.
+
+
+==== tests/cases/conformance/jsdoc/0.js (1 errors) ====
+    // @ts-check
+    /**
+     * @param {number=} n
+     * @param {string} [s]
+     */
+    var x = function foo(n, s) {}
+    var y;
+    /**
+     * @param {boolean!} b
+     */
+    y = function bar(b) {}
+
+    /**
+     * @param {string} s
+                       ~
+!!! error TS8024: JSDoc '@param' tag has name 's', but there is no parameter with that name.
+     */
+    var one = function (s) { }, two = function (untyped) { };
+

tests/cases/fourslash/codeFixDeleteUnmatchedParameter1.ts

@@ -0,0 +1,18 @@
+/// <reference path='fourslash.ts' />
+
+// @filename: a.ts
+/////**
+//// * @param {number} a
+//// * @param {number} b
+//// */
+////function foo() {}
+
+verify.codeFix({
+    description: [ts.Diagnostics.Remove_unused_parameter_0.message, "a"],
+    index: 0,
+    newFileContent:
+`/**
+ * @param {number} b
+ */
+function foo() {}`,
+});

tests/cases/fourslash/codeFixDeleteUnmatchedParameter2.ts

@@ -0,0 +1,18 @@
+/// <reference path='fourslash.ts' />
+
+// @filename: a.ts
+/////**
+//// * @param {number} a
+//// * @param {string} b
+//// */
+////function foo(a: number) {}
+
+verify.codeFix({
+    description: [ts.Diagnostics.Remove_unused_parameter_0.message, "b"],
+    index: 0,
+    newFileContent:
+`/**
+ * @param {number} a
+ */
+function foo(a: number) {}`
+});

tests/cases/fourslash/codeFixDeleteUnmatchedParameter3.ts

@@ -0,0 +1,20 @@
+/// <reference path='fourslash.ts' />
+
+// @filename: a.ts
+/////**
+//// * @param {number} a
+//// * @param {string} b
+//// * @param {number} c
+//// */
+////function foo(a: number, c: number) {}
+
+verify.codeFix({
+    description: [ts.Diagnostics.Remove_unused_parameter_0.message, "b"],
+    index: 0,
+    newFileContent:
+`/**
+ * @param {number} a
+ * @param {number} c
+ */
+function foo(a: number, c: number) {}`
+});

tests/cases/fourslash/codeFixDeleteUnmatchedParameter4.ts

@@ -0,0 +1,15 @@
+/// <reference path='fourslash.ts' />
+
+// @filename: a.ts
+/////**
+//// * @param {number} a
+//// */
+////function foo() {}
+
+verify.codeFix({
+    description: [ts.Diagnostics.Remove_unused_parameter_0.message, "a"],
+    index: 0,
+    newFileContent:
+`/** */
+function foo() {}`
+});

tests/cases/fourslash/codeFixDeleteUnmatchedParameterJS1.ts

@@ -0,0 +1,20 @@
+/// <reference path='fourslash.ts' />
+
+// @allowJs: true
+// @checkJs: true
+// @filename: /a.js
+/////**
+//// * @param {number} a
+//// * @param {number} b
+//// */
+////function foo() {}
+
+verify.codeFix({
+    description: [ts.Diagnostics.Remove_unused_parameter_0.message, "a"],
+    index: 0,
+    newFileContent:
+`/**
+ * @param {number} b
+ */
+function foo() {}`,
+});

tests/cases/fourslash/codeFixDeleteUnmatchedParameterJS2.ts

@@ -0,0 +1,20 @@
+/// <reference path='fourslash.ts' />
+
+// @allowJs: true
+// @checkJs: true
+// @filename: /a.js
+/////**
+//// * @param {number} a
+//// * @param {string} b
+//// */
+////function foo(a) {}
+
+verify.codeFix({
+    description: [ts.Diagnostics.Remove_unused_parameter_0.message, "b"],
+    index: 0,
+    newFileContent:
+`/**
+ * @param {number} a
+ */
+function foo(a) {}`
+});