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

src/rules/noUnusedVariableRule.ts

@@ -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);
     }
@@ -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();
@@ -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);
     }
@@ -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) {

test/rules/no-unused-variable/default/import.ts.fix

@@ -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";

test/rules/no-unused-variable/default/import.ts.lint

@@ -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());
 
@@ -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();