From a479f436530f0afd52fc65c5a45645bd4a2b5b29 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Wed, 2 Aug 2017 10:28:17 +0200 Subject: [PATCH] no-unused-variable: don't crash at destructuring (#3058) Avoid typescript crash when trying to get type of destructured variable declaration. Also avoid walking the AST multiple times when `--declaration` is not enabled. That should result in better performance and fewer false negatives. [bugfix] `no-unused-variable` fixed crash when using destructuring Fixes: #2876 Fixes: #3001 --- src/rules/noUnusedVariableRule.ts | 7 +++++-- .../no-unused-variable/type-checked/destructuring.ts.lint | 6 ++++++ test/rules/no-unused-variable/type-checked/some.test.ts | 3 +++ 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 test/rules/no-unused-variable/type-checked/destructuring.ts.lint create mode 100644 test/rules/no-unused-variable/type-checked/some.test.ts diff --git a/src/rules/noUnusedVariableRule.ts b/src/rules/noUnusedVariableRule.ts index 25ca94025f7..df784aa2bce 100644 --- a/src/rules/noUnusedVariableRule.ts +++ b/src/rules/noUnusedVariableRule.ts @@ -104,6 +104,7 @@ function walk(ctx: Lint.WalkContext, program: ts.Program, { checkParameter const unusedCheckedProgram = getUnusedCheckedProgram(program, checkParameters); const diagnostics = ts.getPreEmitDiagnostics(unusedCheckedProgram, sourceFile); const checker = unusedCheckedProgram.getTypeChecker(); // Doesn't matter which program is used for this. + const declaration = program.getCompilerOptions().declaration; // If all specifiers in an import are unused, we elide the entire import. const importSpecifierFailures = new Map(); @@ -118,7 +119,7 @@ function walk(ctx: Lint.WalkContext, program: ts.Program, { checkParameter if (kind === UnusedKind.VARIABLE_OR_PARAMETER) { const importName = findImport(diag.start, sourceFile); if (importName !== undefined) { - if (isImportUsed(importName, sourceFile, checker)) { + if (declaration && isImportUsed(importName, sourceFile, checker)) { continue; } @@ -295,7 +296,9 @@ function isImportUsed(importSpecifier: ts.Identifier, sourceFile: ts.SourceFile, } function getImplicitType(node: ts.Node, checker: ts.TypeChecker): ts.Type | undefined { - if ((utils.isPropertyDeclaration(node) || utils.isVariableDeclaration(node)) && node.type === undefined) { + if ((utils.isPropertyDeclaration(node) || utils.isVariableDeclaration(node)) && + node.type === undefined && node.name.kind === ts.SyntaxKind.Identifier || + utils.isBindingElement(node) && node.name.kind === ts.SyntaxKind.Identifier) { return checker.getTypeAtLocation(node); } else if (utils.isSignatureDeclaration(node) && node.type === undefined) { const sig = checker.getSignatureFromDeclaration(node); diff --git a/test/rules/no-unused-variable/type-checked/destructuring.ts.lint b/test/rules/no-unused-variable/type-checked/destructuring.ts.lint new file mode 100644 index 00000000000..33d05251b36 --- /dev/null +++ b/test/rules/no-unused-variable/type-checked/destructuring.ts.lint @@ -0,0 +1,6 @@ +import { SomeClass, someVar } from "./some.test"; +const person = { name: "alice" }; +const { name } = person; + ~~~~ ['name' is declared but never used.] + +export const {prop} = someVar; diff --git a/test/rules/no-unused-variable/type-checked/some.test.ts b/test/rules/no-unused-variable/type-checked/some.test.ts new file mode 100644 index 00000000000..334bcaebd46 --- /dev/null +++ b/test/rules/no-unused-variable/type-checked/some.test.ts @@ -0,0 +1,3 @@ +export class SomeClass {} + +export const someVar = {prop: new SomeClass()};