Skip to content

Commit d957b1c

Browse files
author
Andy
authored
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
1 parent d7713f4 commit d957b1c

13 files changed

Lines changed: 371 additions & 246 deletions

‎src/services/codeFixProvider.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,13 @@ namespace ts {
7979
return { fileName, textChanges };
8080
}
8181

82-
export function codeFixAll(context: CodeFixAllContext, errorCodes: number[], use: (changes: textChanges.ChangeTracker, error: DiagnosticWithLocation, commands: Push<CodeActionCommand>) => void): CombinedCodeActions {
82+
export function codeFixAll(
83+
context: CodeFixAllContext,
84+
errorCodes: number[],
85+
use: (changes: textChanges.ChangeTracker, error: DiagnosticWithLocation, commands: Push<CodeActionCommand>) => void,
86+
): CombinedCodeActions {
8387
const commands: CodeActionCommand[] = [];
84-
const changes = textChanges.ChangeTracker.with(context, t =>
85-
eachDiagnostic(context, errorCodes, diag => use(t, diag, commands)));
88+
const changes = textChanges.ChangeTracker.with(context, t => eachDiagnostic(context, errorCodes, diag => use(t, diag, commands)));
8689
return createCombinedCodeActions(changes, commands.length === 0 ? undefined : commands);
8790
}
8891

‎src/services/codefixes/fixUnusedIdentifier.ts‎

Lines changed: 61 additions & 229 deletions
Large diffs are not rendered by default.

‎src/services/findAllReferences.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -754,6 +754,25 @@ namespace ts.FindAllReferences.Core {
754754
}
755755
}
756756

757+
export function eachSignatureCall(signature: SignatureDeclaration, sourceFiles: ReadonlyArray<SourceFile>, checker: TypeChecker, cb: (call: CallExpression) => void): void {
758+
if (!signature.name || !isIdentifier(signature.name)) return;
759+
760+
const symbol = Debug.assertDefined(checker.getSymbolAtLocation(signature.name));
761+
762+
for (const sourceFile of sourceFiles) {
763+
for (const name of getPossibleSymbolReferenceNodes(sourceFile, symbol.name)) {
764+
if (!isIdentifier(name) || name === signature.name || name.escapedText !== signature.name.escapedText) continue;
765+
const called = climbPastPropertyAccess(name);
766+
const call = called.parent;
767+
if (!isCallExpression(call) || call.expression !== called) continue;
768+
const referenceSymbol = checker.getSymbolAtLocation(name);
769+
if (referenceSymbol && checker.getRootSymbols(referenceSymbol).some(s => s === symbol)) {
770+
cb(call);
771+
}
772+
}
773+
}
774+
}
775+
757776
function getPossibleSymbolReferenceNodes(sourceFile: SourceFile, symbolName: string, container: Node = sourceFile): ReadonlyArray<Node> {
758777
return getPossibleSymbolReferencePositions(sourceFile, symbolName, container).map(pos => getTouchingPropertyName(sourceFile, pos));
759778
}

‎src/services/textChanges.ts‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ namespace ts.textChanges {
214214
private readonly newFiles: { readonly oldFile: SourceFile, readonly fileName: string, readonly statements: ReadonlyArray<Statement> }[] = [];
215215
private readonly deletedNodesInLists = new NodeSet(); // Stores ids of nodes in lists that we already deleted. Used to avoid deleting `, ` twice in `a, b`.
216216
private readonly classesWithNodesInsertedAtStart = createMap<ClassDeclaration>(); // Set<ClassDeclaration> implemented as Map<node id, ClassDeclaration>
217+
private readonly deletedDeclarations: { readonly sourceFile: SourceFile, readonly node: Node }[] = [];
217218

218219
public static fromContext(context: TextChangesContext): ChangeTracker {
219220
return new ChangeTracker(getNewLineOrDefaultFromHost(context.host, context.formatContext.options), context.formatContext);
@@ -233,6 +234,10 @@ namespace ts.textChanges {
233234
return this;
234235
}
235236

237+
deleteDeclaration(sourceFile: SourceFile, node: Node): void {
238+
this.deletedDeclarations.push({ sourceFile, node });
239+
}
240+
236241
/** Warning: This deletes comments too. See `copyComments` in `convertFunctionToEs6Class`. */
237242
public deleteNode(sourceFile: SourceFile, node: Node, options: ConfigurableStartEnd = {}) {
238243
const startPosition = getAdjustedStartPosition(sourceFile, node, options, Position.FullStart);
@@ -699,13 +704,22 @@ namespace ts.textChanges {
699704
});
700705
}
701706

707+
private finishDeleteDeclarations(): void {
708+
for (const { sourceFile, node } of this.deletedDeclarations) {
709+
if (!this.deletedDeclarations.some(d => d.sourceFile === sourceFile && rangeContainsRangeExclusive(d.node, node))) {
710+
deleteDeclaration.deleteDeclaration(this, sourceFile, node);
711+
}
712+
}
713+
}
714+
702715
/**
703716
* Note: after calling this, the TextChanges object must be discarded!
704717
* @param validate only for tests
705718
* The reason we must validate as part of this method is that `getNonFormattedText` changes the node's positions,
706719
* so we can only call this once and can't get the non-formatted text separately.
707720
*/
708721
public getChanges(validate?: ValidateNonFormattedText): FileTextChanges[] {
722+
this.finishDeleteDeclarations();
709723
this.finishClassesWithNodesInsertedAtStart();
710724
this.finishTrailingCommaAfterDeletingNodesInList();
711725
const changes = changesToText.getTextChangesFromChanges(this.changes, this.newLineCharacter, this.formatContext, validate);
@@ -1021,4 +1035,170 @@ namespace ts.textChanges {
10211035
return (isPropertySignature(a) || isPropertyDeclaration(a)) && isClassOrTypeElement(b) && b.name!.kind === SyntaxKind.ComputedPropertyName
10221036
|| isStatementButNotDeclaration(a) && isStatementButNotDeclaration(b); // TODO: only if b would start with a `(` or `[`
10231037
}
1038+
1039+
namespace deleteDeclaration {
1040+
export function deleteDeclaration(changes: ChangeTracker, sourceFile: SourceFile, node: Node): void {
1041+
switch (node.kind) {
1042+
case SyntaxKind.Parameter: {
1043+
const oldFunction = node.parent;
1044+
if (isArrowFunction(oldFunction) && oldFunction.parameters.length === 1) {
1045+
// Lambdas with exactly one parameter are special because, after removal, there
1046+
// must be an empty parameter list (i.e. `()`) and this won't necessarily be the
1047+
// case if the parameter is simply removed (e.g. in `x => 1`).
1048+
const newFunction = updateArrowFunction(
1049+
oldFunction,
1050+
oldFunction.modifiers,
1051+
oldFunction.typeParameters,
1052+
/*parameters*/ undefined!, // TODO: GH#18217
1053+
oldFunction.type,
1054+
oldFunction.equalsGreaterThanToken,
1055+
oldFunction.body);
1056+
1057+
// Drop leading and trailing trivia of the new function because we're only going
1058+
// to replace the span (vs the full span) of the old function - the old leading
1059+
// and trailing trivia will remain.
1060+
suppressLeadingAndTrailingTrivia(newFunction);
1061+
1062+
changes.replaceNode(sourceFile, oldFunction, newFunction);
1063+
}
1064+
else {
1065+
changes.deleteNodeInList(sourceFile, node);
1066+
}
1067+
break;
1068+
}
1069+
1070+
case SyntaxKind.ImportDeclaration:
1071+
changes.deleteNode(sourceFile, node);
1072+
break;
1073+
1074+
case SyntaxKind.BindingElement:
1075+
const pattern = (node as BindingElement).parent;
1076+
const preserveComma = pattern.kind === SyntaxKind.ArrayBindingPattern && node !== last(pattern.elements);
1077+
if (preserveComma) {
1078+
changes.deleteNode(sourceFile, node);
1079+
}
1080+
else {
1081+
changes.deleteNodeInList(sourceFile, node);
1082+
}
1083+
break;
1084+
1085+
case SyntaxKind.VariableDeclaration:
1086+
deleteVariableDeclaration(changes, sourceFile, node as VariableDeclaration);
1087+
break;
1088+
1089+
case SyntaxKind.TypeParameter: {
1090+
const typeParameters = getEffectiveTypeParameterDeclarations(<DeclarationWithTypeParameters>node.parent);
1091+
if (typeParameters.length === 1) {
1092+
const { pos, end } = cast(typeParameters, isNodeArray);
1093+
const previousToken = getTokenAtPosition(sourceFile, pos - 1);
1094+
const nextToken = getTokenAtPosition(sourceFile, end);
1095+
Debug.assert(previousToken.kind === SyntaxKind.LessThanToken);
1096+
Debug.assert(nextToken.kind === SyntaxKind.GreaterThanToken);
1097+
1098+
changes.deleteNodeRange(sourceFile, previousToken, nextToken);
1099+
}
1100+
else {
1101+
changes.deleteNodeInList(sourceFile, node);
1102+
}
1103+
break;
1104+
}
1105+
1106+
case SyntaxKind.ImportSpecifier:
1107+
const namedImports = (node as ImportSpecifier).parent;
1108+
if (namedImports.elements.length === 1) {
1109+
deleteImportBinding(changes, sourceFile, namedImports);
1110+
}
1111+
else {
1112+
changes.deleteNodeInList(sourceFile, node);
1113+
}
1114+
break;
1115+
1116+
case SyntaxKind.NamespaceImport:
1117+
deleteImportBinding(changes, sourceFile, node as NamespaceImport);
1118+
break;
1119+
1120+
default:
1121+
if (isImportClause(node.parent) && node.parent.name === node) {
1122+
deleteDefaultImport(changes, sourceFile, node.parent);
1123+
}
1124+
else if (isCallLikeExpression(node.parent)) {
1125+
changes.deleteNodeInList(sourceFile, node);
1126+
}
1127+
else {
1128+
changes.deleteNode(sourceFile, node);
1129+
}
1130+
}
1131+
}
1132+
1133+
function deleteDefaultImport(changes: ChangeTracker, sourceFile: SourceFile, importClause: ImportClause): void {
1134+
if (!importClause.namedBindings) {
1135+
// Delete the whole import
1136+
changes.deleteNode(sourceFile, importClause.parent);
1137+
}
1138+
else {
1139+
// import |d,| * as ns from './file'
1140+
const start = importClause.name!.getStart(sourceFile);
1141+
const nextToken = getTokenAtPosition(sourceFile, importClause.name!.end);
1142+
if (nextToken && nextToken.kind === SyntaxKind.CommaToken) {
1143+
// shift first non-whitespace position after comma to the start position of the node
1144+
const end = skipTrivia(sourceFile.text, nextToken.end, /*stopAfterLineBreaks*/ false, /*stopAtComments*/ true);
1145+
changes.deleteRange(sourceFile, { pos: start, end });
1146+
}
1147+
else {
1148+
changes.deleteNode(sourceFile, importClause.name!);
1149+
}
1150+
}
1151+
}
1152+
1153+
function deleteImportBinding(changes: ChangeTracker, sourceFile: SourceFile, node: NamedImportBindings): void {
1154+
if (node.parent.name) {
1155+
// Delete named imports while preserving the default import
1156+
// import d|, * as ns| from './file'
1157+
// import d|, { a }| from './file'
1158+
const previousToken = Debug.assertDefined(getTokenAtPosition(sourceFile, node.pos - 1));
1159+
changes.deleteRange(sourceFile, { pos: previousToken.getStart(sourceFile), end: node.end });
1160+
}
1161+
else {
1162+
// Delete the entire import declaration
1163+
// |import * as ns from './file'|
1164+
// |import { a } from './file'|
1165+
const importDecl = getAncestor(node, SyntaxKind.ImportDeclaration)!;
1166+
changes.deleteNode(sourceFile, importDecl);
1167+
}
1168+
}
1169+
1170+
function deleteVariableDeclaration(changes: ChangeTracker, sourceFile: SourceFile, node: VariableDeclaration): void {
1171+
const { parent } = node;
1172+
1173+
if (parent.kind === SyntaxKind.CatchClause) {
1174+
// TODO: There's currently no unused diagnostic for this, could be a suggestion
1175+
changes.deleteNodeRange(sourceFile, findChildOfKind(parent, SyntaxKind.OpenParenToken, sourceFile)!, findChildOfKind(parent, SyntaxKind.CloseParenToken, sourceFile)!);
1176+
return;
1177+
}
1178+
1179+
if (parent.declarations.length !== 1) {
1180+
changes.deleteNodeInList(sourceFile, node);
1181+
return;
1182+
}
1183+
1184+
const gp = parent.parent;
1185+
switch (gp.kind) {
1186+
case SyntaxKind.ForOfStatement:
1187+
case SyntaxKind.ForInStatement:
1188+
changes.replaceNode(sourceFile, node, createObjectLiteral());
1189+
break;
1190+
1191+
case SyntaxKind.ForStatement:
1192+
changes.deleteNode(sourceFile, parent);
1193+
break;
1194+
1195+
case SyntaxKind.VariableStatement:
1196+
changes.deleteNode(sourceFile, gp);
1197+
break;
1198+
1199+
default:
1200+
Debug.assertNever(gp);
1201+
}
1202+
}
1203+
}
10241204
}

‎src/services/utilities.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,10 @@ namespace ts {
419419
return startEndContainsRange(r1.pos, r1.end, r2);
420420
}
421421

422+
export function rangeContainsRangeExclusive(r1: TextRange, r2: TextRange): boolean {
423+
return rangeContainsPositionExclusive(r1, r2.pos) && rangeContainsPositionExclusive(r1, r2.end);
424+
}
425+
422426
export function rangeContainsPosition(r: TextRange, pos: number): boolean {
423427
return r.pos <= pos && pos <= r.end;
424428
}
@@ -1340,7 +1344,13 @@ namespace ts {
13401344
return getPropertySymbolsFromBaseTypes(memberSymbol.parent!, memberSymbol.name, checker, _ => true) || false;
13411345
}
13421346

1343-
export class NodeSet {
1347+
export interface ReadonlyNodeSet {
1348+
has(node: Node): boolean;
1349+
forEach(cb: (node: Node) => void): void;
1350+
some(pred: (node: Node) => boolean): boolean;
1351+
}
1352+
1353+
export class NodeSet implements ReadonlyNodeSet {
13441354
private map = createMap<Node>();
13451355

13461356
add(node: Node): void {

‎tests/baselines/reference/api/tsserverlibrary.d.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10717,6 +10717,7 @@ declare namespace ts {
1071710717
}
1071810718
function getLineStartPositionForPosition(position: number, sourceFile: SourceFileLike): number;
1071910719
function rangeContainsRange(r1: TextRange, r2: TextRange): boolean;
10720+
function rangeContainsRangeExclusive(r1: TextRange, r2: TextRange): boolean;
1072010721
function rangeContainsPosition(r: TextRange, pos: number): boolean;
1072110722
function rangeContainsPositionExclusive(r: TextRange, pos: number): boolean;
1072210723
function startEndContainsRange(start: number, end: number, range: TextRange): boolean;
@@ -10834,7 +10835,12 @@ declare namespace ts {
1083410835
*/
1083510836
function getPropertySymbolsFromBaseTypes<T>(symbol: Symbol, propertyName: string, checker: TypeChecker, cb: (symbol: Symbol) => T | undefined): T | undefined;
1083610837
function isMemberSymbolInBaseType(memberSymbol: Symbol, checker: TypeChecker): boolean;
10837-
class NodeSet {
10838+
interface ReadonlyNodeSet {
10839+
has(node: Node): boolean;
10840+
forEach(cb: (node: Node) => void): void;
10841+
some(pred: (node: Node) => boolean): boolean;
10842+
}
10843+
class NodeSet implements ReadonlyNodeSet {
1083810844
private map;
1083910845
add(node: Node): void;
1084010846
has(node: Node): boolean;
@@ -11127,6 +11133,7 @@ declare namespace ts.FindAllReferences.Core {
1112711133
/** Used as a quick check for whether a symbol is used at all in a file (besides its definition). */
1112811134
function isSymbolReferencedInFile(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile): boolean;
1112911135
function eachSymbolReferenceInFile<T>(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile, cb: (token: Identifier) => T): T | undefined;
11136+
function eachSignatureCall(signature: SignatureDeclaration, sourceFiles: ReadonlyArray<SourceFile>, checker: TypeChecker, cb: (call: CallExpression) => void): void;
1113011137
/**
1113111138
* Given an initial searchMeaning, extracted from a location, widen the search scope based on the declarations
1113211139
* 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 {
1150511512
private readonly newFiles;
1150611513
private readonly deletedNodesInLists;
1150711514
private readonly classesWithNodesInsertedAtStart;
11515+
private readonly deletedDeclarations;
1150811516
static fromContext(context: TextChangesContext): ChangeTracker;
1150911517
static with(context: TextChangesContext, cb: (tracker: ChangeTracker) => void): FileTextChanges[];
1151011518
/** Public for tests only. Other callers should use `ChangeTracker.with`. */
1151111519
constructor(newLineCharacter: string, formatContext: formatting.FormatContext);
1151211520
deleteRange(sourceFile: SourceFile, range: TextRange): this;
11521+
deleteDeclaration(sourceFile: SourceFile, node: Node): void;
1151311522
/** Warning: This deletes comments too. See `copyComments` in `convertFunctionToEs6Class`. */
1151411523
deleteNode(sourceFile: SourceFile, node: Node, options?: ConfigurableStartEnd): this;
1151511524
deleteModifier(sourceFile: SourceFile, modifier: Modifier): void;
@@ -11559,6 +11568,7 @@ declare namespace ts.textChanges {
1155911568
insertNodeInListAfter(sourceFile: SourceFile, after: Node, newNode: Node, containingList?: NodeArray<Node> | undefined): this;
1156011569
private finishClassesWithNodesInsertedAtStart;
1156111570
private finishTrailingCommaAfterDeletingNodesInList;
11571+
private finishDeleteDeclarations;
1156211572
/**
1156311573
* Note: after calling this, the TextChanges object must be discarded!
1156411574
* @param validate only for tests

0 commit comments

Comments
 (0)