Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 5725428

Browse filesBrowse files
author
Andy
authored
fixUnusedIdentifier: Handle destructure with all bindings unused (microsoft#23805)
* fixUnusedIdentifier: Handle destructure with all bindings unused * Add parameters test * Add test for 'for' loop
1 parent 556c316 commit 5725428
Copy full SHA for 5725428

16 files changed

+332-80Lines changed: 332 additions & 80 deletions
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/compiler/checker.ts‎

Copy file name to clipboardExpand all lines: src/compiler/checker.ts
+77-68Lines changed: 77 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -22352,10 +22352,6 @@ namespace ts {
2235222352
function checkUnusedIdentifiers(potentiallyUnusedIdentifiers: ReadonlyArray<PotentiallyUnusedIdentifier>, addDiagnostic: AddUnusedDiagnostic) {
2235322353
for (const node of potentiallyUnusedIdentifiers) {
2235422354
switch (node.kind) {
22355-
case SyntaxKind.SourceFile:
22356-
case SyntaxKind.ModuleDeclaration:
22357-
checkUnusedModuleMembers(node, addDiagnostic);
22358-
break;
2235922355
case SyntaxKind.ClassDeclaration:
2236022356
case SyntaxKind.ClassExpression:
2236122357
checkUnusedClassMembers(node, addDiagnostic);
@@ -22364,6 +22360,8 @@ namespace ts {
2236422360
case SyntaxKind.InterfaceDeclaration:
2236522361
checkUnusedTypeParameters(node, addDiagnostic);
2236622362
break;
22363+
case SyntaxKind.SourceFile:
22364+
case SyntaxKind.ModuleDeclaration:
2236722365
case SyntaxKind.Block:
2236822366
case SyntaxKind.CaseBlock:
2236922367
case SyntaxKind.ForStatement:
@@ -22397,35 +22395,6 @@ namespace ts {
2239722395
}
2239822396
}
2239922397

22400-
function checkUnusedLocalsAndParameters(node: Node, addDiagnostic: AddUnusedDiagnostic): void {
22401-
if (!(node.flags & NodeFlags.Ambient)) {
22402-
node.locals.forEach(local => {
22403-
// If it's purely a type parameter, ignore, will be checked in `checkUnusedTypeParameters`.
22404-
// If it's a type parameter merged with a parameter, check if the parameter-side is used.
22405-
if (local.flags & SymbolFlags.TypeParameter ? (local.flags & SymbolFlags.Variable && !(local.isReferenced & SymbolFlags.Variable)) : !local.isReferenced) {
22406-
if (local.valueDeclaration && getRootDeclaration(local.valueDeclaration).kind === SyntaxKind.Parameter) {
22407-
const parameter = <ParameterDeclaration>getRootDeclaration(local.valueDeclaration);
22408-
const name = getNameOfDeclaration(local.valueDeclaration);
22409-
if (!isParameterPropertyDeclaration(parameter) && !parameterIsThisKeyword(parameter) && !parameterNameStartsWithUnderscore(name)) {
22410-
addDiagnostic(UnusedKind.Parameter, createDiagnosticForNode(name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(local)));
22411-
}
22412-
}
22413-
else {
22414-
forEach(local.declarations, d => errorUnusedLocal(d, symbolName(local), addDiagnostic));
22415-
}
22416-
}
22417-
});
22418-
}
22419-
}
22420-
22421-
function isRemovedPropertyFromObjectSpread(node: Node) {
22422-
if (isBindingElement(node) && isObjectBindingPattern(node.parent)) {
22423-
const lastElement = lastOrUndefined(node.parent.elements);
22424-
return lastElement !== node && !!lastElement.dotDotDotToken;
22425-
}
22426-
return false;
22427-
}
22428-
2242922398
function errorUnusedLocal(declaration: Declaration, name: string, addDiagnostic: AddUnusedDiagnostic) {
2243022399
const node = getNameOfDeclaration(declaration) || declaration;
2243122400
if (isIdentifierThatStartsWithUnderScore(node)) {
@@ -22436,10 +22405,8 @@ namespace ts {
2243622405
}
2243722406
}
2243822407

22439-
if (!isRemovedPropertyFromObjectSpread(node.kind === SyntaxKind.Identifier ? node.parent : node)) {
22440-
const message = isTypeDeclaration(declaration) ? Diagnostics._0_is_declared_but_never_used : Diagnostics._0_is_declared_but_its_value_is_never_read;
22441-
addDiagnostic(UnusedKind.Local, createDiagnosticForNodeSpan(getSourceFileOfNode(declaration), declaration, node, message, name));
22442-
}
22408+
const message = isTypeDeclaration(declaration) ? Diagnostics._0_is_declared_but_never_used : Diagnostics._0_is_declared_but_its_value_is_never_read;
22409+
addDiagnostic(UnusedKind.Local, createDiagnosticForNodeSpan(getSourceFileOfNode(declaration), declaration, node, message, name));
2244322410
}
2244422411

2244522412
function parameterNameStartsWithUnderscore(parameterName: DeclarationName) {
@@ -22501,44 +22468,86 @@ namespace ts {
2250122468
}
2250222469
}
2250322470

22504-
function checkUnusedModuleMembers(node: ModuleDeclaration | SourceFile, addDiagnostic: AddUnusedDiagnostic): void {
22505-
if (!(node.flags & NodeFlags.Ambient)) {
22506-
// 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.
22507-
const unusedImports = createMap<[ImportClause, ImportedDeclaration[]]>();
22508-
node.locals.forEach(local => {
22509-
if (local.isReferenced || local.exportSymbol) return;
22510-
for (const declaration of local.declarations) {
22511-
if (isAmbientModule(declaration)) continue;
22512-
if (isImportedDeclaration(declaration)) {
22513-
const importClause = importClauseFromImported(declaration);
22514-
const key = String(getNodeId(importClause));
22515-
const group = unusedImports.get(key);
22516-
if (group) {
22517-
group[1].push(declaration);
22518-
}
22519-
else {
22520-
unusedImports.set(key, [importClause, [declaration]]);
22471+
function addToGroup<K, V>(map: Map<[K, V[]]>, key: K, value: V, getKey: (key: K) => number | string): void {
22472+
const keyString = String(getKey(key));
22473+
const group = map.get(keyString);
22474+
if (group) {
22475+
group[1].push(value);
22476+
}
22477+
else {
22478+
map.set(keyString, [key, [value]]);
22479+
}
22480+
}
22481+
22482+
function tryGetRootParameterDeclaration(node: Node): ParameterDeclaration | undefined {
22483+
return tryCast(getRootDeclaration(node), isParameter);
22484+
}
22485+
22486+
function checkUnusedLocalsAndParameters(nodeWithLocals: Node, addDiagnostic: AddUnusedDiagnostic): void {
22487+
if (nodeWithLocals.flags & NodeFlags.Ambient) return;
22488+
22489+
// 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.
22490+
const unusedImports = createMap<[ImportClause, ImportedDeclaration[]]>();
22491+
const unusedDestructures = createMap<[ObjectBindingPattern, BindingElement[]]>();
22492+
nodeWithLocals.locals.forEach(local => {
22493+
// If it's purely a type parameter, ignore, will be checked in `checkUnusedTypeParameters`.
22494+
// If it's a type parameter merged with a parameter, check if the parameter-side is used.
22495+
if (local.flags & SymbolFlags.TypeParameter ? !(local.flags & SymbolFlags.Variable && !(local.isReferenced & SymbolFlags.Variable)) : local.isReferenced || local.exportSymbol) {
22496+
return;
22497+
}
22498+
22499+
for (const declaration of local.declarations) {
22500+
if (isAmbientModule(declaration)) continue;
22501+
if (isImportedDeclaration(declaration)) {
22502+
addToGroup(unusedImports, importClauseFromImported(declaration), declaration, getNodeId);
22503+
}
22504+
else if (isBindingElement(declaration) && isObjectBindingPattern(declaration.parent)) {
22505+
// In `{ a, ...b }, `a` is considered used since it removes a property from `b`. `b` may still be unused though.
22506+
const lastElement = last(declaration.parent.elements);
22507+
if (declaration === lastElement || !last(declaration.parent.elements).dotDotDotToken) {
22508+
addToGroup(unusedDestructures, declaration.parent, declaration, getNodeId);
22509+
}
22510+
}
22511+
else {
22512+
const parameter = local.valueDeclaration && tryGetRootParameterDeclaration(local.valueDeclaration);
22513+
if (parameter) {
22514+
const name = getNameOfDeclaration(local.valueDeclaration);
22515+
if (!isParameterPropertyDeclaration(parameter) && !parameterIsThisKeyword(parameter) && !parameterNameStartsWithUnderscore(name)) {
22516+
addDiagnostic(UnusedKind.Parameter, createDiagnosticForNode(name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(local)));
2252122517
}
2252222518
}
2252322519
else {
2252422520
errorUnusedLocal(declaration, symbolName(local), addDiagnostic);
2252522521
}
2252622522
}
22527-
});
22528-
22529-
unusedImports.forEach(([importClause, unuseds]) => {
22530-
const importDecl = importClause.parent;
22531-
if (forEachImportedDeclaration(importClause, d => !contains(unuseds, d))) {
22532-
for (const unused of unuseds) errorUnusedLocal(unused, idText(unused.name), addDiagnostic);
22533-
}
22534-
else if (unuseds.length === 1) {
22535-
addDiagnostic(UnusedKind.Local, createDiagnosticForNode(importDecl, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(first(unuseds).name)));
22536-
}
22537-
else {
22538-
addDiagnostic(UnusedKind.Local, createDiagnosticForNode(importDecl, Diagnostics.All_imports_in_import_declaration_are_unused, showModuleSpecifier(importDecl)));
22523+
}
22524+
});
22525+
unusedImports.forEach(([importClause, unuseds]) => {
22526+
const importDecl = importClause.parent;
22527+
if (forEachImportedDeclaration(importClause, d => !contains(unuseds, d))) {
22528+
for (const unused of unuseds) errorUnusedLocal(unused, idText(unused.name), addDiagnostic);
22529+
}
22530+
else if (unuseds.length === 1) {
22531+
addDiagnostic(UnusedKind.Local, createDiagnosticForNode(importDecl, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(first(unuseds).name)));
22532+
}
22533+
else {
22534+
addDiagnostic(UnusedKind.Local, createDiagnosticForNode(importDecl, Diagnostics.All_imports_in_import_declaration_are_unused));
22535+
}
22536+
});
22537+
unusedDestructures.forEach(([bindingPattern, bindingElements]) => {
22538+
const kind = tryGetRootParameterDeclaration(bindingPattern.parent) ? UnusedKind.Parameter : UnusedKind.Local;
22539+
if (!bindingPattern.elements.every(e => contains(bindingElements, e))) {
22540+
for (const e of bindingElements) {
22541+
addDiagnostic(kind, createDiagnosticForNode(e, Diagnostics._0_is_declared_but_its_value_is_never_read, getBindingElementNameText(e)));
2253922542
}
22540-
});
22541-
}
22543+
}
22544+
else if (bindingElements.length === 1) {
22545+
addDiagnostic(kind, createDiagnosticForNode(bindingPattern, Diagnostics._0_is_declared_but_its_value_is_never_read, getBindingElementNameText(first(bindingElements))));
22546+
}
22547+
else {
22548+
addDiagnostic(kind, createDiagnosticForNode(bindingPattern, Diagnostics.All_destructured_elements_are_unused));
22549+
}
22550+
});
2254222551
}
2254322552

2254422553
type ImportedDeclaration = ImportClause | ImportSpecifier | NamespaceImport;
Collapse file

‎src/compiler/diagnosticMessages.json‎

Copy file name to clipboardExpand all lines: src/compiler/diagnosticMessages.json
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3551,6 +3551,11 @@
35513551
"category": "Message",
35523552
"code": 6197
35533553
},
3554+
"All destructured elements are unused.": {
3555+
"category": "Error",
3556+
"code": 6198,
3557+
"reportsUnnecessary": true
3558+
},
35543559

35553560
"Projects to reference": {
35563561
"category": "Message",
@@ -3985,6 +3990,10 @@
39853990
"category": "Message",
39863991
"code": 90008
39873992
},
3993+
"Remove destructuring": {
3994+
"category": "Message",
3995+
"code": 90009
3996+
},
39883997
"Import '{0}' from module \"{1}\"": {
39893998
"category": "Message",
39903999
"code": 90013
Collapse file

‎src/services/codefixes/fixUnusedIdentifier.ts‎

Copy file name to clipboardExpand all lines: src/services/codefixes/fixUnusedIdentifier.ts
+28-1Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ namespace ts.codefix {
88
Diagnostics._0_is_declared_but_never_used.code,
99
Diagnostics.Property_0_is_declared_but_its_value_is_never_read.code,
1010
Diagnostics.All_imports_in_import_declaration_are_unused.code,
11+
Diagnostics.All_destructured_elements_are_unused.code,
1112
];
1213
registerCodeFix({
1314
errorCodes,
@@ -18,6 +19,10 @@ namespace ts.codefix {
1819
const changes = textChanges.ChangeTracker.with(context, t => t.deleteNode(sourceFile, importDecl));
1920
return [createCodeFixAction(fixName, changes, [Diagnostics.Remove_import_from_0, showModuleSpecifier(importDecl)], fixIdDelete, Diagnostics.Delete_all_unused_declarations)];
2021
}
22+
const delDestructure = textChanges.ChangeTracker.with(context, t => tryDeleteFullDestructure(t, sourceFile, context.span.start));
23+
if (delDestructure.length) {
24+
return [createCodeFixAction(fixName, delDestructure, Diagnostics.Remove_destructuring, fixIdDelete, Diagnostics.Delete_all_unused_declarations)];
25+
}
2126

2227
const token = getToken(sourceFile, textSpanEnd(context.span));
2328
const result: CodeFixAction[] = [];
@@ -50,7 +55,9 @@ namespace ts.codefix {
5055
changes.deleteNode(sourceFile, importDecl);
5156
}
5257
else {
53-
tryDeleteDeclaration(changes, sourceFile, token);
58+
if (!tryDeleteFullDestructure(changes, sourceFile, diag.start!)) {
59+
tryDeleteDeclaration(changes, sourceFile, token);
60+
}
5461
}
5562
break;
5663
default:
@@ -65,6 +72,26 @@ namespace ts.codefix {
6572
return startToken.kind === SyntaxKind.ImportKeyword ? tryCast(startToken.parent, isImportDeclaration) : undefined;
6673
}
6774

75+
function tryDeleteFullDestructure(changes: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number): boolean {
76+
const startToken = getTokenAtPosition(sourceFile, pos, /*includeJsDocComment*/ false);
77+
if (startToken.kind !== SyntaxKind.OpenBraceToken || !isObjectBindingPattern(startToken.parent)) return false;
78+
const decl = startToken.parent.parent;
79+
switch (decl.kind) {
80+
case SyntaxKind.VariableDeclaration:
81+
tryDeleteVariableDeclaration(changes, sourceFile, decl);
82+
break;
83+
case SyntaxKind.Parameter:
84+
changes.deleteNodeInList(sourceFile, decl);
85+
break;
86+
case SyntaxKind.BindingElement:
87+
changes.deleteNode(sourceFile, decl);
88+
break;
89+
default:
90+
return Debug.assertNever(decl);
91+
}
92+
return true;
93+
}
94+
6895
function getToken(sourceFile: SourceFile, pos: number): Node {
6996
const token = findPrecedingToken(pos, sourceFile);
7097
// this handles var ["computed"] = 12;
Collapse file

‎tests/baselines/reference/unusedDestructuring.errors.txt‎

Copy file name to clipboard
+35Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
tests/cases/compiler/unusedDestructuring.ts(3,7): error TS6198: All destructured elements are unused.
2+
tests/cases/compiler/unusedDestructuring.ts(4,9): error TS6133: 'c' is declared but its value is never read.
3+
tests/cases/compiler/unusedDestructuring.ts(6,7): error TS6133: 'e' is declared but its value is never read.
4+
tests/cases/compiler/unusedDestructuring.ts(8,1): error TS6133: 'f' is declared but its value is never read.
5+
tests/cases/compiler/unusedDestructuring.ts(8,12): error TS6198: All destructured elements are unused.
6+
tests/cases/compiler/unusedDestructuring.ts(8,24): error TS6133: 'c' is declared but its value is never read.
7+
tests/cases/compiler/unusedDestructuring.ts(8,32): error TS6133: 'e' is declared but its value is never read.
8+
9+
10+
==== tests/cases/compiler/unusedDestructuring.ts (7 errors) ====
11+
export {};
12+
declare const o: any;
13+
const { a, b } = o;
14+
~~~~~~~~
15+
!!! error TS6198: All destructured elements are unused.
16+
const { c, d } = o;
17+
~
18+
!!! error TS6133: 'c' is declared but its value is never read.
19+
d;
20+
const { e } = o;
21+
~~~~~
22+
!!! error TS6133: 'e' is declared but its value is never read.
23+
24+
function f({ a, b }, { c, d }, { e }) {
25+
~~~~~~~~~~
26+
!!! error TS6133: 'f' is declared but its value is never read.
27+
~~~~~~~~
28+
!!! error TS6198: All destructured elements are unused.
29+
~
30+
!!! error TS6133: 'c' is declared but its value is never read.
31+
~~~~~
32+
!!! error TS6133: 'e' is declared but its value is never read.
33+
d;
34+
}
35+
Collapse file

‎tests/baselines/reference/unusedDestructuring.js‎

Copy file name to clipboard
+26Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//// [unusedDestructuring.ts]
2+
export {};
3+
declare const o: any;
4+
const { a, b } = o;
5+
const { c, d } = o;
6+
d;
7+
const { e } = o;
8+
9+
function f({ a, b }, { c, d }, { e }) {
10+
d;
11+
}
12+
13+
14+
//// [unusedDestructuring.js]
15+
"use strict";
16+
exports.__esModule = true;
17+
var a = o.a, b = o.b;
18+
var c = o.c, d = o.d;
19+
d;
20+
var e = o.e;
21+
function f(_a, _b, _c) {
22+
var a = _a.a, b = _a.b;
23+
var c = _b.c, d = _b.d;
24+
var e = _c.e;
25+
d;
26+
}
Collapse file

‎tests/baselines/reference/unusedDestructuring.symbols‎

Copy file name to clipboard
+34Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
=== tests/cases/compiler/unusedDestructuring.ts ===
2+
export {};
3+
declare const o: any;
4+
>o : Symbol(o, Decl(unusedDestructuring.ts, 1, 13))
5+
6+
const { a, b } = o;
7+
>a : Symbol(a, Decl(unusedDestructuring.ts, 2, 7))
8+
>b : Symbol(b, Decl(unusedDestructuring.ts, 2, 10))
9+
>o : Symbol(o, Decl(unusedDestructuring.ts, 1, 13))
10+
11+
const { c, d } = o;
12+
>c : Symbol(c, Decl(unusedDestructuring.ts, 3, 7))
13+
>d : Symbol(d, Decl(unusedDestructuring.ts, 3, 10))
14+
>o : Symbol(o, Decl(unusedDestructuring.ts, 1, 13))
15+
16+
d;
17+
>d : Symbol(d, Decl(unusedDestructuring.ts, 3, 10))
18+
19+
const { e } = o;
20+
>e : Symbol(e, Decl(unusedDestructuring.ts, 5, 7))
21+
>o : Symbol(o, Decl(unusedDestructuring.ts, 1, 13))
22+
23+
function f({ a, b }, { c, d }, { e }) {
24+
>f : Symbol(f, Decl(unusedDestructuring.ts, 5, 16))
25+
>a : Symbol(a, Decl(unusedDestructuring.ts, 7, 12))
26+
>b : Symbol(b, Decl(unusedDestructuring.ts, 7, 15))
27+
>c : Symbol(c, Decl(unusedDestructuring.ts, 7, 22))
28+
>d : Symbol(d, Decl(unusedDestructuring.ts, 7, 25))
29+
>e : Symbol(e, Decl(unusedDestructuring.ts, 7, 32))
30+
31+
d;
32+
>d : Symbol(d, Decl(unusedDestructuring.ts, 7, 25))
33+
}
34+

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.