diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 9fc854515ee..47fd8d85cc5 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -33401,7 +33401,7 @@ namespace ts { function checkUnusedLocalsAndParameters(nodeWithLocals: Node, addDiagnostic: AddUnusedDiagnostic): void { // Ideally we could use the ImportClause directly as a key, but must wait until we have full ES6 maps. So must store key along with value. const unusedImports = new Map(); - const unusedDestructures = new Map(); + const unusedDestructures = new Map(); const unusedVariables = new Map(); nodeWithLocals.locals!.forEach(local => { // If it's purely a type parameter, ignore, will be checked in `checkUnusedTypeParameters`. @@ -33433,7 +33433,12 @@ namespace ts { const name = local.valueDeclaration && getNameOfDeclaration(local.valueDeclaration); if (parameter && name) { if (!isParameterPropertyDeclaration(parameter, parameter.parent) && !parameterIsThisKeyword(parameter) && !isIdentifierThatStartsWithUnderscore(name)) { - addDiagnostic(parameter, UnusedKind.Parameter, createDiagnosticForNode(name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(local))); + if (isBindingElement(declaration) && isArrayBindingPattern(declaration.parent)) { + addToGroup(unusedDestructures, declaration.parent, declaration, getNodeId); + } + else { + addDiagnostic(parameter, UnusedKind.Parameter, createDiagnosticForNode(name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(local))); + } } } else { diff --git a/src/services/codefixes/fixUnusedIdentifier.ts b/src/services/codefixes/fixUnusedIdentifier.ts index a41a08420be..19956a097d9 100644 --- a/src/services/codefixes/fixUnusedIdentifier.ts +++ b/src/services/codefixes/fixUnusedIdentifier.ts @@ -42,7 +42,7 @@ namespace ts.codefix { } } - if (isObjectBindingPattern(token.parent)) { + if (isObjectBindingPattern(token.parent) || isArrayBindingPattern(token.parent)) { if (isParameter(token.parent.parent)) { const elements = token.parent.elements; const diagnostic: [DiagnosticMessage, string] = [ @@ -51,7 +51,7 @@ namespace ts.codefix { ]; return [ createDeleteFix(textChanges.ChangeTracker.with(context, t => - deleteDestructuringElements(t, sourceFile, token.parent)), diagnostic) + deleteDestructuringElements(t, sourceFile, token.parent as ObjectBindingPattern | ArrayBindingPattern)), diagnostic) ]; } return [ @@ -121,13 +121,16 @@ namespace ts.codefix { deleteTypeParameters(changes, sourceFile, token); } else if (isObjectBindingPattern(token.parent)) { - if (isParameter(token.parent.parent)) { - deleteDestructuringElements(changes, sourceFile, token.parent); + if (token.parent.parent.initializer) { + break; } - else { + else if (!isParameter(token.parent.parent) || isNotProvidedArguments(token.parent.parent, checker, sourceFiles)) { changes.delete(sourceFile, token.parent.parent); } } + else if (isArrayBindingPattern(token.parent.parent) && token.parent.parent.parent.initializer) { + break; + } else if (canDeleteEntireVariableStatement(sourceFile, token)) { deleteEntireVariableStatement(changes, sourceFile, token.parent); } @@ -165,7 +168,7 @@ namespace ts.codefix { || token.kind === SyntaxKind.Identifier && (token.parent.kind === SyntaxKind.ImportSpecifier || token.parent.kind === SyntaxKind.ImportClause); } - // Sometimes the diagnostic span is an entire ImportDeclaration, so we should remove the whole thing. + /** Sometimes the diagnostic span is an entire ImportDeclaration, so we should remove the whole thing. */ function tryGetFullImport(token: Node): ImportDeclaration | undefined { return token.kind === SyntaxKind.ImportKeyword ? tryCast(token.parent, isImportDeclaration) : undefined; } @@ -178,7 +181,7 @@ namespace ts.codefix { changes.delete(sourceFile, node.parent.kind === SyntaxKind.VariableStatement ? node.parent : node); } - function deleteDestructuringElements(changes: textChanges.ChangeTracker, sourceFile: SourceFile, node: ObjectBindingPattern) { + function deleteDestructuringElements(changes: textChanges.ChangeTracker, sourceFile: SourceFile, node: ObjectBindingPattern | ArrayBindingPattern) { forEach(node.elements, n => changes.delete(sourceFile, n)); } @@ -219,16 +222,14 @@ namespace ts.codefix { function tryDeleteDeclaration(sourceFile: SourceFile, token: Node, changes: textChanges.ChangeTracker, checker: TypeChecker, sourceFiles: readonly SourceFile[], program: Program, cancellationToken: CancellationToken, isFixAll: boolean) { tryDeleteDeclarationWorker(token, changes, sourceFile, checker, sourceFiles, program, cancellationToken, isFixAll); - if (isIdentifier(token)) deleteAssignments(changes, sourceFile, token, checker); - } - - function deleteAssignments(changes: textChanges.ChangeTracker, sourceFile: SourceFile, token: Identifier, checker: TypeChecker) { - FindAllReferences.Core.eachSymbolReferenceInFile(token, checker, sourceFile, (ref: Node) => { - if (isPropertyAccessExpression(ref.parent) && ref.parent.name === ref) ref = ref.parent; - if (isBinaryExpression(ref.parent) && isExpressionStatement(ref.parent.parent) && ref.parent.left === ref) { - changes.delete(sourceFile, ref.parent.parent); - } - }); + if (isIdentifier(token)) { + FindAllReferences.Core.eachSymbolReferenceInFile(token, checker, sourceFile, (ref: Node) => { + if (isPropertyAccessExpression(ref.parent) && ref.parent.name === ref) ref = ref.parent; + if (!isFixAll && isBinaryExpression(ref.parent) && isExpressionStatement(ref.parent.parent) && ref.parent.left === ref) { + changes.delete(sourceFile, ref.parent.parent); + } + }); + } } function tryDeleteDeclarationWorker(token: Node, changes: textChanges.ChangeTracker, sourceFile: SourceFile, checker: TypeChecker, sourceFiles: readonly SourceFile[], program: Program, cancellationToken: CancellationToken, isFixAll: boolean): void { @@ -236,24 +237,37 @@ namespace ts.codefix { if (isParameter(parent)) { tryDeleteParameter(changes, sourceFile, parent, checker, sourceFiles, program, cancellationToken, isFixAll); } - else { + else if (!isFixAll || !(isIdentifier(token) && FindAllReferences.Core.isSymbolReferencedInFile(token, checker, sourceFile))) { changes.delete(sourceFile, isImportClause(parent) ? token : isComputedPropertyName(parent) ? parent.parent : parent); } } - function tryDeleteParameter(changes: textChanges.ChangeTracker, sourceFile: SourceFile, p: ParameterDeclaration, checker: TypeChecker, sourceFiles: readonly SourceFile[], program: Program, cancellationToken: CancellationToken, isFixAll = false): void { - if (mayDeleteParameter(checker, sourceFile, p, sourceFiles, program, cancellationToken, 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)); + function tryDeleteParameter( + changes: textChanges.ChangeTracker, + sourceFile: SourceFile, + parameter: ParameterDeclaration, + checker: TypeChecker, + sourceFiles: readonly SourceFile[], + program: Program, + cancellationToken: CancellationToken, + isFixAll = false): void { + if (mayDeleteParameter(checker, sourceFile, parameter, sourceFiles, program, cancellationToken, isFixAll)) { + if (parameter.modifiers && parameter.modifiers.length > 0 && + (!isIdentifier(parameter.name) || FindAllReferences.Core.isSymbolReferencedInFile(parameter.name, checker, sourceFile))) { + parameter.modifiers.forEach(modifier => changes.deleteModifier(sourceFile, modifier)); } - else { - changes.delete(sourceFile, p); - deleteUnusedArguments(changes, sourceFile, p, sourceFiles, checker); + else if (!parameter.initializer && isNotProvidedArguments(parameter, checker, sourceFiles)) { + changes.delete(sourceFile, parameter); } } } + function isNotProvidedArguments(parameter: ParameterDeclaration, checker: TypeChecker, sourceFiles: readonly SourceFile[]) { + const index = parameter.parent.parameters.indexOf(parameter); + // Just in case the call didn't provide enough arguments. + return !FindAllReferences.Core.someSignatureUsage(parameter.parent, sourceFiles, checker, (_, call) => !call || call.arguments.length > index); + } + function mayDeleteParameter(checker: TypeChecker, sourceFile: SourceFile, parameter: ParameterDeclaration, sourceFiles: readonly SourceFile[], program: Program, cancellationToken: CancellationToken, isFixAll: boolean): boolean { const { parent } = parameter; switch (parent.kind) { @@ -305,15 +319,6 @@ namespace ts.codefix { } } - function deleteUnusedArguments(changes: textChanges.ChangeTracker, sourceFile: SourceFile, deletedParameter: ParameterDeclaration, sourceFiles: readonly SourceFile[], checker: TypeChecker): void { - FindAllReferences.Core.eachSignatureCall(deletedParameter.parent, sourceFiles, checker, call => { - const index = deletedParameter.parent.parameters.indexOf(deletedParameter); - if (call.arguments.length > index) { // Just in case the call didn't provide enough arguments. - changes.delete(sourceFile, call.arguments[index]); - } - }); - } - 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); diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index d7741036b4f..bc6e1f4cb80 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -1240,8 +1240,13 @@ namespace ts.FindAllReferences { } } - export function eachSignatureCall(signature: SignatureDeclaration, sourceFiles: readonly SourceFile[], checker: TypeChecker, cb: (call: CallExpression) => void): void { - if (!signature.name || !isIdentifier(signature.name)) return; + export function someSignatureUsage( + signature: SignatureDeclaration, + sourceFiles: readonly SourceFile[], + checker: TypeChecker, + cb: (name: Identifier, call?: CallExpression) => boolean + ): boolean { + if (!signature.name || !isIdentifier(signature.name)) return false; const symbol = Debug.checkDefined(checker.getSymbolAtLocation(signature.name)); @@ -1249,14 +1254,16 @@ namespace ts.FindAllReferences { 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 call = isCallExpression(called.parent) && called.parent.expression === called ? called.parent : undefined; const referenceSymbol = checker.getSymbolAtLocation(name); if (referenceSymbol && checker.getRootSymbols(referenceSymbol).some(s => s === symbol)) { - cb(call); + if (cb(name, call)) { + return true; + } } } } + return false; } function getPossibleSymbolReferenceNodes(sourceFile: SourceFile, symbolName: string, container: Node = sourceFile): readonly Node[] { diff --git a/tests/baselines/reference/unusedDestructuringParameters.errors.txt b/tests/baselines/reference/unusedDestructuringParameters.errors.txt index a9423145cbe..d31968e06b5 100644 --- a/tests/baselines/reference/unusedDestructuringParameters.errors.txt +++ b/tests/baselines/reference/unusedDestructuringParameters.errors.txt @@ -1,10 +1,10 @@ -tests/cases/compiler/unusedDestructuringParameters.ts(1,13): error TS6133: 'a' is declared but its value is never read. +tests/cases/compiler/unusedDestructuringParameters.ts(1,12): error TS6133: 'a' is declared but its value is never read. tests/cases/compiler/unusedDestructuringParameters.ts(3,13): error TS6133: 'a' is declared but its value is never read. ==== tests/cases/compiler/unusedDestructuringParameters.ts (2 errors) ==== const f = ([a]) => { }; - ~ + ~~~ !!! error TS6133: 'a' is declared but its value is never read. f([1]); const f2 = ({a}) => { }; diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts index f4c57083066..eeef35e4473 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts @@ -12,8 +12,9 @@ ////function f(a, b) { //// const x = 0; ////} -////function g(a, b, c) { return a; } -////f; g; +////function g(a) { return a; } +////function h(c) { return c; } +////f(); g(); h(); //// ////interface I { //// m(x: number): void; @@ -70,7 +71,8 @@ used1; used2; function f() { } function g(a) { return a; } -f; g; +function h(c) { return c; } +f(); g(); h(); interface I { m(x: number): void; @@ -87,10 +89,10 @@ takesCb(() => {}); takesCb((x) => { x; }); takesCb((x, y) => { y; }); -function fn1(): void {} +function fn1(x: number, y: string): void {} takesCb(fn1); -function fn2(x: number): void { x; } +function fn2(x: number, y: string): void { x; } takesCb(fn2); function fn3(x: number, y: string): void { y; } diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_deleteWrite.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_deleteWrite.ts index 945f79bc010..7d51b4701c4 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_deleteWrite.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_deleteWrite.ts @@ -5,20 +5,10 @@ ////let x = 0; ////x = 1; -//// -////export class C { -//// private p: number; -//// -//// m() { this.p = 0; } -////} +////export {}; -verify.codeFixAll({ - fixId: "unusedIdentifier_delete", - fixAllDescription: ts.Diagnostics.Delete_all_unused_declarations.message, +verify.codeFix({ + description: "Remove unused declaration for: 'x'", newFileContent: -` -export class C { - - m() { } -}`, +`export {};`, }); diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_deleteWrite2.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_deleteWrite2.ts new file mode 100644 index 00000000000..2fe01399355 --- /dev/null +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_deleteWrite2.ts @@ -0,0 +1,19 @@ +/// + +// @noLib: true +// @noUnusedLocals: true + +////export class C { +//// private p: number; +//// +//// m() { this.p = 0; } +////} + +verify.codeFix({ + description: "Remove unused declaration for: 'p'", + newFileContent: +`export class C { + + m() { } +}`, +}); diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts index 5a16f374f34..c9ff5591454 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts @@ -3,20 +3,27 @@ // @noUnusedLocals: true // @noUnusedParameters: true -////const { x, y } = o; -////const { a, b } = o; -////a; -////export function f({ a, b }, { x, y }) { -//// a; -////} +//// const { x, y } = o; +//// const { a, b } = o; +//// a; +//// export function f({ fa, fb }, { fx, fy }) { +//// fb; +//// } +//// export function g([ ga, gb ], [ gary, gygax ]) { +//// ga; +//// } verify.codeFixAll({ fixId: "unusedIdentifier_delete", fixAllDescription: ts.Diagnostics.Delete_all_unused_declarations.message, newFileContent: -`const { a } = o; +`const { x, y } = o; +const { a } = o; a; -export function f({ a }, { }) { - a; +export function f({ fb }) { + fb; +} +export function g([ ga ],) { + ga; }`, }); diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_partlyUnused.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_partlyUnused.ts index e885054f359..1d1553afc4f 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_partlyUnused.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_partlyUnused.ts @@ -57,19 +57,19 @@ verify.codeFixAll({ x; z; } { - const [x] = o; + const [x, y] = o; x; } { - const [, y] = o; + const [x, y] = o; y; } { - const [, y] = o; + const [x, y, z] = o; y; } { - const [x,, z] = o; + const [x, y, z] = o; x; z; }`, }); diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_parameter.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_parameter.ts new file mode 100644 index 00000000000..ee1b066b823 --- /dev/null +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_parameter.ts @@ -0,0 +1,9 @@ +/// + +// @noUnusedLocals: true +// @noUnusedParameters: true + +////function g(a, b) { b; } +////g(1, 2); + +verify.not.codeFixAvailable("Remove unused declaration for: 'a'"); diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_parameter_all.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_parameter_all.ts index 16c9e6b4815..cc9becb33eb 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_parameter_all.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_parameter_all.ts @@ -5,37 +5,75 @@ ////function f(a, b, { x, y }) { b; } ////f(0, 1, { x: 1, y: 1 }); +////function g(a, b, { x, y }: { x: number, y: number }) { b; } +////g(); +////function h(a, b?) { a; } +////h(1); +////function i(x = 1) { } +////i(); +//// +////function container(o) { +//// const { x, y } = { x: 1, y: 2 } +//// const { a, b } = o +//// const [ z, ka ] = [ 3, 4 ] +//// const [ c, d ] = o +////} //// ////class C { //// m(a, b, c) { b; } +//// n(a, b, c) { b; } ////} ////new C().m(0, 1, 2); +////new C().n(); //// ////// Test of deletedAncestors ////function a(a: any, unused: any) { a; } ////function b(a: any, unused: any) { a; } +////function c(a: any, unused: any) { a; } //// ////b(1, { //// prop: a(2, [ //// b(3, a(4, undefined)), //// ]), ////}); +////b(1, { prop: c() }); verify.codeFixAll({ fixId: "unusedIdentifier_delete", fixAllDescription: ts.Diagnostics.Delete_all_unused_declarations.message, newFileContent: -`function f(b, { }) { b; } -f(1, { x: 1, y: 1 }); +`function f(a, b, { x, y }) { b; } +f(0, 1, { x: 1, y: 1 }); +function g(b) { b; } +g(); +function h(a) { a; } +h(1); +function i(x = 1) { } +i(); + +function container(o) { + const { x, y } = { x: 1, y: 2 } + const { a, b } = o + const [ z, ka ] = [ 3, 4 ] + const [ c, d ] = o +} class C { - m(b) { b; } + m(a, b, c) { b; } + n(b) { b; } } -new C().m(1); +new C().m(0, 1, 2); +new C().n(); // Test of deletedAncestors -function a(a: any) { a; } -function b(a: any) { a; } +function a(a: any, unused: any) { a; } +function b(a: any, unused: any) { a; } +function c(a: any) { a; } -b(1);`, +b(1, { + prop: a(2, [ + b(3, a(4, undefined)), + ]), +}); +b(1, { prop: c() });`, });