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

[Breaking change] Remove all use of the language service. Use only the Program. #2235

Merged
merged 7 commits into from Mar 14, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
/lib/
/test/executable/tslint.json
node_modules/
!test/rules/**/node_modules
tscommand*.txt
npm-debug.log
# created by grunt-ts for faster compiling
Expand Down
19 changes: 6 additions & 13 deletions docs/develop/custom-rules/performance.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
## Performance tips

### Don't call the LanguageService repeatedly
The LanguageService is designed to serve editors. By design it does as little work to serve requests as possible.
For most requests no cache is used.

Let's say you need all usages of a variable. The LanguageService needs to check the whole AST subtree in which the variable is in scope.
Doing that once is barely noticable. But doing it over and over again, will result in pretty bad performance (looking at you `no-unused-variable`).

### Use the TypeChecker only when needed
The TypeChecker is a really mighty tool, but that comes with a cost. To create a TypeChecker the Program first has to locate, read, parse and bind all SourceFiles referenced.
To avoid that cost, try to avoid the TypeChecker where possible.
Expand All @@ -21,7 +14,7 @@ Some rules work directly on the content of the source file.
`max-file-line-count` and `linebreak-style` don't need to walk the AST at all.

Other rules define exceptions: `no-consecutive-blank-lines` ignores template strings.
To optimize for the best case, this rule can first look for failures in the source.
To optimize for the best case, this rule can first look for failures in the source.
If and only if there are any failures, walk the AST to find the location of all template strings to filter the failures.

### Implement your own walking algorithm
Expand All @@ -46,7 +39,7 @@ class MyWalker extends Lint.AbstractWalker<MyOptionsType> {

### Don't walk the whole AST if possible
__The Spec is your friend:__
The language spec defines where each statement can occur. If you are interested in `import` statements for example, you only need to search
The language spec defines where each statement can occur. If you are interested in `import` statements for example, you only need to search
in `sourceFile.statements` and nested `NamespaceDeclaration`s.

__Don't visit AST branches you're not interested in:__
Expand Down Expand Up @@ -76,12 +69,12 @@ Instead of stuffing the whole logic in a single closure, consider splitting it u
Each function should handle similar kinds of nodes. Don't worry too much about the function call, since V8 eventually inlines the function
if possible.

The AST nodes have different properties, therefore they have a different hidden class in V8. A function can only be optimized for a certain
The AST nodes have different properties, therefore they have a different hidden class in V8. A function can only be optimized for a certain
amount of different hidden classes. Above that threshold the function will be deoptimized and is never optimized again.

### Pass the optional `sourceFile` parameter
There are serveral methods that have an optional parameter `sourceFile`. Don't omit this parameter if you care for performance.
If ommitted, typescript needs to walk up the node's parent chain until it reaches the SourceFile. This *can* be quite costly when done
If ommitted, typescript needs to walk up the node's parent chain until it reaches the SourceFile. This *can* be quite costly when done
frequently on deeply nested nodes.

Some examples:
Expand All @@ -106,15 +99,15 @@ declare node: ts.Identifier;
node.getText() === node.text; // prefer node.text where available
```

__Bonus points:__ If you know the width of the node (either from the `text` property or because it is a keyword of known width),
__Bonus points:__ If you know the width of the node (either from the `text` property or because it is a keyword of known width),
you can use `node.getEnd() - width` to calculate the node's start.
`node.getEnd()` is effectively for free as it only returns the `end` property. This way you avoid the cost of skipping leading trivia.

### Make use of tail calls
Tail calls are function or method calls at the end of the control flow of a function. It's only a tail call if the return value of that call
is directly returned unchanged. Browsers can optimize this pattern for performance.
Further optimization is specced in ES2015 as "Proper Tail Calls".
With proper tail calls the browser reuses the stack frame of the current function. When done right this allows for infinite recursion.
With proper tail calls the browser reuses the stack frame of the current function. When done right this allows for infinite recursion.
```ts
function foo() {
if (condition)
Expand Down
7 changes: 3 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,9 @@
"compile:core": "tsc -p src",
"compile:scripts": "tsc -p scripts",
"compile:test": "tsc -p test",
"lint": "npm-run-all -p lint:core lint:test lint:from-bin",
"lint:core": "tslint \"src/**/*.ts\"",
"lint:test": "tslint \"test/**/*.ts\" -e \"test/**/*.test.ts\"",
"lint:from-bin": "node bin/tslint \"{src,test}/**/*.ts\" -e \"test/**/*.test.ts\"",
"lint": "npm-run-all -p lint:global lint:from-bin",
"lint:global": "tslint --project test/tsconfig.json --format stylish # test includes 'src' too",
"lint:from-bin": "node bin/tslint --project test/tsconfig.json --format stylish",
"test": "npm-run-all test:pre -p test:mocha test:rules",
"test:pre": "cd ./test/config && npm install",
"test:mocha": "mocha --reporter spec --colors \"build/test/**/*Tests.js\" build/test/assert.js",
Expand Down
1 change: 0 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export * from "./enableDisableRules";
export * from "./formatterLoader";
export * from "./ruleLoader";
export * from "./language/utils";
export * from "./language/languageServiceHost";
export * from "./language/walker";
export * from "./language/formatter/formatter";

Expand Down
89 changes: 0 additions & 89 deletions src/language/languageServiceHost.ts

This file was deleted.

2 changes: 1 addition & 1 deletion src/language/rule/abstractRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export abstract class AbstractRule implements IRule {
return this.options;
}

public abstract apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[];
public abstract apply(sourceFile: ts.SourceFile): RuleFailure[];

public applyWithWalker(walker: IWalker): RuleFailure[] {
walker.walk(walker.getSourceFile());
Expand Down
3 changes: 1 addition & 2 deletions src/language/rule/optionallyTypedRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,5 @@ import {AbstractRule} from "./abstractRule";
import {ITypedRule, RuleFailure} from "./rule";

export abstract class OptionallyTypedRule extends AbstractRule implements ITypedRule {

public abstract applyWithProgram(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[];
public abstract applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[];
}
4 changes: 2 additions & 2 deletions src/language/rule/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,12 @@ export interface IDisabledInterval {
export interface IRule {
getOptions(): IOptions;
isEnabled(): boolean;
apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[];
apply(sourceFile: ts.SourceFile): RuleFailure[];
applyWithWalker(walker: IWalker): RuleFailure[];
}

export interface ITypedRule extends IRule {
applyWithProgram(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[];
applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[];
}

export interface IRuleFailureJson {
Expand Down
2 changes: 1 addition & 1 deletion src/language/rule/typedRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,5 @@ export abstract class TypedRule extends AbstractRule implements ITypedRule {
throw new Error(`The '${this.ruleName}' rule requires type checking`);
}

public abstract applyWithProgram(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[];
public abstract applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[];
}
30 changes: 1 addition & 29 deletions src/language/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,35 +22,7 @@ import {IDisabledInterval, RuleFailure} from "./rule/rule";

export function getSourceFile(fileName: string, source: string): ts.SourceFile {
const normalizedName = path.normalize(fileName).replace(/\\/g, "/");
const compilerOptions = createCompilerOptions();

const compilerHost: ts.CompilerHost = {
fileExists: () => true,
getCanonicalFileName: (filename: string) => filename,
getCurrentDirectory: () => "",
getDefaultLibFileName: () => "lib.d.ts",
getDirectories: (_path: string) => [],
getNewLine: () => "\n",
getSourceFile: (filenameToGet: string) => {
const target = compilerOptions.target == null ? ts.ScriptTarget.ES5 : compilerOptions.target;
return ts.createSourceFile(filenameToGet, source, target, true);
},
readFile: (x: string) => x,
useCaseSensitiveFileNames: () => true,
writeFile: (x: string) => x,
};

const program = ts.createProgram([normalizedName], compilerOptions, compilerHost);

return program.getSourceFile(normalizedName);
}

export function createCompilerOptions(): ts.CompilerOptions {
return {
allowJs: true,
noResolve: true,
target: ts.ScriptTarget.ES5,
};
return ts.createSourceFile(normalizedName, source, ts.ScriptTarget.ES5, /*setParentNodes*/ true);
}

export function doesIntersect(failure: RuleFailure, disabledIntervals: IDisabledInterval[]) {
Expand Down
33 changes: 12 additions & 21 deletions src/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import { isError, showWarningOnce } from "./error";
import { findFormatter } from "./formatterLoader";
import { ILinterOptions, LintResult } from "./index";
import { IFormatter } from "./language/formatter/formatter";
import { createLanguageService, wrapProgram } from "./language/languageServiceHost";
import { Fix, IRule, isTypedRule, RuleFailure, RuleSeverity } from "./language/rule/rule";
import * as utils from "./language/utils";
import { loadRules } from "./ruleLoader";
Expand All @@ -53,7 +52,6 @@ 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.
Expand Down Expand Up @@ -93,10 +91,6 @@ 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 {
Expand Down Expand Up @@ -179,9 +173,9 @@ class Linter {
let ruleFailures: RuleFailure[] = [];
try {
if (this.program && isTypedRule(rule)) {
ruleFailures = rule.applyWithProgram(sourceFile, this.languageService);
ruleFailures = rule.applyWithProgram(sourceFile, this.program);
} else {
ruleFailures = rule.apply(sourceFile, this.languageService);
ruleFailures = rule.apply(sourceFile);
}
} catch (error) {
if (isError(error)) {
Expand Down Expand Up @@ -214,25 +208,22 @@ class Linter {
}

private getSourceFile(fileName: string, source: string) {
let sourceFile: ts.SourceFile;
if (this.program) {
sourceFile = this.program.getSourceFile(fileName);
const sourceFile = this.program.getSourceFile(fileName);
if (sourceFile === undefined) {
const INVALID_SOURCE_ERROR = dedent`
Invalid source file: ${fileName}. Ensure that the files supplied to lint have a .ts, .tsx, .js or .jsx extension.
`;
throw new Error(INVALID_SOURCE_ERROR);
}
// check if the program has been type checked
if (sourceFile && !("resolvedModules" in sourceFile)) {
if (!("resolvedModules" in sourceFile)) {
throw new Error("Program must be type checked before linting");
}
return sourceFile;
} else {
sourceFile = utils.getSourceFile(fileName, source);
this.languageService = createLanguageService(fileName, source);
}

if (sourceFile === undefined) {
const INVALID_SOURCE_ERROR = dedent`
Invalid source file: ${fileName}. Ensure that the files supplied to lint have a .ts, .tsx, .js or .jsx extension.
`;
throw new Error(INVALID_SOURCE_ERROR);
return utils.getSourceFile(fileName, source);
}
return sourceFile;
}

private containsRule(rules: RuleFailure[], rule: RuleFailure) {
Expand Down
4 changes: 2 additions & 2 deletions src/rules/awaitPromiseRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ export class Rule extends Lint.Rules.TypedRule {

public static FAILURE_STRING = "'await' of non-Promise.";

public applyWithProgram(srcFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] {
return this.applyWithWalker(new Walker(srcFile, this.getOptions(), langSvc.getProgram()));
public applyWithProgram(srcFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
return this.applyWithWalker(new Walker(srcFile, this.getOptions(), program));
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/rules/completedDocsRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,9 @@ export class Rule extends Lint.Rules.TypedRule {
};
/* tslint:enable:object-literal-sort-keys */

public applyWithProgram(sourceFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] {
public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
const options = this.getOptions();
const completedDocsWalker = new CompletedDocsWalker(sourceFile, options, langSvc.getProgram());
const completedDocsWalker = new CompletedDocsWalker(sourceFile, options, program);

completedDocsWalker.setRequirements(this.getRequirements(options.ruleArguments));

Expand Down
4 changes: 2 additions & 2 deletions src/rules/matchDefaultExportNameRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ export class Rule extends Lint.Rules.TypedRule {
return `Expected import '${importName}' to match the default export '${exportName}'.`;
}

public applyWithProgram(sourceFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] {
return this.applyWithWalker(new Walker(sourceFile, this.getOptions(), langSvc.getProgram()));
public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
return this.applyWithWalker(new Walker(sourceFile, this.getOptions(), program));
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/rules/noBooleanLiteralCompareRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ export class Rule extends Lint.Rules.TypedRule {
return `This expression is unnecessarily compared to a boolean. Just ${negate ? "negate it" : "use it directly"}.`;
}

public applyWithProgram(sourceFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] {
return this.applyWithWalker(new Walker(sourceFile, this.getOptions(), langSvc.getProgram()));
public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
return this.applyWithWalker(new Walker(sourceFile, this.getOptions(), program));
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/rules/noFloatingPromisesRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ export class Rule extends Lint.Rules.TypedRule {

public static FAILURE_STRING = "Promises must be handled appropriately";

public applyWithProgram(sourceFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] {
const walker = new NoFloatingPromisesWalker(sourceFile, this.getOptions(), langSvc.getProgram());
public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
const walker = new NoFloatingPromisesWalker(sourceFile, this.getOptions(), program);

for (const className of this.getOptions().ruleArguments) {
walker.addPromiseClass(className);
Expand Down
Loading