fix(39676): skip removing unused parameters for functions used as callback references (microsoft#40299)

Author: a-tarasyuk

src/services/codefixes/fixUnusedIdentifier.ts

@@ -224,12 +224,10 @@ namespace ts.codefix {
     }
 
     function tryDeleteParameter(changes: textChanges.ChangeTracker, sourceFile: SourceFile, p: ParameterDeclaration, checker: TypeChecker, sourceFiles: readonly SourceFile[], isFixAll = false): void {
-        if (mayDeleteParameter(p, checker, isFixAll)) {
-            if (p.modifiers && p.modifiers.length > 0
-                    && (!isIdentifier(p.name) || FindAllReferences.Core.isSymbolReferencedInFile(p.name, checker, sourceFile))) {
-                p.modifiers.forEach(modifier => {
-                    changes.deleteModifier(sourceFile, modifier);
-                });
+        if (mayDeleteParameter(checker, sourceFile, p, isFixAll)) {
+            if (p.modifiers && p.modifiers.length > 0 &&
+                (!isIdentifier(p.name) || FindAllReferences.Core.isSymbolReferencedInFile(p.name, checker, sourceFile))) {
+                p.modifiers.forEach(modifier => changes.deleteModifier(sourceFile, modifier));
             }
             else {
                 changes.delete(sourceFile, p);
@@ -238,29 +236,26 @@ namespace ts.codefix {
         }
     }
 
-    function mayDeleteParameter(p: ParameterDeclaration, checker: TypeChecker, isFixAll: boolean): boolean {
-        const { parent } = p;
+    function mayDeleteParameter(checker: TypeChecker, sourceFile: SourceFile, parameter: ParameterDeclaration, isFixAll: boolean): boolean {
+        const { parent } = parameter;
         switch (parent.kind) {
             case SyntaxKind.MethodDeclaration:
                 // Don't remove a parameter if this overrides something.
                 const symbol = checker.getSymbolAtLocation(parent.name)!;
                 if (isMemberSymbolInBaseType(symbol, checker)) return false;
                 // falls through
-
             case SyntaxKind.Constructor:
-            case SyntaxKind.FunctionDeclaration:
                 return true;
-
+            case SyntaxKind.FunctionDeclaration: {
+                if (parent.name && isCallbackLike(checker, sourceFile, parent.name)) {
+                    return isLastParameter(parent, parameter, isFixAll);
+                }
+                return true;
+            }
             case SyntaxKind.FunctionExpression:
-            case SyntaxKind.ArrowFunction: {
+            case SyntaxKind.ArrowFunction:
                 // Can't remove a non-last parameter in a callback. Can remove a parameter in code-fix-all if future parameters are also unused.
-                const { parameters } = parent;
-                const index = parameters.indexOf(p);
-                Debug.assert(index !== -1, "The parameter should already be in the list");
-                return isFixAll
-                    ? parameters.slice(index + 1).every(p => p.name.kind === SyntaxKind.Identifier && !p.symbol.isReferenced)
-                    : index === parameters.length - 1;
-            }
+                return isLastParameter(parent, parameter, isFixAll);
 
             case SyntaxKind.SetAccessor:
                 // Setter must have a parameter
@@ -279,4 +274,18 @@ namespace ts.codefix {
             }
         });
     }
+
+    function isCallbackLike(checker: TypeChecker, sourceFile: SourceFile, name: Identifier): boolean {
+        return !!FindAllReferences.Core.eachSymbolReferenceInFile(name, checker, sourceFile, reference =>
+            isIdentifier(reference) && isCallExpression(reference.parent) && reference.parent.arguments.indexOf(reference) >= 0);
+    }
+
+    function isLastParameter(func: FunctionLikeDeclaration, parameter: ParameterDeclaration, isFixAll: boolean): boolean {
+        const parameters = func.parameters;
+        const index = parameters.indexOf(parameter);
+        Debug.assert(index !== -1, "The parameter should already be in the list");
+        return isFixAll ?
+            parameters.slice(index + 1).every(p => isIdentifier(p.name) && !p.symbol.isReferenced) :
+            index === parameters.length - 1;
+    }
 }

tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts

@@ -31,6 +31,15 @@
 ////takesCb((x, y) => { x; });
 ////takesCb((x, y) => { y; });
 ////
+////function fn1(x: number, y: string): void {}
+////takesCb(fn1);
+////
+////function fn2(x: number, y: string): void { x; }
+////takesCb(fn2);
+////
+////function fn3(x: number, y: string): void { y; }
+////takesCb(fn3);
+////
 ////x => {
 ////    const y = 0;
 ////};
@@ -76,6 +85,15 @@ takesCb(() => {});
 takesCb((x) => { x; });
 takesCb((x, y) => { y; });
 
+function fn1(): void {}
+takesCb(fn1);
+
+function fn2(x: number): void { x; }
+takesCb(fn2);
+
+function fn3(x: number, y: string): void { y; }
+takesCb(fn3);
+
 () => {
 };
 

tests/cases/fourslash/codeFixUnusedIdentifier_parameter_callback.ts renamed to tests/cases/fourslash/codeFixUnusedIdentifier_parameter_callback1.ts

@@ -0,0 +1,12 @@
+/// <reference path='fourslash.ts' />
+
+// @noUnusedParameters: true
+
+////declare function foo(cb: (x: number, y: string) => void): void;
+////function fn(x: number, y: number): any {
+////    return y;
+////}
+////foo(fn);
+
+// No codefix to remove a non-last parameter in a callback
+verify.codeFixAvailable([{ description: "Prefix 'x' with an underscore" }]);