diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 3e3c3964b9455..251cc88f6bc18 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -22617,14 +22617,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)); } @@ -22709,6 +22701,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. @@ -22728,6 +22721,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) { @@ -22744,32 +22742,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; @@ -22778,12 +22807,6 @@ namespace ts { return decl.kind === SyntaxKind.ImportClause ? decl : decl.kind === SyntaxKind.NamespaceImport ? decl.parent : decl.parent.parent; } - function forEachImportedDeclaration(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) { diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 6196191328016..2926c30f2a01a 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -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 diff --git a/src/services/codefixes/fixUnusedIdentifier.ts b/src/services/codefixes/fixUnusedIdentifier.ts index e15407b625bcd..8e38c675d74c7 100644 --- a/src/services/codefixes/fixUnusedIdentifier.ts +++ b/src/services/codefixes/fixUnusedIdentifier.ts @@ -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; diff --git a/tests/baselines/reference/unusedDestructuring.errors.txt b/tests/baselines/reference/unusedDestructuring.errors.txt index c148eea620abe..67b97697323a5 100644 --- a/tests/baselines/reference/unusedDestructuring.errors.txt +++ b/tests/baselines/reference/unusedDestructuring.errors.txt @@ -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 }) { ~~~~~~~~~~ diff --git a/tests/baselines/reference/unusedDestructuring.js b/tests/baselines/reference/unusedDestructuring.js index afac434cfafd4..d663c31d3c614 100644 --- a/tests/baselines/reference/unusedDestructuring.js +++ b/tests/baselines/reference/unusedDestructuring.js @@ -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; diff --git a/tests/baselines/reference/unusedDestructuring.symbols b/tests/baselines/reference/unusedDestructuring.symbols index c7767480eb8fe..3af5dc13317d4 100644 --- a/tests/baselines/reference/unusedDestructuring.symbols +++ b/tests/baselines/reference/unusedDestructuring.symbols @@ -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)) } diff --git a/tests/baselines/reference/unusedDestructuring.types b/tests/baselines/reference/unusedDestructuring.types index bbc40cd359b80..da5353dd0bcef 100644 --- a/tests/baselines/reference/unusedDestructuring.types +++ b/tests/baselines/reference/unusedDestructuring.types @@ -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 diff --git a/tests/baselines/reference/unusedLocalsInMethod2.errors.txt b/tests/baselines/reference/unusedLocalsInMethod2.errors.txt index 2aec1afb13a63..8d42611d57c08 100644 --- a/tests/baselines/reference/unusedLocalsInMethod2.errors.txt +++ b/tests/baselines/reference/unusedLocalsInMethod2.errors.txt @@ -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++; } } \ No newline at end of file diff --git a/tests/baselines/reference/unusedLocalsInMethod3.errors.txt b/tests/baselines/reference/unusedLocalsInMethod3.errors.txt index 6e34814457b81..37bd20bca070f 100644 --- a/tests/baselines/reference/unusedLocalsInMethod3.errors.txt +++ b/tests/baselines/reference/unusedLocalsInMethod3.errors.txt @@ -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; } } \ No newline at end of file diff --git a/tests/cases/compiler/unusedDestructuring.ts b/tests/cases/compiler/unusedDestructuring.ts index bfec022ab52ee..1d85f59c7d7ed 100644 --- a/tests/cases/compiler/unusedDestructuring.ts +++ b/tests/cases/compiler/unusedDestructuring.ts @@ -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; diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts index 67f75a894da09..56558c3765dfe 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts @@ -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 (; ;) {}`, });