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 802dc2b

Browse filesBrowse files
author
Andy
authored
fixUnusedIdentifier: If every VariableDeclaration is unused, remove the VariableStatement (microsoft#24231)
1 parent d2f6f6a commit 802dc2b
Copy full SHA for 802dc2b

11 files changed

+129-63Lines changed: 129 additions & 63 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
+47-24Lines changed: 47 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -22620,14 +22620,6 @@ namespace ts {
2262022620

2262122621
function errorUnusedLocal(declaration: Declaration, name: string, addDiagnostic: AddUnusedDiagnostic) {
2262222622
const node = getNameOfDeclaration(declaration) || declaration;
22623-
if (isIdentifierThatStartsWithUnderScore(node)) {
22624-
const declaration = getRootDeclaration(node.parent);
22625-
if ((declaration.kind === SyntaxKind.VariableDeclaration && isForInOrOfStatement(declaration.parent.parent)) ||
22626-
declaration.kind === SyntaxKind.TypeParameter) {
22627-
return;
22628-
}
22629-
}
22630-
2263122623
const message = isTypeDeclaration(declaration) ? Diagnostics._0_is_declared_but_never_used : Diagnostics._0_is_declared_but_its_value_is_never_read;
2263222624
addDiagnostic(UnusedKind.Local, createDiagnosticForNodeSpan(getSourceFileOfNode(declaration), declaration, node, message, name));
2263322625
}
@@ -22712,6 +22704,7 @@ namespace ts {
2271222704
// 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.
2271322705
const unusedImports = createMap<[ImportClause, ImportedDeclaration[]]>();
2271422706
const unusedDestructures = createMap<[ObjectBindingPattern, BindingElement[]]>();
22707+
const unusedVariables = createMap<[VariableDeclarationList, VariableDeclaration[]]>();
2271522708
nodeWithLocals.locals.forEach(local => {
2271622709
// If it's purely a type parameter, ignore, will be checked in `checkUnusedTypeParameters`.
2271722710
// If it's a type parameter merged with a parameter, check if the parameter-side is used.
@@ -22731,6 +22724,11 @@ namespace ts {
2273122724
addToGroup(unusedDestructures, declaration.parent, declaration, getNodeId);
2273222725
}
2273322726
}
22727+
else if (isVariableDeclaration(declaration)) {
22728+
if (!isIdentifierThatStartsWithUnderScore(declaration.name) || !isForInOrOfStatement(declaration.parent.parent)) {
22729+
addToGroup(unusedVariables, declaration.parent, declaration, getNodeId);
22730+
}
22731+
}
2273422732
else {
2273522733
const parameter = local.valueDeclaration && tryGetRootParameterDeclaration(local.valueDeclaration);
2273622734
if (parameter) {
@@ -22747,32 +22745,63 @@ namespace ts {
2274722745
});
2274822746
unusedImports.forEach(([importClause, unuseds]) => {
2274922747
const importDecl = importClause.parent;
22750-
if (forEachImportedDeclaration(importClause, d => !contains(unuseds, d))) {
22751-
for (const unused of unuseds) errorUnusedLocal(unused, idText(unused.name), addDiagnostic);
22752-
}
22753-
else if (unuseds.length === 1) {
22754-
addDiagnostic(UnusedKind.Local, createDiagnosticForNode(importDecl, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(first(unuseds).name)));
22748+
const nDeclarations = (importClause.name ? 1 : 0) +
22749+
(importClause.namedBindings ?
22750+
(importClause.namedBindings.kind === SyntaxKind.NamespaceImport ? 1 : importClause.namedBindings.elements.length)
22751+
: 0);
22752+
if (nDeclarations === unuseds.length) {
22753+
addDiagnostic(UnusedKind.Local, unuseds.length === 1
22754+
? createDiagnosticForNode(importDecl, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(first(unuseds).name))
22755+
: createDiagnosticForNode(importDecl, Diagnostics.All_imports_in_import_declaration_are_unused));
2275522756
}
2275622757
else {
22757-
addDiagnostic(UnusedKind.Local, createDiagnosticForNode(importDecl, Diagnostics.All_imports_in_import_declaration_are_unused));
22758+
for (const unused of unuseds) errorUnusedLocal(unused, idText(unused.name), addDiagnostic);
2275822759
}
2275922760
});
2276022761
unusedDestructures.forEach(([bindingPattern, bindingElements]) => {
2276122762
const kind = tryGetRootParameterDeclaration(bindingPattern.parent) ? UnusedKind.Parameter : UnusedKind.Local;
22762-
if (!bindingPattern.elements.every(e => contains(bindingElements, e))) {
22763+
if (bindingPattern.elements.length === bindingElements.length) {
22764+
if (bindingElements.length === 1 && bindingPattern.parent.kind === SyntaxKind.VariableDeclaration && bindingPattern.parent.parent.kind === SyntaxKind.VariableDeclarationList) {
22765+
addToGroup(unusedVariables, bindingPattern.parent.parent, bindingPattern.parent, getNodeId);
22766+
}
22767+
else {
22768+
addDiagnostic(kind, bindingElements.length === 1
22769+
? createDiagnosticForNode(bindingPattern, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(cast(first(bindingElements).name, isIdentifier)))
22770+
: createDiagnosticForNode(bindingPattern, Diagnostics.All_destructured_elements_are_unused));
22771+
}
22772+
}
22773+
else {
2276322774
for (const e of bindingElements) {
2276422775
addDiagnostic(kind, createDiagnosticForNode(e, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(cast(e.name, isIdentifier))));
2276522776
}
2276622777
}
22767-
else if (bindingElements.length === 1) {
22768-
addDiagnostic(kind, createDiagnosticForNode(bindingPattern, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(cast(first(bindingElements).name, isIdentifier))));
22778+
});
22779+
unusedVariables.forEach(([declarationList, declarations]) => {
22780+
if (declarationList.declarations.length === declarations.length) {
22781+
addDiagnostic(UnusedKind.Local, declarations.length === 1
22782+
? createDiagnosticForNode(first(declarations).name, Diagnostics._0_is_declared_but_its_value_is_never_read, bindingNameText(first(declarations).name))
22783+
: createDiagnosticForNode(declarationList.parent.kind === SyntaxKind.VariableStatement ? declarationList.parent : declarationList, Diagnostics.All_variables_are_unused));
2276922784
}
2277022785
else {
22771-
addDiagnostic(kind, createDiagnosticForNode(bindingPattern, Diagnostics.All_destructured_elements_are_unused));
22786+
for (const decl of declarations) {
22787+
addDiagnostic(UnusedKind.Local, createDiagnosticForNode(decl, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(cast(decl.name, isIdentifier))));
22788+
}
2277222789
}
2277322790
});
2277422791
}
2277522792

22793+
function bindingNameText(name: BindingName): string {
22794+
switch (name.kind) {
22795+
case SyntaxKind.Identifier:
22796+
return idText(name);
22797+
case SyntaxKind.ArrayBindingPattern:
22798+
case SyntaxKind.ObjectBindingPattern:
22799+
return bindingNameText(cast(first(name.elements), isBindingElement).name);
22800+
default:
22801+
return Debug.assertNever(name);
22802+
}
22803+
}
22804+
2277622805
type ImportedDeclaration = ImportClause | ImportSpecifier | NamespaceImport;
2277722806
function isImportedDeclaration(node: Node): node is ImportedDeclaration {
2277822807
return node.kind === SyntaxKind.ImportClause || node.kind === SyntaxKind.ImportSpecifier || node.kind === SyntaxKind.NamespaceImport;
@@ -22781,12 +22810,6 @@ namespace ts {
2278122810
return decl.kind === SyntaxKind.ImportClause ? decl : decl.kind === SyntaxKind.NamespaceImport ? decl.parent : decl.parent.parent;
2278222811
}
2278322812

22784-
function forEachImportedDeclaration<T>(importClause: ImportClause, cb: (im: ImportedDeclaration) => T | undefined): T | undefined {
22785-
const { name: defaultName, namedBindings } = importClause;
22786-
return (defaultName && cb(importClause)) ||
22787-
namedBindings && (namedBindings.kind === SyntaxKind.NamespaceImport ? cb(namedBindings) : forEach(namedBindings.elements, cb));
22788-
}
22789-
2279022813
function checkBlock(node: Block) {
2279122814
// Grammar checking for SyntaxKind.Block
2279222815
if (node.kind === SyntaxKind.Block) {
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
@@ -3556,6 +3556,11 @@
35563556
"code": 6198,
35573557
"reportsUnnecessary": true
35583558
},
3559+
"All variables are unused.": {
3560+
"category": "Error",
3561+
"code": 6199,
3562+
"reportsUnnecessary": true
3563+
},
35593564

35603565
"Projects to reference": {
35613566
"category": "Message",
@@ -3994,6 +3999,10 @@
39943999
"category": "Message",
39954000
"code": 90009
39964001
},
4002+
"Remove variable statement": {
4003+
"category": "Message",
4004+
"code": 90010
4005+
},
39974006
"Import '{0}' from module \"{1}\"": {
39984007
"category": "Message",
39994008
"code": 90013
Collapse file

‎src/services/codefixes/fixUnusedIdentifier.ts‎

Copy file name to clipboardExpand all lines: src/services/codefixes/fixUnusedIdentifier.ts
+26-12Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,27 @@ namespace ts.codefix {
99
Diagnostics.Property_0_is_declared_but_its_value_is_never_read.code,
1010
Diagnostics.All_imports_in_import_declaration_are_unused.code,
1111
Diagnostics.All_destructured_elements_are_unused.code,
12+
Diagnostics.All_variables_are_unused.code,
1213
];
1314
registerCodeFix({
1415
errorCodes,
1516
getCodeActions(context) {
1617
const { errorCode, sourceFile } = context;
17-
const importDecl = tryGetFullImport(sourceFile, context.span.start);
18+
const startToken = getTokenAtPosition(sourceFile, context.span.start, /*includeJsDocComment*/ false);
19+
20+
const importDecl = tryGetFullImport(startToken);
1821
if (importDecl) {
1922
const changes = textChanges.ChangeTracker.with(context, t => t.deleteNode(sourceFile, importDecl));
2023
return [createCodeFixAction(fixName, changes, [Diagnostics.Remove_import_from_0, showModuleSpecifier(importDecl)], fixIdDelete, Diagnostics.Delete_all_unused_declarations)];
2124
}
22-
const delDestructure = textChanges.ChangeTracker.with(context, t => tryDeleteFullDestructure(t, sourceFile, context.span.start, /*deleted*/ undefined));
25+
const delDestructure = textChanges.ChangeTracker.with(context, t => tryDeleteFullDestructure(t, sourceFile, startToken, /*deleted*/ undefined));
2326
if (delDestructure.length) {
2427
return [createCodeFixAction(fixName, delDestructure, Diagnostics.Remove_destructuring, fixIdDelete, Diagnostics.Delete_all_unused_declarations)];
2528
}
29+
const delVar = textChanges.ChangeTracker.with(context, t => tryDeleteFullVariableStatement(t, sourceFile, startToken, /*deleted*/ undefined));
30+
if (delVar.length) {
31+
return [createCodeFixAction(fixName, delDestructure, Diagnostics.Remove_variable_statement, fixIdDelete, Diagnostics.Delete_all_unused_declarations)];
32+
}
2633

2734
const token = getToken(sourceFile, textSpanEnd(context.span));
2835
const result: CodeFixAction[] = [];
@@ -45,6 +52,7 @@ namespace ts.codefix {
4552
const deleted = new NodeSet();
4653
return codeFixAll(context, errorCodes, (changes, diag) => {
4754
const { sourceFile } = context;
55+
const startToken = getTokenAtPosition(sourceFile, diag.start, /*includeJsDocComment*/ false);
4856
const token = findPrecedingToken(textSpanEnd(diag), diag.file!);
4957
switch (context.fixId) {
5058
case fixIdPrefix:
@@ -56,14 +64,12 @@ namespace ts.codefix {
5664
// Ignore if this range was already deleted.
5765
if (deleted.some(d => rangeContainsPosition(d, diag.start!))) break;
5866

59-
const importDecl = tryGetFullImport(diag.file!, diag.start!);
67+
const importDecl = tryGetFullImport(startToken);
6068
if (importDecl) {
6169
changes.deleteNode(sourceFile, importDecl);
6270
}
63-
else {
64-
if (!tryDeleteFullDestructure(changes, sourceFile, diag.start!, deleted)) {
65-
tryDeleteDeclaration(changes, sourceFile, token, deleted);
66-
}
71+
else if (!tryDeleteFullDestructure(changes, sourceFile, startToken, deleted) && !tryDeleteFullVariableStatement(changes, sourceFile, startToken, deleted)) {
72+
tryDeleteDeclaration(changes, sourceFile, token, deleted);
6773
}
6874
break;
6975
default:
@@ -74,15 +80,13 @@ namespace ts.codefix {
7480
});
7581

7682
// Sometimes the diagnostic span is an entire ImportDeclaration, so we should remove the whole thing.
77-
function tryGetFullImport(sourceFile: SourceFile, pos: number): ImportDeclaration | undefined {
78-
const startToken = getTokenAtPosition(sourceFile, pos, /*includeJsDocComment*/ false);
83+
function tryGetFullImport(startToken: Node): ImportDeclaration | undefined {
7984
return startToken.kind === SyntaxKind.ImportKeyword ? tryCast(startToken.parent, isImportDeclaration) : undefined;
8085
}
8186

82-
function tryDeleteFullDestructure(changes: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number, deletedAncestors: NodeSet | undefined): boolean {
83-
const startToken = getTokenAtPosition(sourceFile, pos, /*includeJsDocComment*/ false);
87+
function tryDeleteFullDestructure(changes: textChanges.ChangeTracker, sourceFile: SourceFile, startToken: Node, deletedAncestors: NodeSet | undefined): boolean {
8488
if (startToken.kind !== SyntaxKind.OpenBraceToken || !isObjectBindingPattern(startToken.parent)) return false;
85-
const decl = startToken.parent.parent;
89+
const decl = cast(startToken.parent, isObjectBindingPattern).parent;
8690
switch (decl.kind) {
8791
case SyntaxKind.VariableDeclaration:
8892
tryDeleteVariableDeclaration(changes, sourceFile, decl, deletedAncestors);
@@ -101,6 +105,16 @@ namespace ts.codefix {
101105
return true;
102106
}
103107

108+
function tryDeleteFullVariableStatement(changes: textChanges.ChangeTracker, sourceFile: SourceFile, startToken: Node, deletedAncestors: NodeSet | undefined) {
109+
const declarationList = tryCast(startToken.parent, isVariableDeclarationList);
110+
if (declarationList && declarationList.getChildren(sourceFile)[0] === startToken) {
111+
if (deletedAncestors) deletedAncestors.add(declarationList);
112+
changes.deleteNode(sourceFile, declarationList.parent.kind === SyntaxKind.VariableStatement ? declarationList.parent : declarationList);
113+
return true;
114+
}
115+
return false;
116+
}
117+
104118
function getToken(sourceFile: SourceFile, pos: number): Node {
105119
const token = findPrecedingToken(pos, sourceFile, /*startNode*/ undefined, /*includeJsDoc*/ true);
106120
// this handles var ["computed"] = 12;
Collapse file

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

Copy file name to clipboardExpand all lines: tests/baselines/reference/unusedDestructuring.errors.txt
+9-5Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@ tests/cases/compiler/unusedDestructuring.ts(3,7): error TS6198: All destructured
22
tests/cases/compiler/unusedDestructuring.ts(4,9): error TS6133: 'c' is declared but its value is never read.
33
tests/cases/compiler/unusedDestructuring.ts(6,7): error TS6133: 'e' is declared but its value is never read.
44
tests/cases/compiler/unusedDestructuring.ts(7,7): error TS6133: 'g' is declared but its value is never read.
5-
tests/cases/compiler/unusedDestructuring.ts(9,1): error TS6133: 'f' is declared but its value is never read.
6-
tests/cases/compiler/unusedDestructuring.ts(9,12): error TS6198: All destructured elements are unused.
7-
tests/cases/compiler/unusedDestructuring.ts(9,24): error TS6133: 'c' is declared but its value is never read.
8-
tests/cases/compiler/unusedDestructuring.ts(9,32): error TS6133: 'e' is declared but its value is never read.
5+
tests/cases/compiler/unusedDestructuring.ts(8,1): error TS6199: All variables are unused.
6+
tests/cases/compiler/unusedDestructuring.ts(10,1): error TS6133: 'f' is declared but its value is never read.
7+
tests/cases/compiler/unusedDestructuring.ts(10,12): error TS6198: All destructured elements are unused.
8+
tests/cases/compiler/unusedDestructuring.ts(10,24): error TS6133: 'c' is declared but its value is never read.
9+
tests/cases/compiler/unusedDestructuring.ts(10,32): error TS6133: 'e' is declared but its value is never read.
910

1011

11-
==== tests/cases/compiler/unusedDestructuring.ts (8 errors) ====
12+
==== tests/cases/compiler/unusedDestructuring.ts (9 errors) ====
1213
export {};
1314
declare const o: any;
1415
const { a, b } = o;
@@ -24,6 +25,9 @@ tests/cases/compiler/unusedDestructuring.ts(9,32): error TS6133: 'e' is declared
2425
const { f: g } = o;
2526
~~~~~~~~
2627
!!! error TS6133: 'g' is declared but its value is never read.
28+
const { h } = o, { i } = o;
29+
~~~~~~~~~~~~~~~~~~~~~~~~~~~
30+
!!! error TS6199: All variables are unused.
2731

2832
function f({ a, b }, { c, d }, { e }) {
2933
~~~~~~~~~~
Collapse file

‎tests/baselines/reference/unusedDestructuring.js‎

Copy file name to clipboardExpand all lines: tests/baselines/reference/unusedDestructuring.js
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const { c, d } = o;
66
d;
77
const { e } = o;
88
const { f: g } = o;
9+
const { h } = o, { i } = o;
910

1011
function f({ a, b }, { c, d }, { e }) {
1112
d;
@@ -20,6 +21,7 @@ var c = o.c, d = o.d;
2021
d;
2122
var e = o.e;
2223
var g = o.f;
24+
var h = o.h, i = o.i;
2325
function f(_a, _b, _c) {
2426
var a = _a.a, b = _a.b;
2527
var c = _b.c, d = _b.d;
Collapse file

‎tests/baselines/reference/unusedDestructuring.symbols‎

Copy file name to clipboardExpand all lines: tests/baselines/reference/unusedDestructuring.symbols
+13-7Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,21 @@ const { f: g } = o;
2424
>g : Symbol(g, Decl(unusedDestructuring.ts, 6, 7))
2525
>o : Symbol(o, Decl(unusedDestructuring.ts, 1, 13))
2626

27+
const { h } = o, { i } = o;
28+
>h : Symbol(h, Decl(unusedDestructuring.ts, 7, 7))
29+
>o : Symbol(o, Decl(unusedDestructuring.ts, 1, 13))
30+
>i : Symbol(i, Decl(unusedDestructuring.ts, 7, 18))
31+
>o : Symbol(o, Decl(unusedDestructuring.ts, 1, 13))
32+
2733
function f({ a, b }, { c, d }, { e }) {
28-
>f : Symbol(f, Decl(unusedDestructuring.ts, 6, 19))
29-
>a : Symbol(a, Decl(unusedDestructuring.ts, 8, 12))
30-
>b : Symbol(b, Decl(unusedDestructuring.ts, 8, 15))
31-
>c : Symbol(c, Decl(unusedDestructuring.ts, 8, 22))
32-
>d : Symbol(d, Decl(unusedDestructuring.ts, 8, 25))
33-
>e : Symbol(e, Decl(unusedDestructuring.ts, 8, 32))
34+
>f : Symbol(f, Decl(unusedDestructuring.ts, 7, 27))
35+
>a : Symbol(a, Decl(unusedDestructuring.ts, 9, 12))
36+
>b : Symbol(b, Decl(unusedDestructuring.ts, 9, 15))
37+
>c : Symbol(c, Decl(unusedDestructuring.ts, 9, 22))
38+
>d : Symbol(d, Decl(unusedDestructuring.ts, 9, 25))
39+
>e : Symbol(e, Decl(unusedDestructuring.ts, 9, 32))
3440

3541
d;
36-
>d : Symbol(d, Decl(unusedDestructuring.ts, 8, 25))
42+
>d : Symbol(d, Decl(unusedDestructuring.ts, 9, 25))
3743
}
3844

Collapse file

‎tests/baselines/reference/unusedDestructuring.types‎

Copy file name to clipboardExpand all lines: tests/baselines/reference/unusedDestructuring.types
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ const { f: g } = o;
2525
>g : any
2626
>o : any
2727

28+
const { h } = o, { i } = o;
29+
>h : any
30+
>o : any
31+
>i : any
32+
>o : any
33+
2834
function f({ a, b }, { c, d }, { e }) {
2935
>f : ({ a, b }: { a: any; b: any; }, { c, d }: { c: any; d: any; }, { e }: { e: any; }) => void
3036
>a : any
Collapse file

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

Copy file name to clipboard
+4-7Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
1-
tests/cases/compiler/unusedLocalsInMethod2.ts(3,13): error TS6133: 'x' is declared but its value is never read.
2-
tests/cases/compiler/unusedLocalsInMethod2.ts(3,16): error TS6133: 'y' is declared but its value is never read.
1+
tests/cases/compiler/unusedLocalsInMethod2.ts(3,9): error TS6199: All variables are unused.
32

43

5-
==== tests/cases/compiler/unusedLocalsInMethod2.ts (2 errors) ====
4+
==== tests/cases/compiler/unusedLocalsInMethod2.ts (1 errors) ====
65
class greeter {
76
public function1() {
87
var x, y = 10;
9-
~
10-
!!! error TS6133: 'x' is declared but its value is never read.
11-
~
12-
!!! error TS6133: 'y' is declared but its value is never read.
8+
~~~~~~~~~~~~~~
9+
!!! error TS6199: All variables are unused.
1310
y++;
1411
}
1512
}

0 commit comments

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