microsoft/TypeScript
1
0
Fork 0
mirror of https://github.com/microsoft/TypeScript.git synced 2026-02-04 21:53:42 -06:00
Code Issues Packages Projects Releases 212 Wiki Activity

fixUnusedIdentifier: If every VariableDeclaration is unused, remove the VariableStatement (#24231)

Browse Source
This commit is contained in:
Andy 2018-05-21 15:54:33 -07:00 committed by GitHub
parent d2f6f6a0a4
commit 802dc2bb9b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 129 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
src/compiler/checker.ts
View File

@ -22620,14 +22620,6 @@ namespace ts {
function errorUnusedLocal(declaration: Declaration, name: string, addDiagnostic: AddUnusedDiagnostic) {
const node = getNameOfDeclaration(declaration) || declaration;
if (isIdentifierThatStartsWithUnderScore(node)) {
const declaration = getRootDeclaration(node.parent);
if ((declaration.kind === SyntaxKind.VariableDeclaration && isForInOrOfStatement(declaration.parent.parent)) ||
declaration.kind === SyntaxKind.TypeParameter) {
return;
}
}
const message = isTypeDeclaration(declaration) ? Diagnostics._0_is_declared_but_never_used : Diagnostics._0_is_declared_but_its_value_is_never_read;
addDiagnostic(UnusedKind.Local, createDiagnosticForNodeSpan(getSourceFileOfNode(declaration), declaration, node, message, name));
}
@ -22712,6 +22704,7 @@ namespace ts {
// 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 = createMap<[ImportClause, ImportedDeclaration[]]>();
const unusedDestructures = createMap<[ObjectBindingPattern, BindingElement[]]>();
const unusedVariables = createMap<[VariableDeclarationList, VariableDeclaration[]]>();
nodeWithLocals.locals.forEach(local => {
// If it's purely a type parameter, ignore, will be checked in `checkUnusedTypeParameters`.
// If it's a type parameter merged with a parameter, check if the parameter-side is used.
@ -22731,6 +22724,11 @@ namespace ts {
addToGroup(unusedDestructures, declaration.parent, declaration, getNodeId);
}
}
else if (isVariableDeclaration(declaration)) {
if (!isIdentifierThatStartsWithUnderScore(declaration.name) || !isForInOrOfStatement(declaration.parent.parent)) {
addToGroup(unusedVariables, declaration.parent, declaration, getNodeId);
}
}
else {
const parameter = local.valueDeclaration && tryGetRootParameterDeclaration(local.valueDeclaration);
if (parameter) {
@ -22747,32 +22745,63 @@ namespace ts {
});
unusedImports.forEach(([importClause, unuseds]) => {
const importDecl = importClause.parent;
if (forEachImportedDeclaration(importClause, d => !contains(unuseds, d))) {
for (const unused of unuseds) errorUnusedLocal(unused, idText(unused.name), addDiagnostic);
}
else if (unuseds.length === 1) {
addDiagnostic(UnusedKind.Local, createDiagnosticForNode(importDecl, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(first(unuseds).name)));
const nDeclarations = (importClause.name ? 1 : 0) +
(importClause.namedBindings ?
(importClause.namedBindings.kind === SyntaxKind.NamespaceImport ? 1 : importClause.namedBindings.elements.length)
: 0);
if (nDeclarations === unuseds.length) {
addDiagnostic(UnusedKind.Local, unuseds.length === 1
? createDiagnosticForNode(importDecl, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(first(unuseds).name))
: createDiagnosticForNode(importDecl, Diagnostics.All_imports_in_import_declaration_are_unused));
}
else {
addDiagnostic(UnusedKind.Local, createDiagnosticForNode(importDecl, Diagnostics.All_imports_in_import_declaration_are_unused));
for (const unused of unuseds) errorUnusedLocal(unused, idText(unused.name), addDiagnostic);
}
});
unusedDestructures.forEach(([bindingPattern, bindingElements]) => {
const kind = tryGetRootParameterDeclaration(bindingPattern.parent) ? UnusedKind.Parameter : UnusedKind.Local;
if (!bindingPattern.elements.every(e => contains(bindingElements, e))) {
if (bindingPattern.elements.length === bindingElements.length) {
if (bindingElements.length === 1 && bindingPattern.parent.kind === SyntaxKind.VariableDeclaration && bindingPattern.parent.parent.kind === SyntaxKind.VariableDeclarationList) {
addToGroup(unusedVariables, bindingPattern.parent.parent, bindingPattern.parent, getNodeId);
}
else {
addDiagnostic(kind, bindingElements.length === 1
? createDiagnosticForNode(bindingPattern, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(cast(first(bindingElements).name, isIdentifier)))
: createDiagnosticForNode(bindingPattern, Diagnostics.All_destructured_elements_are_unused));
}
}
else {
for (const e of bindingElements) {
addDiagnostic(kind, createDiagnosticForNode(e, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(cast(e.name, isIdentifier))));
}
}
else if (bindingElements.length === 1) {
addDiagnostic(kind, createDiagnosticForNode(bindingPattern, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(cast(first(bindingElements).name, isIdentifier))));
});
unusedVariables.forEach(([declarationList, declarations]) => {
if (declarationList.declarations.length === declarations.length) {
addDiagnostic(UnusedKind.Local, declarations.length === 1
? createDiagnosticForNode(first(declarations).name, Diagnostics._0_is_declared_but_its_value_is_never_read, bindingNameText(first(declarations).name))
: createDiagnosticForNode(declarationList.parent.kind === SyntaxKind.VariableStatement ? declarationList.parent : declarationList, Diagnostics.All_variables_are_unused));
}
else {
addDiagnostic(kind, createDiagnosticForNode(bindingPattern, Diagnostics.All_destructured_elements_are_unused));
for (const decl of declarations) {
addDiagnostic(UnusedKind.Local, createDiagnosticForNode(decl, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(cast(decl.name, isIdentifier))));
}
}
});
}
function bindingNameText(name: BindingName): string {
switch (name.kind) {
case SyntaxKind.Identifier:
return idText(name);
case SyntaxKind.ArrayBindingPattern:
case SyntaxKind.ObjectBindingPattern:
return bindingNameText(cast(first(name.elements), isBindingElement).name);
default:
return Debug.assertNever(name);
}
}
type ImportedDeclaration = ImportClause | ImportSpecifier | NamespaceImport;
function isImportedDeclaration(node: Node): node is ImportedDeclaration {
return node.kind === SyntaxKind.ImportClause || node.kind === SyntaxKind.ImportSpecifier || node.kind === SyntaxKind.NamespaceImport;
@ -22781,12 +22810,6 @@ namespace ts {
return decl.kind === SyntaxKind.ImportClause ? decl : decl.kind === SyntaxKind.NamespaceImport ? decl.parent : decl.parent.parent;
}
function forEachImportedDeclaration<T>(importClause: ImportClause, cb: (im: ImportedDeclaration) => T | undefined): T | undefined {
const { name: defaultName, namedBindings } = importClause;
return (defaultName && cb(importClause)) ||
namedBindings && (namedBindings.kind === SyntaxKind.NamespaceImport ? cb(namedBindings) : forEach(namedBindings.elements, cb));
}
function checkBlock(node: Block) {
// Grammar checking for SyntaxKind.Block
if (node.kind === SyntaxKind.Block) {

9
src/compiler/diagnosticMessages.json
View File

@ -3556,6 +3556,11 @@
"code": 6198,
"reportsUnnecessary": true
},
"All variables are unused.": {
"category": "Error",
"code": 6199,
"reportsUnnecessary": true
},
"Projects to reference": {
"category": "Message",
@ -3994,6 +3999,10 @@
"category": "Message",
"code": 90009
},
"Remove variable statement": {
"category": "Message",
"code": 90010
},
"Import '{0}' from module \"{1}\"": {
"category": "Message",
"code": 90013

38
src/services/codefixes/fixUnusedIdentifier.ts
View File

@ -9,20 +9,27 @@ namespace ts.codefix {
Diagnostics.Property_0_is_declared_but_its_value_is_never_read.code,
Diagnostics.All_imports_in_import_declaration_are_unused.code,
Diagnostics.All_destructured_elements_are_unused.code,
Diagnostics.All_variables_are_unused.code,
];
registerCodeFix({
errorCodes,
getCodeActions(context) {
const { errorCode, sourceFile } = context;
const importDecl = tryGetFullImport(sourceFile, context.span.start);
const startToken = getTokenAtPosition(sourceFile, context.span.start, /*includeJsDocComment*/ false);
const importDecl = tryGetFullImport(startToken);
if (importDecl) {
const changes = textChanges.ChangeTracker.with(context, t => t.deleteNode(sourceFile, importDecl));
return [createCodeFixAction(fixName, changes, [Diagnostics.Remove_import_from_0, showModuleSpecifier(importDecl)], fixIdDelete, Diagnostics.Delete_all_unused_declarations)];
}
const delDestructure = textChanges.ChangeTracker.with(context, t => tryDeleteFullDestructure(t, sourceFile, context.span.start, /*deleted*/ undefined));
const delDestructure = textChanges.ChangeTracker.with(context, t => tryDeleteFullDestructure(t, sourceFile, startToken, /*deleted*/ undefined));
if (delDestructure.length) {
return [createCodeFixAction(fixName, delDestructure, Diagnostics.Remove_destructuring, fixIdDelete, Diagnostics.Delete_all_unused_declarations)];
}
const delVar = textChanges.ChangeTracker.with(context, t => tryDeleteFullVariableStatement(t, sourceFile, startToken, /*deleted*/ undefined));
if (delVar.length) {
return [createCodeFixAction(fixName, delDestructure, Diagnostics.Remove_variable_statement, fixIdDelete, Diagnostics.Delete_all_unused_declarations)];
}
const token = getToken(sourceFile, textSpanEnd(context.span));
const result: CodeFixAction[] = [];
@ -45,6 +52,7 @@ namespace ts.codefix {
const deleted = new NodeSet();
return codeFixAll(context, errorCodes, (changes, diag) => {
const { sourceFile } = context;
const startToken = getTokenAtPosition(sourceFile, diag.start, /*includeJsDocComment*/ false);
const token = findPrecedingToken(textSpanEnd(diag), diag.file!);
switch (context.fixId) {
case fixIdPrefix:
@ -56,14 +64,12 @@ namespace ts.codefix {
// Ignore if this range was already deleted.
if (deleted.some(d => rangeContainsPosition(d, diag.start!))) break;
const importDecl = tryGetFullImport(diag.file!, diag.start!);
const importDecl = tryGetFullImport(startToken);
if (importDecl) {
changes.deleteNode(sourceFile, importDecl);
}
else {
if (!tryDeleteFullDestructure(changes, sourceFile, diag.start!, deleted)) {
tryDeleteDeclaration(changes, sourceFile, token, deleted);
}
else if (!tryDeleteFullDestructure(changes, sourceFile, startToken, deleted) && !tryDeleteFullVariableStatement(changes, sourceFile, startToken, deleted)) {
tryDeleteDeclaration(changes, sourceFile, token, deleted);
}
break;
default:
@ -74,15 +80,13 @@ namespace ts.codefix {
});
// Sometimes the diagnostic span is an entire ImportDeclaration, so we should remove the whole thing.
function tryGetFullImport(sourceFile: SourceFile, pos: number): ImportDeclaration | undefined {
const startToken = getTokenAtPosition(sourceFile, pos, /*includeJsDocComment*/ false);
function tryGetFullImport(startToken: Node): ImportDeclaration | undefined {
return startToken.kind === SyntaxKind.ImportKeyword ? tryCast(startToken.parent, isImportDeclaration) : undefined;
}
function tryDeleteFullDestructure(changes: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number, deletedAncestors: NodeSet | undefined): boolean {
const startToken = getTokenAtPosition(sourceFile, pos, /*includeJsDocComment*/ false);
function tryDeleteFullDestructure(changes: textChanges.ChangeTracker, sourceFile: SourceFile, startToken: Node, deletedAncestors: NodeSet | undefined): boolean {
if (startToken.kind !== SyntaxKind.OpenBraceToken || !isObjectBindingPattern(startToken.parent)) return false;
const decl = startToken.parent.parent;
const decl = cast(startToken.parent, isObjectBindingPattern).parent;
switch (decl.kind) {
case SyntaxKind.VariableDeclaration:
tryDeleteVariableDeclaration(changes, sourceFile, decl, deletedAncestors);
@ -101,6 +105,16 @@ namespace ts.codefix {
return true;
}
function tryDeleteFullVariableStatement(changes: textChanges.ChangeTracker, sourceFile: SourceFile, startToken: Node, deletedAncestors: NodeSet | undefined) {
const declarationList = tryCast(startToken.parent, isVariableDeclarationList);
if (declarationList && declarationList.getChildren(sourceFile)[0] === startToken) {
if (deletedAncestors) deletedAncestors.add(declarationList);
changes.deleteNode(sourceFile, declarationList.parent.kind === SyntaxKind.VariableStatement ? declarationList.parent : declarationList);
return true;
}
return false;
}
function getToken(sourceFile: SourceFile, pos: number): Node {
const token = findPrecedingToken(pos, sourceFile, /*startNode*/ undefined, /*includeJsDoc*/ true);
// this handles var ["computed"] = 12;

14
tests/baselines/reference/unusedDestructuring.errors.txt
View File

@ -2,13 +2,14 @@ tests/cases/compiler/unusedDestructuring.ts(3,7): error TS6198: All destructured
tests/cases/compiler/unusedDestructuring.ts(4,9): error TS6133: 'c' is declared but its value is never read.
tests/cases/compiler/unusedDestructuring.ts(6,7): error TS6133: 'e' is declared but its value is never read.
tests/cases/compiler/unusedDestructuring.ts(7,7): error TS6133: 'g' is declared but its value is never read.
tests/cases/compiler/unusedDestructuring.ts(9,1): error TS6133: 'f' is declared but its value is never read.
tests/cases/compiler/unusedDestructuring.ts(9,12): error TS6198: All destructured elements are unused.
tests/cases/compiler/unusedDestructuring.ts(9,24): error TS6133: 'c' is declared but its value is never read.
tests/cases/compiler/unusedDestructuring.ts(9,32): error TS6133: 'e' is declared but its value is never read.
tests/cases/compiler/unusedDestructuring.ts(8,1): error TS6199: All variables are unused.
tests/cases/compiler/unusedDestructuring.ts(10,1): error TS6133: 'f' is declared but its value is never read.
tests/cases/compiler/unusedDestructuring.ts(10,12): error TS6198: All destructured elements are unused.
tests/cases/compiler/unusedDestructuring.ts(10,24): error TS6133: 'c' is declared but its value is never read.
tests/cases/compiler/unusedDestructuring.ts(10,32): error TS6133: 'e' is declared but its value is never read.
==== tests/cases/compiler/unusedDestructuring.ts (8 errors) ====
==== tests/cases/compiler/unusedDestructuring.ts (9 errors) ====
export {};
declare const o: any;
const { a, b } = o;
@ -24,6 +25,9 @@ tests/cases/compiler/unusedDestructuring.ts(9,32): error TS6133: 'e' is declared
const { f: g } = o;
~~~~~~~~
!!! error TS6133: 'g' is declared but its value is never read.
const { h } = o, { i } = o;
~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6199: All variables are unused.
function f({ a, b }, { c, d }, { e }) {
~~~~~~~~~~

2
tests/baselines/reference/unusedDestructuring.js
View File

@ -6,6 +6,7 @@ const { c, d } = o;
d;
const { e } = o;
const { f: g } = o;
const { h } = o, { i } = o;
function f({ a, b }, { c, d }, { e }) {
d;
@ -20,6 +21,7 @@ var c = o.c, d = o.d;
d;
var e = o.e;
var g = o.f;
var h = o.h, i = o.i;
function f(_a, _b, _c) {
var a = _a.a, b = _a.b;
var c = _b.c, d = _b.d;

20
tests/baselines/reference/unusedDestructuring.symbols
View File

@ -24,15 +24,21 @@ const { f: g } = o;
>g : Symbol(g, Decl(unusedDestructuring.ts, 6, 7))
>o : Symbol(o, Decl(unusedDestructuring.ts, 1, 13))
const { h } = o, { i } = o;
>h : Symbol(h, Decl(unusedDestructuring.ts, 7, 7))
>o : Symbol(o, Decl(unusedDestructuring.ts, 1, 13))
>i : Symbol(i, Decl(unusedDestructuring.ts, 7, 18))
>o : Symbol(o, Decl(unusedDestructuring.ts, 1, 13))
function f({ a, b }, { c, d }, { e }) {
>f : Symbol(f, Decl(unusedDestructuring.ts, 6, 19))
>a : Symbol(a, Decl(unusedDestructuring.ts, 8, 12))
>b : Symbol(b, Decl(unusedDestructuring.ts, 8, 15))
>c : Symbol(c, Decl(unusedDestructuring.ts, 8, 22))
>d : Symbol(d, Decl(unusedDestructuring.ts, 8, 25))
>e : Symbol(e, Decl(unusedDestructuring.ts, 8, 32))
>f : Symbol(f, Decl(unusedDestructuring.ts, 7, 27))
>a : Symbol(a, Decl(unusedDestructuring.ts, 9, 12))
>b : Symbol(b, Decl(unusedDestructuring.ts, 9, 15))
>c : Symbol(c, Decl(unusedDestructuring.ts, 9, 22))
>d : Symbol(d, Decl(unusedDestructuring.ts, 9, 25))
>e : Symbol(e, Decl(unusedDestructuring.ts, 9, 32))
d;
>d : Symbol(d, Decl(unusedDestructuring.ts, 8, 25))
>d : Symbol(d, Decl(unusedDestructuring.ts, 9, 25))
}

6
tests/baselines/reference/unusedDestructuring.types
View File

@ -25,6 +25,12 @@ const { f: g } = o;
>g : any
>o : any
const { h } = o, { i } = o;
>h : any
>o : any
>i : any
>o : any
function f({ a, b }, { c, d }, { e }) {
>f : ({ a, b }: { a: any; b: any; }, { c, d }: { c: any; d: any; }, { e }: { e: any; }) => void
>a : any

11
tests/baselines/reference/unusedLocalsInMethod2.errors.txt
View File

@ -1,15 +1,12 @@
tests/cases/compiler/unusedLocalsInMethod2.ts(3,13): error TS6133: 'x' is declared but its value is never read.
tests/cases/compiler/unusedLocalsInMethod2.ts(3,16): error TS6133: 'y' is declared but its value is never read.
tests/cases/compiler/unusedLocalsInMethod2.ts(3,9): error TS6199: All variables are unused.
==== tests/cases/compiler/unusedLocalsInMethod2.ts (2 errors) ====
==== tests/cases/compiler/unusedLocalsInMethod2.ts (1 errors) ====
class greeter {
public function1() {
var x, y = 10;
~
!!! error TS6133: 'x' is declared but its value is never read.
~
!!! error TS6133: 'y' is declared but its value is never read.
~~~~~~~~~~~~~~
!!! error TS6199: All variables are unused.
y++;
}
}

11
tests/baselines/reference/unusedLocalsInMethod3.errors.txt
View File

@ -1,15 +1,12 @@
tests/cases/compiler/unusedLocalsInMethod3.ts(3,13): error TS6133: 'x' is declared but its value is never read.
tests/cases/compiler/unusedLocalsInMethod3.ts(3,16): error TS6133: 'y' is declared but its value is never read.
tests/cases/compiler/unusedLocalsInMethod3.ts(3,9): error TS6199: All variables are unused.
==== tests/cases/compiler/unusedLocalsInMethod3.ts (2 errors) ====
==== tests/cases/compiler/unusedLocalsInMethod3.ts (1 errors) ====
class greeter {
public function1() {
var x, y;
~
!!! error TS6133: 'x' is declared but its value is never read.
~
!!! error TS6133: 'y' is declared but its value is never read.
~~~~~~~~~
!!! error TS6199: All variables are unused.
y = 1;
}
}

1
tests/cases/compiler/unusedDestructuring.ts
View File

@ -8,6 +8,7 @@ const { c, d } = o;
d;
const { e } = o;
const { f: g } = o;
const { h } = o, { i } = o;
function f({ a, b }, { c, d }, { e }) {
d;

9
tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts
View File

@ -7,6 +7,10 @@
//// const x = 0;
////}
////function g(a, b, c) { return a; }
////{
//// let a, b;
////}
////for (let i = 0, j = 0; ;) {}
verify.codeFixAll({
fixId: "unusedIdentifier_delete",
@ -14,5 +18,8 @@ verify.codeFixAll({
newFileContent:
`function f() {
}
function g(a) { return a; }`,
function g(a) { return a; }
{
}
for (; ;) {}`,
});