Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Commit

Permalink
Add fix for no-unused-variable, only for imports.
Browse files Browse the repository at this point in the history
  • Loading branch information
alexeagle committed Sep 23, 2016
1 parent 5cc19bd commit 91e591a
Show file tree
Hide file tree
Showing 3 changed files with 173 additions and 24 deletions.
117 changes: 99 additions & 18 deletions src/rules/noUnusedVariableRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,24 +174,87 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
}

public visitImportDeclaration(node: ts.ImportDeclaration) {
if (!Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword)) {
const importClause = node.importClause;
const importClause = node.importClause;

// named imports & namespace imports handled by other walker methods
// If the imports are exported, they may be used externally
if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword) ||
// importClause will be null for bare imports
if (importClause != null && importClause.name != null) {
const variableIdentifier = importClause.name;
this.validateReferencesForVariable(Rule.FAILURE_TYPE_IMPORT, variableIdentifier.text, variableIdentifier.getStart());
importClause == null) {
super.visitImportDeclaration(node);
return;
}

// Two passes: first collect what's unused, then produce failures. This allows the fix to lookahead.
let usesDefaultImport = false;
let usedNamedImports: boolean[] = [];
if (importClause.name != null) {
const variableIdentifier = importClause.name;
usesDefaultImport = this.isUsed(variableIdentifier.text, variableIdentifier.getStart());
}
if (importClause.namedBindings != null) {
if (importClause.namedBindings.kind === ts.SyntaxKind.NamedImports) {
let imports = node.importClause.namedBindings as ts.NamedImports;
usedNamedImports = imports.elements.map(e => this.isUsed(e.name.text, e.name.getStart()));
}
// Avoid deleting the whole statement if there's an import * inside
if (importClause.namedBindings.kind === ts.SyntaxKind.NamespaceImport) {
usesDefaultImport = true;
}
}

// Delete the entire import statement if named and default imports all unused
if (!usesDefaultImport && usedNamedImports.every(e => !e)) {
this.fail(Rule.FAILURE_TYPE_IMPORT, node.getText(), node.getStart(), this.deleteImportStatement(node));
super.visitImportDeclaration(node);
return;
}

// Delete the default import and trailing comma if unused
if (importClause.name != null && !usesDefaultImport) {
// There must be some named imports or we would have been in case 1
const end = importClause.namedBindings.getStart();
this.fail(Rule.FAILURE_TYPE_IMPORT, importClause.name.text, importClause.name.getStart(), [
this.deleteText(importClause.name.getStart(), end - importClause.name.getStart()),
]);
}
if (importClause.namedBindings != null &&
importClause.namedBindings.kind === ts.SyntaxKind.NamedImports) {
// Delete the entire named imports if all unused, including curly braces.
if (usedNamedImports.every(e => !e)) {
const start = importClause.name != null ? importClause.name.getEnd() : importClause.namedBindings.getStart();
this.fail(Rule.FAILURE_TYPE_IMPORT, importClause.namedBindings.getText(), importClause.namedBindings.getStart(), [
this.deleteText(start, importClause.namedBindings.getEnd() - start),
]);
} else {
let imports = node.importClause.namedBindings as ts.NamedImports;
let priorElementUsed = false;
for (let idx = 0; idx < imports.elements.length; idx++) {
const namedImport = imports.elements[idx];
if (usedNamedImports[idx]) {
priorElementUsed = true;
} else {
const isLast = idx === imports.elements.length - 1;
// Before the first used import, consume trailing commas.
// Afterward, consume leading commas instead.
let start = priorElementUsed ? imports.elements[idx - 1].getEnd() : namedImport.getStart();
let end = priorElementUsed || isLast ? namedImport.getEnd() : imports.elements[idx + 1].getStart();
this.fail(Rule.FAILURE_TYPE_IMPORT, namedImport.name.text, namedImport.name.getStart(), [
this.deleteText(start, end - start),
]);
}
}
}
}

// import x = 'y' & import * as x from 'y' handled by other walker methods
// because they only have one identifier that might be unused
super.visitImportDeclaration(node);
}

public visitImportEqualsDeclaration(node: ts.ImportEqualsDeclaration) {
if (!Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword)) {
const name = node.name;
this.validateReferencesForVariable(Rule.FAILURE_TYPE_IMPORT, name.text, name.getStart());
this.validateReferencesForVariable(Rule.FAILURE_TYPE_IMPORT, name.text, name.getStart(), this.deleteImportStatement(node));
}
super.visitImportEqualsDeclaration(node);
}
Expand Down Expand Up @@ -239,13 +302,6 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
this.skipParameterDeclaration = false;
}

public visitNamedImports(node: ts.NamedImports) {
for (const namedImport of node.elements) {
this.validateReferencesForVariable(Rule.FAILURE_TYPE_IMPORT, namedImport.name.text, namedImport.name.getStart());
}
super.visitNamedImports(node);
}

public visitNamespaceImport(node: ts.NamespaceImport) {
const importDeclaration = <ts.ImportDeclaration> node.parent.parent;
const moduleSpecifier = importDeclaration.moduleSpecifier.getText();
Expand All @@ -263,7 +319,8 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
this.isReactUsed = true;
}
} else {
this.validateReferencesForVariable(Rule.FAILURE_TYPE_IMPORT, node.name.text, node.name.getStart());
this.validateReferencesForVariable(Rule.FAILURE_TYPE_IMPORT, node.name.text, node.name.getStart(),
this.deleteImportStatement(importDeclaration));
}
super.visitNamespaceImport(node);
}
Expand Down Expand Up @@ -332,12 +389,36 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
this.skipVariableDeclaration = false;
}

private validateReferencesForVariable(type: string, name: string, position: number) {
/**
* Delete the statement along with leading trivia.
* BUT since imports are typically at the top of the file, the leading trivia is often a license.
* So when the leading trivia includes a block comment, delete the statement without leading trivia instead.
*/
private deleteImportStatement(node: ts.Statement): Lint.Replacement[] {
if (node.getFullText().substr(0, node.getLeadingTriviaWidth()).indexOf("/*") >= 0) {
return [this.deleteText(node.getStart(), node.getWidth())];
}
return [this.deleteText(node.getFullStart(), node.getFullWidth())];
}

private validateReferencesForVariable(type: string, name: string, position: number, replacements?: Lint.Replacement[]) {
if (!this.isUsed(name, position)) {
this.fail(type, name, position, replacements);
}
}

private isUsed(name: string, position: number): boolean {
const fileName = this.getSourceFile().fileName;
const highlights = this.languageService.getDocumentHighlights(fileName, position, [fileName]);
if ((highlights == null || highlights[0].highlightSpans.length <= 1) && !this.isIgnored(name)) {
this.addFailure(this.createFailure(position, name.length, Rule.FAILURE_STRING_FACTORY(type, name)));
return (highlights != null && highlights[0].highlightSpans.length > 1) || this.isIgnored(name);
}

private fail(type: string, name: string, position: number, replacements?: Lint.Replacement[]) {
let fix: Lint.Fix;
if (replacements && replacements.length) {
fix = new Lint.Fix("no-unused-variable", replacements);
}
this.addFailure(this.createFailure(position, name.length, Rule.FAILURE_STRING_FACTORY(type, name), fix));
}

private isIgnored(name: string) {
Expand Down
39 changes: 39 additions & 0 deletions test/rules/no-unused-variable/default/import.ts.fix
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* Some license that appears in the leading trivia of a statement that will be deleted.
* This text will not be deleted.
*/

import $ = require("jquery");
import _ = require("underscore");
import {a2 as aa2} from "modA";
aa2;
import {a3 as aa3} from "modA";
aa3;
import {a8} from "modA";
a8;
import {a13} from "modA";
a13;
import {a14, a16} from "modA";
a14;
a16;

export import a = require("a");

$(_.xyz());

/// <reference path="../externalFormatter.test.ts" />

module S {
var template = "";
}
import * as bar from "libB";
import baz from "libC";
import { namedExport } from "libD";
import d3 from "libD";
d3;

bar.someFunc();
baz();
namedExport();

import "jquery";
41 changes: 35 additions & 6 deletions test/rules/no-unused-variable/default/import.ts.lint
Original file line number Diff line number Diff line change
@@ -1,12 +1,36 @@
import $ = require("jquery");
import _ = require("underscore");
/**
* Some license that appears in the leading trivia of a statement that will be deleted.
* This text will not be deleted.
*/
import xyz = require("xyz");
~~~ [Unused import: 'xyz']
import {createReadStream, createWriteStream} from "fs";
~~~~~~~~~~~~~~~~ [Unused import: 'createReadStream']
export import a = require("a");
import $ = require("jquery");
import _ = require("underscore");
import {a1 as aa1, a2 as aa2} from "modA";
~~~ [Unused import: 'aa1']
aa2;
import {a3 as aa3, a4 as aa4} from "modA";
~~~ [Unused import: 'aa4']
aa3;
// This import statement is unused and will be deleted along with this comment.
import {a5, a6} from "modA";
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Unused import: 'import {a5, a6} from "modA";']
import {a7} from "modA";
~~~~~~~~~~~~~~~~~~~~~~~~ [Unused import: 'import {a7} from "modA";']
import {a8, a9, a10} from "modA";
~~ [Unused import: 'a9']
~~~ [Unused import: 'a10']
a8;
import {a11, a12, a13} from "modA";
~~~ [Unused import: 'a11']
~~~ [Unused import: 'a12']
a13;
import {a14, a15, a16} from "modA";
~~~ [Unused import: 'a15']
a14;
a16;

createWriteStream();
export import a = require("a");

$(_.xyz());

Expand All @@ -23,6 +47,11 @@ import * as bar from "libB";
import baz from "libC";
import defaultExport, { namedExport } from "libD";
~~~~~~~~~~~~~ [Unused import: 'defaultExport']
import d1, { d2 } from "libD";
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Unused import: 'import d1, { d2 } from "libD";']
import d3, { d4 } from "libD";
~~~~~~ [Unused import: '{ d4 }']
d3;

bar.someFunc();
baz();
Expand Down

0 comments on commit 91e591a

Please sign in to comment.