Check for newly introduced diagnostics before reporting failures (for unused import)

src/language/languageServiceHost.ts

@@ -19,6 +19,57 @@ import * as ts from "typescript";
 
 import {createCompilerOptions} from "./utils";
 
+interface LanguageServiceEditableHost extends ts.LanguageServiceHost {
+    editFile(fileName: string, newContent: string): void;
+}
+
+export function wrapProgram(program: ts.Program): ts.LanguageService {
+    const files: {[name: string]: string} = {};
+    const fileVersions: {[name: string]: number} = {};
+    const host: LanguageServiceEditableHost = {
+            getCompilationSettings: () => program.getCompilerOptions(),
+            getCurrentDirectory: () => program.getCurrentDirectory(),
+            getDefaultLibFileName: () => null,
+            getScriptFileNames: () => program.getSourceFiles().map(sf => sf.fileName),
+            getScriptSnapshot: (name: string) => {
+                if (files.hasOwnProperty(name)) {
+                    return ts.ScriptSnapshot.fromString(files[name]);
+                }
+                if (!program.getSourceFile(name)) {
+                    return null;
+                }
+                return ts.ScriptSnapshot.fromString(program.getSourceFile(name).getFullText());
+            },
+            getScriptVersion: (name: string) => fileVersions.hasOwnProperty(name) ? fileVersions[name] + "" : "1",
+            log: () => { /* */ },
+            editFile(fileName: string, newContent: string) {
+                files[fileName] = newContent;
+                if (fileVersions.hasOwnProperty(fileName)) {
+                    fileVersions[fileName]++;
+                } else {
+                    fileVersions[fileName] = 0;
+                }
+            },
+        };
+    const langSvc = ts.createLanguageService(host, ts.createDocumentRegistry());
+    (langSvc as any).editFile = host.editFile;
+    return langSvc;
+}
+
+export function checkEdit(ls: ts.LanguageService, sf: ts.SourceFile, newText: string) {
+    if (ls.hasOwnProperty("editFile")) {
+        const host = ls as any as LanguageServiceEditableHost;
+        host.editFile(sf.fileName, newText);
+        const newProgram = ls.getProgram();
+        const newSf = newProgram.getSourceFile(sf.fileName);
+        const newDiags = ts.getPreEmitDiagnostics(newProgram, newSf);
+        // revert
+        host.editFile(sf.fileName, sf.getFullText());
+        return newDiags;
+    }
+    return [];
+}
+
 export function createLanguageServiceHost(fileName: string, source: string): ts.LanguageServiceHost {
     return {
         getCompilationSettings: () => createCompilerOptions(),

src/language/rule/abstractRule.ts

@@ -43,7 +43,7 @@ export abstract class AbstractRule implements IRule {
         return this.options;
     }
 
-    public abstract apply(sourceFile: ts.SourceFile): RuleFailure[];
+    public abstract apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[];
 
     public applyWithWalker(walker: RuleWalker): RuleFailure[] {
         walker.walk(walker.getSourceFile());

src/language/rule/rule.ts

@@ -85,7 +85,7 @@ export interface IDisabledInterval {
 export interface IRule {
     getOptions(): IOptions;
     isEnabled(): boolean;
-    apply(sourceFile: ts.SourceFile): RuleFailure[];
+    apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[];
     applyWithWalker(walker: RuleWalker): RuleFailure[];
 }
 

src/language/rule/typedRule.ts

@@ -21,10 +21,10 @@ import {AbstractRule} from "./abstractRule";
 import {RuleFailure} from "./rule";
 
 export abstract class TypedRule extends AbstractRule {
-    public apply(sourceFile: ts.SourceFile): RuleFailure[] {
+    public apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[] {
         // if no program is given to the linter, throw an error
         throw new Error(`${this.getOptions().ruleName} requires type checking`);
     }
 
-    public abstract applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[];
+    public abstract applyWithProgram(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[];
 }

src/rules/noForInArrayRule.ts

@@ -48,8 +48,8 @@ export class Rule extends Lint.Rules.TypedRule {
 
     public static FAILURE_STRING = "for-in loops over arrays are forbidden. Use for-of or array.forEach instead.";
 
-    public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
-        const noForInArrayWalker = new NoForInArrayWalker(sourceFile, this.getOptions(), program);
+    public applyWithProgram(sourceFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] {
+        const noForInArrayWalker = new NoForInArrayWalker(sourceFile, this.getOptions(), langSvc.getProgram());
         return this.applyWithWalker(noForInArrayWalker);
     }
 }

src/rules/noMergeableNamespaceRule.ts

@@ -35,8 +35,7 @@ export class Rule extends Lint.Rules.AbstractRule {
         return `Mergeable namespace ${identifier} found. Merge its contents with the namespace on line ${locationToMerge.line}.`;
     }
 
-    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        const languageService = Lint.createLanguageService(sourceFile.fileName, sourceFile.getFullText());
+    public apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): Lint.RuleFailure[] {
         const noMergeableNamespaceWalker = new NoMergeableNamespaceWalker(sourceFile, this.getOptions(), languageService);
         return this.applyWithWalker(noMergeableNamespaceWalker);
     }

src/rules/noUnusedVariableRule.ts

@@ -75,8 +75,7 @@ export class Rule extends Lint.Rules.AbstractRule {
 
     public static FAILURE_STRING_FACTORY = (type: string, name: string) => `Unused ${type}: '${name}'`;
 
-    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        const languageService = Lint.createLanguageService(sourceFile.fileName, sourceFile.getFullText());
+    public apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): Lint.RuleFailure[] {
         return this.applyWithWalker(new NoUnusedVariablesWalker(sourceFile, this.getOptions(), languageService));
     }
 }
@@ -90,8 +89,10 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
     private ignorePattern: RegExp;
     private isReactUsed: boolean;
     private reactImport: ts.NamespaceImport;
+    private possibleFailures: Lint.RuleFailure[] = [];
 
-    constructor(sourceFile: ts.SourceFile, options: Lint.IOptions, private languageService: ts.LanguageService) {
+    constructor(sourceFile: ts.SourceFile, options: Lint.IOptions,
+                private languageService: ts.LanguageService) {
         super(sourceFile, options);
         this.skipVariableDeclaration = false;
         this.skipParameterDeclaration = false;
@@ -135,9 +136,33 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
             if (!this.isIgnored(nameText)) {
                 const start = this.reactImport.name.getStart();
                 const msg = Rule.FAILURE_STRING_FACTORY(Rule.FAILURE_TYPE_IMPORT, nameText);
-                this.addFailure(this.createFailure(start, nameText.length, msg));
+                this.possibleFailures.push(this.createFailure(start, nameText.length, msg));
             }
         }
+
+        let someFixBrokeIt = false;
+        // Performance optimization: type-check the whole file before verifying individual fixes
+        if (this.possibleFailures.some(f => f.hasFix())) {
+            let newText = Lint.Fix.applyAll(this.getSourceFile().getFullText(),
+                this.possibleFailures.map(f => f.getFix()).filter(f => !!f));
+
+            // If we have the program, we can verify that the fix doesn't introduce failures
+            if (Lint.checkEdit(this.languageService, this.getSourceFile(), newText).length > 0) {
+                console.error(`Fixes caused errors in ${this.getSourceFile().fileName}`);
+                someFixBrokeIt = true;
+            }
+        }
+
+        this.possibleFailures.forEach(f => {
+            if (!someFixBrokeIt || !f.hasFix()) {
+                this.addFailure(f);
+            } else {
+                let newText = f.getFix().apply(this.getSourceFile().getFullText());
+                if (Lint.checkEdit(this.languageService, this.getSourceFile(), newText).length === 0) {
+                    this.addFailure(f);
+                }
+            }
+        });
     }
 
     public visitBindingElement(node: ts.BindingElement) {
@@ -418,7 +443,7 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
         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));
+        this.possibleFailures.push(this.createFailure(position, name.length, Rule.FAILURE_STRING_FACTORY(type, name), fix));
     }
 
     private isIgnored(name: string) {

src/rules/noUseBeforeDeclareRule.ts

@@ -37,8 +37,7 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static FAILURE_STRING_PREFIX = "variable '";
     public static FAILURE_STRING_POSTFIX = "' used before declaration";
 
-    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        const languageService = Lint.createLanguageService(sourceFile.fileName, sourceFile.getFullText());
+    public apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): Lint.RuleFailure[] {
         return this.applyWithWalker(new NoUseBeforeDeclareWalker(sourceFile, this.getOptions(), languageService));
     }
 }

src/rules/preferForOfRule.ts

@@ -33,8 +33,7 @@ export class Rule extends Lint.Rules.AbstractRule {
 
     public static FAILURE_STRING = "Expected a 'for-of' loop instead of a 'for' loop with this simple iteration";
 
-    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        const languageService = Lint.createLanguageService(sourceFile.fileName, sourceFile.getFullText());
+    public apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): Lint.RuleFailure[] {
         return this.applyWithWalker(new PreferForOfWalker(sourceFile, this.getOptions(), languageService));
     }
 }
@@ -67,7 +66,7 @@ class PreferForOfWalker extends Lint.RuleWalker {
                     // so remove those from the count to get the count inside the loop block
                     const incrementorCount = highlights[0].highlightSpans.length - 3;
 
-                    // Find `array[i]`-like usages by building up a regex 
+                    // Find `array[i]`-like usages by building up a regex
                     const arrayTokenForRegex = arrayToken.getText().replace(".", "\\.");
                     const incrementorForRegex = incrementorVariable.getText().replace(".", "\\.");
                     const regex = new RegExp(`${arrayTokenForRegex}\\[\\s*${incrementorForRegex}\\s*\\]`, "g");

src/rules/restrictPlusOperandsRule.ts

@@ -35,8 +35,8 @@ export class Rule extends Lint.Rules.TypedRule {
     public static MISMATCHED_TYPES_FAILURE = "Types of values used in '+' operation must match";
     public static UNSUPPORTED_TYPE_FAILURE_FACTORY = (type: string) => `cannot add type ${type}`;
 
-    public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
-        return this.applyWithWalker(new RestrictPlusOperandsWalker(sourceFile, this.getOptions(), program));
+    public applyWithProgram(sourceFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] {
+        return this.applyWithWalker(new RestrictPlusOperandsWalker(sourceFile, this.getOptions(), langSvc.getProgram()));
     }
 }
 

src/test.ts

@@ -23,7 +23,6 @@ import * as path from "path";
 import * as ts from "typescript";
 
 import {Fix} from "./language/rule/rule";
-import {createCompilerOptions} from "./language/utils";
 import {LintError} from "./test/lintError";
 import * as parse from "./test/parse";
 import * as Linter from "./tslint";
@@ -48,6 +47,15 @@ export interface TestResult {
 export function runTest(testDirectory: string, rulesDirectory?: string | string[]): TestResult {
     const filesToLint = glob.sync(path.join(testDirectory, `**/*${MARKUP_FILE_EXTENSION}`));
     const tslintConfig = Linter.findConfiguration(path.join(testDirectory, "tslint.json"), null);
+    const tsConfig = path.join(testDirectory, "tsconfig.json");
+    let compilerOptions: ts.CompilerOptions = {};
+    if (fs.existsSync(tsConfig)) {
+        const {config, error} = ts.readConfigFile(tsConfig, ts.sys.readFile);
+        if (error) {
+            throw new Error(JSON.stringify(error));
+        }
+        compilerOptions = ts.parseJsonConfigFileContent(config, {readDirectory: ts.sys.readDirectory}, testDirectory).options;
+    }
     const results: TestResult = { directory: testDirectory, results: {} };
 
     for (const fileToLint of filesToLint) {
@@ -59,7 +67,6 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[
 
         let program: ts.Program;
         if (tslintConfig.linterOptions && tslintConfig.linterOptions.typeCheck) {
-            const compilerOptions = createCompilerOptions();
             const compilerHost: ts.CompilerHost = {
                 fileExists: () => true,
                 getCanonicalFileName: (filename: string) => filename,
@@ -74,6 +81,9 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[
                         return ts.createSourceFile(filenameToGet, fileText, compilerOptions.target);
                     } else if (filenameToGet === fileCompileName) {
                         return ts.createSourceFile(fileBasename, fileTextWithoutMarkup, compilerOptions.target, true);
+                    } else if (fs.existsSync(path.resolve(path.dirname(fileToLint), filenameToGet))) {
+                        const text = fs.readFileSync(path.resolve(path.dirname(fileToLint), filenameToGet), {encoding: "utf-8"});
+                        return ts.createSourceFile(filenameToGet, text, compilerOptions.target, true);
                     }
                 },
                 readFile: () => null,

src/tslintMulti.ts

@@ -29,6 +29,7 @@ import {
 import { EnableDisableRulesWalker } from "./enableDisableRules";
 import { findFormatter } from "./formatterLoader";
 import { IFormatter } from "./language/formatter/formatter";
+import { createLanguageService, wrapProgram } from "./language/languageServiceHost";
 import { RuleFailure } from "./language/rule/rule";
 import { TypedRule } from "./language/rule/typedRule";
 import { getSourceFile } from "./language/utils";
@@ -48,6 +49,7 @@ class MultiLinter {
     public static loadConfigurationFromPath = loadConfigurationFromPath;
 
     private failures: RuleFailure[] = [];
+    private languageService: ts.LanguageService;
 
     /**
      * Creates a TypeScript program object from a tsconfig.json file path and optional project directory.
@@ -84,7 +86,9 @@ class MultiLinter {
     }
 
     constructor(private options: IMultiLinterOptions, private program?: ts.Program) {
-        // Empty
+        if (program) {
+            this.languageService = wrapProgram(program);
+        }
     }
 
     public lint(fileName: string, source?: string, configuration: any = DEFAULT_CONFIG): void {
@@ -97,6 +101,7 @@ class MultiLinter {
             }
         } else {
             sourceFile = getSourceFile(fileName, source);
+            this.languageService = createLanguageService(fileName, source);
         }
 
         if (sourceFile === undefined) {
@@ -119,9 +124,9 @@ class MultiLinter {
         for (let rule of enabledRules) {
             let ruleFailures: RuleFailure[] = [];
             if (this.program && rule instanceof TypedRule) {
-                ruleFailures = rule.applyWithProgram(sourceFile, this.program);
+                ruleFailures = rule.applyWithProgram(sourceFile, this.languageService);
             } else {
-                ruleFailures = rule.apply(sourceFile);
+                ruleFailures = rule.apply(sourceFile, this.languageService);
             }
             for (let ruleFailure of ruleFailures) {
                 if (!this.containsRule(this.failures, ruleFailure)) {

test/rules/no-unused-variable/type-checked/a.ts

@@ -0,0 +1,2 @@
+export class A {}
+export var a: A;

test/rules/no-unused-variable/type-checked/test.ts.lint

@@ -0,0 +1,5 @@
+import {a, A} from './a';
+
+export class B {
+  static thing = a;
+}

test/rules/no-unused-variable/type-checked/tsconfig.json

@@ -0,0 +1,5 @@
+{
+    "compilerOptions": {
+        "declaration": true
+    }
+}

test/rules/no-unused-variable/type-checked/tslint.json

@@ -0,0 +1,8 @@
+{
+  "linterOptions": {
+    "typeCheck": true
+  },
+  "rules": {
+    "no-unused-variable": true
+  }
+}