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

@@ -42,7 +42,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

@@ -95,7 +95,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

@@ -26,10 +26,10 @@ export abstract class TypedRule extends AbstractRule {
         return "applyWithProgram" in rule;
     }
 
-    public apply(_sourceFile: ts.SourceFile): RuleFailure[] {
+    public apply(): 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/linter.ts

@@ -31,6 +31,7 @@ import { EnableDisableRulesWalker } from "./enableDisableRules";
 import { findFormatter } from "./formatterLoader";
 import { ILinterOptions, LintResult } from "./index";
 import { IFormatter } from "./language/formatter/formatter";
+import { createLanguageService, wrapProgram } from "./language/languageServiceHost";
 import { Fix, IRule, RuleFailure } from "./language/rule/rule";
 import { TypedRule } from "./language/rule/typedRule";
 import * as utils from "./language/utils";
@@ -50,6 +51,7 @@ class Linter {
 
     private failures: RuleFailure[] = [];
     private fixes: RuleFailure[] = [];
+    private languageService: ts.LanguageService;
 
     /**
      * Creates a TypeScript program object from a tsconfig.json file path and optional project directory.
@@ -93,6 +95,10 @@ class Linter {
             throw new Error("ILinterOptions does not contain the property `configuration` as of version 4. " +
                 "Did you mean to pass the `IConfigurationFile` object to lint() ? ");
         }
+
+        if (program) {
+            this.languageService = wrapProgram(program);
+        }
     }
 
     public lint(fileName: string, source?: string, configuration: IConfigurationFile = DEFAULT_CONFIG): void {
@@ -158,9 +164,9 @@ class Linter {
     private applyRule(rule: IRule, sourceFile: ts.SourceFile) {
         let ruleFailures: RuleFailure[] = [];
         if (this.program && TypedRule.isTypedRule(rule)) {
-            ruleFailures = rule.applyWithProgram(sourceFile, this.program);
+            ruleFailures = rule.applyWithProgram(sourceFile, this.languageService);
         } else {
-            ruleFailures = rule.apply(sourceFile);
+            ruleFailures = rule.apply(sourceFile, this.languageService);
         }
         let fileFailures: RuleFailure[] = [];
         for (let ruleFailure of ruleFailures) {
@@ -201,6 +207,7 @@ class Linter {
             }
         } else {
             sourceFile = utils.getSourceFile(fileName, source);
+            this.languageService = createLanguageService(fileName, source);
         }
 
         if (sourceFile === undefined) {

src/rules/completedDocsRule.ts

@@ -55,9 +55,9 @@ export class Rule extends Lint.Rules.TypedRule {
         Rule.ARGUMENT_PROPERTIES,
     ];
 
-    public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
+    public applyWithProgram(sourceFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] {
         const options = this.getOptions();
-        const completedDocsWalker = new CompletedDocsWalker(sourceFile, options, program);
+        const completedDocsWalker = new CompletedDocsWalker(sourceFile, options, langSvc.getProgram());
 
         const nodesToCheck = this.getNodesToCheck(options.ruleArguments);
         completedDocsWalker.setNodesToCheck(nodesToCheck);

src/rules/noForInArrayRule.ts

@@ -49,8 +49,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

@@ -36,8 +36,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

@@ -78,8 +78,7 @@ export class Rule extends Lint.Rules.AbstractRule {
         return `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));
     }
 }
@@ -93,8 +92,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;
@@ -138,9 +139,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) {
@@ -421,7 +446,7 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
         if (replacements && replacements.length) {
             fix = new Lint.Fix(Rule.metadata.ruleName, 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

@@ -38,8 +38,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

@@ -74,6 +74,7 @@ class PreferForOfWalker extends Lint.RuleWalker {
         if (indexVariableName != null) {
             const incrementorState = this.incrementorMap[indexVariableName];
             if (incrementorState.onlyArrayAccess) {
+                    // Find `array[i]`-like usages by building up a regex 
                 const length = incrementorState.endIncrementPos - node.getStart() + 1;
                 const failure = this.createFailure(node.getStart(), length, Rule.FAILURE_STRING);
                 this.addFailure(failure);

src/rules/restrictPlusOperandsRule.ts

@@ -38,8 +38,8 @@ export class Rule extends Lint.Rules.TypedRule {
         return `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 * as Linter from "./linter";
 import {LintError} from "./test/lintError";
 import * as parse from "./test/parse";
@@ -48,6 +47,21 @@ 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).results;
+    const tsConfig = path.join(testDirectory, "tsconfig.json");
+    let compilerOptions: ts.CompilerOptions = { allowJs: true };
+    if (fs.existsSync(tsConfig)) {
+        const {config, error} = ts.readConfigFile(tsConfig, ts.sys.readFile);
+        if (error) {
+            throw new Error(JSON.stringify(error));
+        }
+
+        const parseConfigHost = {
+            fileExists: fs.existsSync,
+            readDirectory: ts.sys.readDirectory,
+            useCaseSensitiveFileNames: true,
+        };
+        compilerOptions = ts.parseJsonConfigFileContent(config, parseConfigHost, testDirectory).options;
+    }
     const results: TestResult = { directory: testDirectory, results: {} };
 
     for (const fileToLint of filesToLint) {
@@ -59,7 +73,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,
@@ -73,6 +86,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,

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
+  }
+}