Skip to content

Commit

Permalink
no-unused-variable: don't crash at destructuring (palantir#3058)
Browse files Browse the repository at this point in the history
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: palantir#2876
Fixes: palantir#3001
  • Loading branch information
ajafff authored and HyphnKnight committed Apr 9, 2018
1 parent 979b0e5 commit 84a34a5
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 2 deletions.
7 changes: 5 additions & 2 deletions src/rules/noUnusedVariableRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ function walk(ctx: Lint.WalkContext<void>, 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<ts.Identifier, string>();
Expand All @@ -118,7 +119,7 @@ function walk(ctx: Lint.WalkContext<void>, 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;
}

Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
3 changes: 3 additions & 0 deletions test/rules/no-unused-variable/type-checked/some.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export class SomeClass {}

export const someVar = {prop: new SomeClass()};

0 comments on commit 84a34a5

Please sign in to comment.