fixUnusedIdentifier: Remove arguments corresponding to unused parameters (microsoft#25011)

* fixUnusedIdentifier: Remove arguments corresponding to unused parameters

* Update API (microsoft#24966)

* Fix handling of deletions: Make a list of things to delete and don't delete until the end

* Remove dummy test

* Bug fixes

* Update API (microsoft#24966)

* Move code to textChanges

Author: Andy

src/services/codeFixProvider.ts

@@ -79,10 +79,13 @@ namespace ts {
             return { fileName, textChanges };
         }
 
-        export function codeFixAll(context: CodeFixAllContext, errorCodes: number[], use: (changes: textChanges.ChangeTracker, error: DiagnosticWithLocation, commands: Push<CodeActionCommand>) => void): CombinedCodeActions {
+        export function codeFixAll(
+            context: CodeFixAllContext,
+            errorCodes: number[],
+            use: (changes: textChanges.ChangeTracker, error: DiagnosticWithLocation, commands: Push<CodeActionCommand>) => void,
+        ): CombinedCodeActions {
             const commands: CodeActionCommand[] = [];
-            const changes = textChanges.ChangeTracker.with(context, t =>
-                eachDiagnostic(context, errorCodes, diag => use(t, diag, commands)));
+            const changes = textChanges.ChangeTracker.with(context, t => eachDiagnostic(context, errorCodes, diag => use(t, diag, commands)));
             return createCombinedCodeActions(changes, commands.length === 0 ? undefined : commands);
         }
 

src/services/codefixes/fixUnusedIdentifier.ts

@@ -754,6 +754,25 @@ namespace ts.FindAllReferences.Core {
         }
     }
 
+    export function eachSignatureCall(signature: SignatureDeclaration, sourceFiles: ReadonlyArray<SourceFile>, checker: TypeChecker, cb: (call: CallExpression) => void): void {
+        if (!signature.name || !isIdentifier(signature.name)) return;
+
+        const symbol = Debug.assertDefined(checker.getSymbolAtLocation(signature.name));
+
+        for (const sourceFile of sourceFiles) {
+            for (const name of getPossibleSymbolReferenceNodes(sourceFile, symbol.name)) {
+                if (!isIdentifier(name) || name === signature.name || name.escapedText !== signature.name.escapedText) continue;
+                const called = climbPastPropertyAccess(name);
+                const call = called.parent;
+                if (!isCallExpression(call) || call.expression !== called) continue;
+                const referenceSymbol = checker.getSymbolAtLocation(name);
+                if (referenceSymbol && checker.getRootSymbols(referenceSymbol).some(s => s === symbol)) {
+                    cb(call);
+                }
+            }
+        }
+    }
+
     function getPossibleSymbolReferenceNodes(sourceFile: SourceFile, symbolName: string, container: Node = sourceFile): ReadonlyArray<Node> {
         return getPossibleSymbolReferencePositions(sourceFile, symbolName, container).map(pos => getTouchingPropertyName(sourceFile, pos));
     }

src/services/findAllReferences.ts

@@ -214,6 +214,7 @@ namespace ts.textChanges {
         private readonly newFiles: { readonly oldFile: SourceFile, readonly fileName: string, readonly statements: ReadonlyArray<Statement> }[] = [];
         private readonly deletedNodesInLists = new NodeSet(); // Stores ids of nodes in lists that we already deleted. Used to avoid deleting `, ` twice in `a, b`.
         private readonly classesWithNodesInsertedAtStart = createMap<ClassDeclaration>(); // Set<ClassDeclaration> implemented as Map<node id, ClassDeclaration>
+        private readonly deletedDeclarations: { readonly sourceFile: SourceFile, readonly node: Node }[] = [];
 
         public static fromContext(context: TextChangesContext): ChangeTracker {
             return new ChangeTracker(getNewLineOrDefaultFromHost(context.host, context.formatContext.options), context.formatContext);
@@ -233,6 +234,10 @@ namespace ts.textChanges {
             return this;
         }
 
+        deleteDeclaration(sourceFile: SourceFile, node: Node): void {
+            this.deletedDeclarations.push({ sourceFile, node });
+        }
+
         /** Warning: This deletes comments too. See `copyComments` in `convertFunctionToEs6Class`. */
         public deleteNode(sourceFile: SourceFile, node: Node, options: ConfigurableStartEnd = {}) {
             const startPosition = getAdjustedStartPosition(sourceFile, node, options, Position.FullStart);
@@ -699,13 +704,22 @@ namespace ts.textChanges {
             });
         }
 
+        private finishDeleteDeclarations(): void {
+            for (const { sourceFile, node } of this.deletedDeclarations) {
+                if (!this.deletedDeclarations.some(d => d.sourceFile === sourceFile && rangeContainsRangeExclusive(d.node, node))) {
+                    deleteDeclaration.deleteDeclaration(this, sourceFile, node);
+                }
+            }
+        }
+
         /**
          * Note: after calling this, the TextChanges object must be discarded!
          * @param validate only for tests
          *    The reason we must validate as part of this method is that `getNonFormattedText` changes the node's positions,
          *    so we can only call this once and can't get the non-formatted text separately.
          */
         public getChanges(validate?: ValidateNonFormattedText): FileTextChanges[] {
+            this.finishDeleteDeclarations();
             this.finishClassesWithNodesInsertedAtStart();
             this.finishTrailingCommaAfterDeletingNodesInList();
             const changes = changesToText.getTextChangesFromChanges(this.changes, this.newLineCharacter, this.formatContext, validate);
@@ -1021,4 +1035,170 @@ namespace ts.textChanges {
         return (isPropertySignature(a) || isPropertyDeclaration(a)) && isClassOrTypeElement(b) && b.name!.kind === SyntaxKind.ComputedPropertyName
             || isStatementButNotDeclaration(a) && isStatementButNotDeclaration(b); // TODO: only if b would start with a `(` or `[`
     }
+
+    namespace deleteDeclaration {
+        export function deleteDeclaration(changes: ChangeTracker, sourceFile: SourceFile, node: Node): void {
+            switch (node.kind) {
+                case SyntaxKind.Parameter: {
+                    const oldFunction = node.parent;
+                    if (isArrowFunction(oldFunction) && oldFunction.parameters.length === 1) {
+                        // Lambdas with exactly one parameter are special because, after removal, there
+                        // must be an empty parameter list (i.e. `()`) and this won't necessarily be the
+                        // case if the parameter is simply removed (e.g. in `x => 1`).
+                        const newFunction = updateArrowFunction(
+                            oldFunction,
+                            oldFunction.modifiers,
+                            oldFunction.typeParameters,
+                            /*parameters*/ undefined!, // TODO: GH#18217
+                            oldFunction.type,
+                            oldFunction.equalsGreaterThanToken,
+                            oldFunction.body);
+
+                        // Drop leading and trailing trivia of the new function because we're only going
+                        // to replace the span (vs the full span) of the old function - the old leading
+                        // and trailing trivia will remain.
+                        suppressLeadingAndTrailingTrivia(newFunction);
+
+                        changes.replaceNode(sourceFile, oldFunction, newFunction);
+                    }
+                    else {
+                        changes.deleteNodeInList(sourceFile, node);
+                    }
+                    break;
+                }
+
+                case SyntaxKind.ImportDeclaration:
+                    changes.deleteNode(sourceFile, node);
+                    break;
+
+                case SyntaxKind.BindingElement:
+                    const pattern = (node as BindingElement).parent;
+                    const preserveComma = pattern.kind === SyntaxKind.ArrayBindingPattern && node !== last(pattern.elements);
+                    if (preserveComma) {
+                        changes.deleteNode(sourceFile, node);
+                    }
+                    else {
+                        changes.deleteNodeInList(sourceFile, node);
+                    }
+                    break;
+
+                case SyntaxKind.VariableDeclaration:
+                    deleteVariableDeclaration(changes, sourceFile, node as VariableDeclaration);
+                    break;
+
+                case SyntaxKind.TypeParameter: {
+                    const typeParameters = getEffectiveTypeParameterDeclarations(<DeclarationWithTypeParameters>node.parent);
+                    if (typeParameters.length === 1) {
+                        const { pos, end } = cast(typeParameters, isNodeArray);
+                        const previousToken = getTokenAtPosition(sourceFile, pos - 1);
+                        const nextToken = getTokenAtPosition(sourceFile, end);
+                        Debug.assert(previousToken.kind === SyntaxKind.LessThanToken);
+                        Debug.assert(nextToken.kind === SyntaxKind.GreaterThanToken);
+
+                        changes.deleteNodeRange(sourceFile, previousToken, nextToken);
+                    }
+                    else {
+                        changes.deleteNodeInList(sourceFile, node);
+                    }
+                    break;
+                }
+
+                case SyntaxKind.ImportSpecifier:
+                    const namedImports = (node as ImportSpecifier).parent;
+                    if (namedImports.elements.length === 1) {
+                        deleteImportBinding(changes, sourceFile, namedImports);
+                    }
+                    else {
+                        changes.deleteNodeInList(sourceFile, node);
+                    }
+                    break;
+
+                case SyntaxKind.NamespaceImport:
+                    deleteImportBinding(changes, sourceFile, node as NamespaceImport);
+                    break;
+
+                default:
+                    if (isImportClause(node.parent) && node.parent.name === node) {
+                        deleteDefaultImport(changes, sourceFile, node.parent);
+                    }
+                    else if (isCallLikeExpression(node.parent)) {
+                        changes.deleteNodeInList(sourceFile, node);
+                    }
+                    else {
+                        changes.deleteNode(sourceFile, node);
+                    }
+            }
+        }
+
+        function deleteDefaultImport(changes: ChangeTracker, sourceFile: SourceFile, importClause: ImportClause): void {
+            if (!importClause.namedBindings) {
+                // Delete the whole import
+                changes.deleteNode(sourceFile, importClause.parent);
+            }
+            else {
+                // import |d,| * as ns from './file'
+                const start = importClause.name!.getStart(sourceFile);
+                const nextToken = getTokenAtPosition(sourceFile, importClause.name!.end);
+                if (nextToken && nextToken.kind === SyntaxKind.CommaToken) {
+                    // shift first non-whitespace position after comma to the start position of the node
+                    const end = skipTrivia(sourceFile.text, nextToken.end, /*stopAfterLineBreaks*/ false, /*stopAtComments*/ true);
+                    changes.deleteRange(sourceFile, { pos: start, end });
+                }
+                else {
+                    changes.deleteNode(sourceFile, importClause.name!);
+                }
+            }
+        }
+
+        function deleteImportBinding(changes: ChangeTracker, sourceFile: SourceFile, node: NamedImportBindings): void {
+            if (node.parent.name) {
+                // Delete named imports while preserving the default import
+                // import d|, * as ns| from './file'
+                // import d|, { a }| from './file'
+                const previousToken = Debug.assertDefined(getTokenAtPosition(sourceFile, node.pos - 1));
+                changes.deleteRange(sourceFile, { pos: previousToken.getStart(sourceFile), end: node.end });
+            }
+            else {
+                // Delete the entire import declaration
+                // |import * as ns from './file'|
+                // |import { a } from './file'|
+                const importDecl = getAncestor(node, SyntaxKind.ImportDeclaration)!;
+                changes.deleteNode(sourceFile, importDecl);
+            }
+        }
+
+        function deleteVariableDeclaration(changes: ChangeTracker, sourceFile: SourceFile, node: VariableDeclaration): void {
+            const { parent } = node;
+
+            if (parent.kind === SyntaxKind.CatchClause) {
+                // TODO: There's currently no unused diagnostic for this, could be a suggestion
+                changes.deleteNodeRange(sourceFile, findChildOfKind(parent, SyntaxKind.OpenParenToken, sourceFile)!, findChildOfKind(parent, SyntaxKind.CloseParenToken, sourceFile)!);
+                return;
+            }
+
+            if (parent.declarations.length !== 1) {
+                changes.deleteNodeInList(sourceFile, node);
+                return;
+            }
+
+            const gp = parent.parent;
+            switch (gp.kind) {
+                case SyntaxKind.ForOfStatement:
+                case SyntaxKind.ForInStatement:
+                    changes.replaceNode(sourceFile, node, createObjectLiteral());
+                    break;
+
+                case SyntaxKind.ForStatement:
+                    changes.deleteNode(sourceFile, parent);
+                    break;
+
+                case SyntaxKind.VariableStatement:
+                    changes.deleteNode(sourceFile, gp);
+                    break;
+
+                default:
+                    Debug.assertNever(gp);
+            }
+        }
+    }
 }

src/services/textChanges.ts

@@ -419,6 +419,10 @@ namespace ts {
         return startEndContainsRange(r1.pos, r1.end, r2);
     }
 
+    export function rangeContainsRangeExclusive(r1: TextRange, r2: TextRange): boolean {
+        return rangeContainsPositionExclusive(r1, r2.pos) && rangeContainsPositionExclusive(r1, r2.end);
+    }
+
     export function rangeContainsPosition(r: TextRange, pos: number): boolean {
         return r.pos <= pos && pos <= r.end;
     }
@@ -1340,7 +1344,13 @@ namespace ts {
         return getPropertySymbolsFromBaseTypes(memberSymbol.parent!, memberSymbol.name, checker, _ => true) || false;
     }
 
-    export class NodeSet {
+    export interface ReadonlyNodeSet {
+        has(node: Node): boolean;
+        forEach(cb: (node: Node) => void): void;
+        some(pred: (node: Node) => boolean): boolean;
+    }
+
+    export class NodeSet implements ReadonlyNodeSet {
         private map = createMap<Node>();
 
         add(node: Node): void {

src/services/utilities.ts

@@ -10717,6 +10717,7 @@ declare namespace ts {
     }
     function getLineStartPositionForPosition(position: number, sourceFile: SourceFileLike): number;
     function rangeContainsRange(r1: TextRange, r2: TextRange): boolean;
+    function rangeContainsRangeExclusive(r1: TextRange, r2: TextRange): boolean;
     function rangeContainsPosition(r: TextRange, pos: number): boolean;
     function rangeContainsPositionExclusive(r: TextRange, pos: number): boolean;
     function startEndContainsRange(start: number, end: number, range: TextRange): boolean;
@@ -10834,7 +10835,12 @@ declare namespace ts {
      */
     function getPropertySymbolsFromBaseTypes<T>(symbol: Symbol, propertyName: string, checker: TypeChecker, cb: (symbol: Symbol) => T | undefined): T | undefined;
     function isMemberSymbolInBaseType(memberSymbol: Symbol, checker: TypeChecker): boolean;
-    class NodeSet {
+    interface ReadonlyNodeSet {
+        has(node: Node): boolean;
+        forEach(cb: (node: Node) => void): void;
+        some(pred: (node: Node) => boolean): boolean;
+    }
+    class NodeSet implements ReadonlyNodeSet {
         private map;
         add(node: Node): void;
         has(node: Node): boolean;
@@ -11127,6 +11133,7 @@ declare namespace ts.FindAllReferences.Core {
     /** Used as a quick check for whether a symbol is used at all in a file (besides its definition). */
     function isSymbolReferencedInFile(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile): boolean;
     function eachSymbolReferenceInFile<T>(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile, cb: (token: Identifier) => T): T | undefined;
+    function eachSignatureCall(signature: SignatureDeclaration, sourceFiles: ReadonlyArray<SourceFile>, checker: TypeChecker, cb: (call: CallExpression) => void): void;
     /**
      * Given an initial searchMeaning, extracted from a location, widen the search scope based on the declarations
      * of the corresponding symbol. e.g. if we are searching for "Foo" in value position, but "Foo" references a class
@@ -11505,11 +11512,13 @@ declare namespace ts.textChanges {
         private readonly newFiles;
         private readonly deletedNodesInLists;
         private readonly classesWithNodesInsertedAtStart;
+        private readonly deletedDeclarations;
         static fromContext(context: TextChangesContext): ChangeTracker;
         static with(context: TextChangesContext, cb: (tracker: ChangeTracker) => void): FileTextChanges[];
         /** Public for tests only. Other callers should use `ChangeTracker.with`. */
         constructor(newLineCharacter: string, formatContext: formatting.FormatContext);
         deleteRange(sourceFile: SourceFile, range: TextRange): this;
+        deleteDeclaration(sourceFile: SourceFile, node: Node): void;
         /** Warning: This deletes comments too. See `copyComments` in `convertFunctionToEs6Class`. */
         deleteNode(sourceFile: SourceFile, node: Node, options?: ConfigurableStartEnd): this;
         deleteModifier(sourceFile: SourceFile, modifier: Modifier): void;
@@ -11559,6 +11568,7 @@ declare namespace ts.textChanges {
         insertNodeInListAfter(sourceFile: SourceFile, after: Node, newNode: Node, containingList?: NodeArray<Node> | undefined): this;
         private finishClassesWithNodesInsertedAtStart;
         private finishTrailingCommaAfterDeletingNodesInList;
+        private finishDeleteDeclarations;
         /**
          * Note: after calling this, the TextChanges object must be discarded!
          * @param validate only for tests