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

Add fix for no-unused-variable, only for imports #1568

Merged
merged 1 commit into from
Oct 7, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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