diff --git a/src/language/languageServiceHost.ts b/src/language/languageServiceHost.ts index 180e110c054..fd00e8fd5c2 100644 --- a/src/language/languageServiceHost.ts +++ b/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(), diff --git a/src/language/rule/abstractRule.ts b/src/language/rule/abstractRule.ts index 1859227dd78..94a0ed38125 100644 --- a/src/language/rule/abstractRule.ts +++ b/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()); diff --git a/src/language/rule/rule.ts b/src/language/rule/rule.ts index 3cdd626cc27..c44cb3f96e1 100644 --- a/src/language/rule/rule.ts +++ b/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[]; } diff --git a/src/language/rule/typedRule.ts b/src/language/rule/typedRule.ts index 661eda826c0..2e91c092bca 100644 --- a/src/language/rule/typedRule.ts +++ b/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[]; } diff --git a/src/linter.ts b/src/linter.ts index e522288f358..6655415bdd7 100644 --- a/src/linter.ts +++ b/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) { diff --git a/src/rules/completedDocsRule.ts b/src/rules/completedDocsRule.ts index a43ad746ac2..13e6c905865 100644 --- a/src/rules/completedDocsRule.ts +++ b/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); diff --git a/src/rules/noForInArrayRule.ts b/src/rules/noForInArrayRule.ts index d8e7f949e1c..2c335a58d9f 100644 --- a/src/rules/noForInArrayRule.ts +++ b/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); } } diff --git a/src/rules/noMergeableNamespaceRule.ts b/src/rules/noMergeableNamespaceRule.ts index 0829b8157c2..680dc278f31 100644 --- a/src/rules/noMergeableNamespaceRule.ts +++ b/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); } diff --git a/src/rules/noUnusedVariableRule.ts b/src/rules/noUnusedVariableRule.ts index 70c1703d0f3..f943c7c9cac 100644 --- a/src/rules/noUnusedVariableRule.ts +++ b/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) { diff --git a/src/rules/noUseBeforeDeclareRule.ts b/src/rules/noUseBeforeDeclareRule.ts index a173360422c..7eaa15de404 100644 --- a/src/rules/noUseBeforeDeclareRule.ts +++ b/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)); } } diff --git a/src/rules/preferForOfRule.ts b/src/rules/preferForOfRule.ts index 7142a298ded..21649389c75 100644 --- a/src/rules/preferForOfRule.ts +++ b/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); diff --git a/src/rules/restrictPlusOperandsRule.ts b/src/rules/restrictPlusOperandsRule.ts index 05becd841fa..ec16c412894 100644 --- a/src/rules/restrictPlusOperandsRule.ts +++ b/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())); } } diff --git a/src/test.ts b/src/test.ts index 90c8e94cc75..84fcc53ada5 100644 --- a/src/test.ts +++ b/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, diff --git a/test/rules/no-unused-variable/type-checked/a.ts b/test/rules/no-unused-variable/type-checked/a.ts new file mode 100644 index 00000000000..abfc7078006 --- /dev/null +++ b/test/rules/no-unused-variable/type-checked/a.ts @@ -0,0 +1,2 @@ +export class A {} +export var a: A; diff --git a/test/rules/no-unused-variable/type-checked/test.ts.lint b/test/rules/no-unused-variable/type-checked/test.ts.lint new file mode 100644 index 00000000000..de64a5953ac --- /dev/null +++ b/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; +} diff --git a/test/rules/no-unused-variable/type-checked/tsconfig.json b/test/rules/no-unused-variable/type-checked/tsconfig.json new file mode 100644 index 00000000000..db953a729b9 --- /dev/null +++ b/test/rules/no-unused-variable/type-checked/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "declaration": true + } +} \ No newline at end of file diff --git a/test/rules/no-unused-variable/type-checked/tslint.json b/test/rules/no-unused-variable/type-checked/tslint.json new file mode 100644 index 00000000000..e81cc662cf1 --- /dev/null +++ b/test/rules/no-unused-variable/type-checked/tslint.json @@ -0,0 +1,8 @@ +{ + "linterOptions": { + "typeCheck": true + }, + "rules": { + "no-unused-variable": true + } +}