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

Commit

Permalink
WIP: optimize by checking whole file first
Browse files Browse the repository at this point in the history
  • Loading branch information
alexeagle committed Oct 7, 2016
1 parent ecaa31f commit f836dc1
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 44 deletions.
41 changes: 30 additions & 11 deletions src/language/languageServiceHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,27 @@ import * as ts from "typescript";

import {createCompilerOptions} from "./utils";

export interface LanguageServiceEditableHost extends ts.LanguageServiceHost {
interface LanguageServiceEditableHost extends ts.LanguageServiceHost {
editFile(fileName: string, newContent: string): void;
}

export function hostFromProgram(program: ts.Program): LanguageServiceEditableHost {
export function wrapProgram(program: ts.Program): ts.LanguageService {
const files: {[name: string]: string} = {};
const fileVersions: {[name: string]: number} = {};
return {
const host: LanguageServiceEditableHost = {
getCompilationSettings: () => program.getCompilerOptions(),
getCurrentDirectory: () => program.getCurrentDirectory(),
getDefaultLibFileName: () => "lib.d.ts",
getScriptFileNames: () => program.getRootFileNames(),
getScriptSnapshot: (name: string) => ts.ScriptSnapshot.fromString(
files.hasOwnProperty(name) ? files[name] :
program.getSourceFile(name).getFullText()
),
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) {
Expand All @@ -46,6 +51,20 @@ export function hostFromProgram(program: ts.Program): LanguageServiceEditableHos
}
},
};
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) {
(ls as any).editFile(sf.fileName, newText);
const newProgram = ls.getProgram();
const newSf = newProgram.getSourceFile(sf.fileName);
const newDiags = ts.getPreEmitDiagnostics(newProgram, newSf);
// revert
(ls as any).editFile(sf.fileName, sf.getFullText());
return newDiags;

}

export function createLanguageServiceHost(fileName: string, source: string): ts.LanguageServiceHost {
Expand All @@ -60,7 +79,7 @@ export function createLanguageServiceHost(fileName: string, source: string): ts.
};
}

export function createLanguageService(fileName: string, source: string, program?: ts.Program) {
const languageServiceHost = program ? hostFromProgram(program) : createLanguageServiceHost(fileName, source);
export function createLanguageService(fileName: string, source: string) {
const languageServiceHost = createLanguageServiceHost(fileName, source);
return ts.createLanguageService(languageServiceHost);
}
2 changes: 1 addition & 1 deletion src/language/rule/typedRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@ export abstract class TypedRule extends AbstractRule {
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[];
}
4 changes: 2 additions & 2 deletions src/rules/noForInArrayRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: Lint.TslintLanguageService): Lint.RuleFailure[] {
const noForInArrayWalker = new NoForInArrayWalker(sourceFile, this.getOptions(), langSvc.getProgram());
return this.applyWithWalker(noForInArrayWalker);
}
}
Expand Down
63 changes: 37 additions & 26 deletions src/rules/noUnusedVariableRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,8 @@ export class Rule extends Lint.Rules.TypedRule {
return this.applyWithProgram(sourceFile, undefined);
}

public applyWithProgram(sourceFile: ts.SourceFile, program?: ts.Program): Lint.RuleFailure[] {
let languageServiceHost = program ? Lint.hostFromProgram(program) :
Lint.createLanguageServiceHost(sourceFile.fileName, sourceFile.getFullText());
return this.applyWithWalker(new NoUnusedVariablesWalker(sourceFile, this.getOptions(), languageServiceHost, program));
public applyWithProgram(sourceFile: ts.SourceFile, langSvc?: ts.LanguageService): Lint.RuleFailure[] {
return this.applyWithWalker(new NoUnusedVariablesWalker(sourceFile, this.getOptions(), langSvc));
}
}

Expand All @@ -96,16 +94,20 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
private ignorePattern: RegExp;
private isReactUsed: boolean;
private reactImport: ts.NamespaceImport;
private languageService: ts.LanguageService;
private dummyLanguageService: boolean;
private possibleFailures: Lint.RuleFailure[] = [];

constructor(sourceFile: ts.SourceFile, options: Lint.IOptions,
private languageServiceHost: ts.LanguageServiceHost, private program?: ts.Program) {
private languageService?: ts.LanguageService) {
super(sourceFile, options);
this.languageService = ts.createLanguageService(languageServiceHost);
this.skipVariableDeclaration = false;
this.skipParameterDeclaration = false;
this.hasSeenJsxElement = false;
this.isReactUsed = false;
if (!languageService) {
this.dummyLanguageService = true;
this.languageService = Lint.createLanguageService(sourceFile.fileName, sourceFile.getFullText());
}

const ignorePatternOption = this.getOptions().filter((option: any) => {
return typeof option === "object" && option["ignore-pattern"] != null;
Expand Down Expand Up @@ -144,9 +146,35 @@ 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()) && !this.dummyLanguageService) {
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) {
console.error(`Found one of the broken fixes in ${this.getSourceFile().fileName}`);
} else {
this.addFailure(f);
}
}
});
}

public visitBindingElement(node: ts.BindingElement) {
Expand Down Expand Up @@ -427,24 +455,7 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
if (replacements && replacements.length) {
fix = new Lint.Fix("no-unused-variable", replacements);
}
// If we have the program, we can verify that the fix doesn't introduce failures
if (this.program) {
const program = this.languageService.getProgram();
const sf = this.getSourceFile();
const existingDiags = ts.getPreEmitDiagnostics(program, sf);
if (existingDiags.length > 0) {
throw new Error("Fix existing diagnostics before running no-unused-variable");
}
(this.languageServiceHost as any).editFile(sf.fileName, fix.apply(sf.getFullText()));
const newProgram = this.languageService.getProgram();
const newSf = newProgram.getSourceFile(sf.fileName);
const newDiags = ts.getPreEmitDiagnostics(newProgram, newSf);
if (newDiags.length > 0) {
console.error("rejecting fix because it causes errors");
return;
}
}
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) {
Expand Down
4 changes: 2 additions & 2 deletions src/rules/restrictPlusOperandsRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: Lint.TslintLanguageService): Lint.RuleFailure[] {
return this.applyWithWalker(new RestrictPlusOperandsWalker(sourceFile, this.getOptions(), langSvc.getProgram()));
}
}

Expand Down
8 changes: 6 additions & 2 deletions src/tslintMulti.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
} from "./configuration";
import { EnableDisableRulesWalker } from "./enableDisableRules";
import { findFormatter } from "./formatterLoader";
import { wrapProgram } from "./language/languageServiceHost";
import { IFormatter } from "./language/formatter/formatter";
import { RuleFailure } from "./language/rule/rule";
import { TypedRule } from "./language/rule/typedRule";
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -119,7 +123,7 @@ 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);
}
Expand Down

0 comments on commit f836dc1

Please sign in to comment.