diff --git a/.gitignore b/.gitignore index 8a0d1147a6e..0018c1d0e48 100644 --- a/.gitignore +++ b/.gitignore @@ -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 diff --git a/docs/develop/custom-rules/performance.md b/docs/develop/custom-rules/performance.md index 51c4d6aba1a..e546039d26b 100644 --- a/docs/develop/custom-rules/performance.md +++ b/docs/develop/custom-rules/performance.md @@ -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. @@ -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 @@ -46,7 +39,7 @@ class MyWalker extends Lint.AbstractWalker { ### 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:__ @@ -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: @@ -106,7 +99,7 @@ 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. @@ -114,7 +107,7 @@ you can use `node.getEnd() - width` to calculate the node's start. 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) diff --git a/package.json b/package.json index 4e8c14afa6d..7dc3e73e327 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/src/index.ts b/src/index.ts index df8cd5c934f..e106badf6e4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -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"; diff --git a/src/language/languageServiceHost.ts b/src/language/languageServiceHost.ts deleted file mode 100644 index bab1df1c031..00000000000 --- a/src/language/languageServiceHost.ts +++ /dev/null @@ -1,89 +0,0 @@ -/** - * @license - * Copyright 2014 Palantir Technologies, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -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 = new Map(); // file name -> content - const fileVersions = new Map(); - const host: LanguageServiceEditableHost = { - getCompilationSettings: () => program.getCompilerOptions(), - getCurrentDirectory: () => program.getCurrentDirectory(), - getDefaultLibFileName: () => "lib.d.ts", - getScriptFileNames: () => program.getSourceFiles().map((sf) => sf.fileName), - getScriptSnapshot: (name: string) => { - const file = files.get(name); - if (file !== undefined) { - return ts.ScriptSnapshot.fromString(file); - } - if (!program.getSourceFile(name)) { - return undefined; - } - return ts.ScriptSnapshot.fromString(program.getSourceFile(name).getFullText()); - }, - getScriptVersion: (name: string) => { - const version = fileVersions.get(name); - return version === undefined ? "1" : String(version); - }, - log: () => { /* */ }, - editFile(fileName: string, newContent: string) { - files.set(fileName, newContent); - const prevVersion = fileVersions.get(fileName); - fileVersions.set(fileName, prevVersion === undefined ? 0 : prevVersion + 1); - }, - }; - 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(), - getCurrentDirectory: () => "", - getDefaultLibFileName: () => "lib.d.ts", - getScriptFileNames: () => [fileName], - getScriptSnapshot: (name: string) => ts.ScriptSnapshot.fromString(name === fileName ? source : ""), - getScriptVersion: () => "1", - log: () => { /* */ }, - }; -} - -export function createLanguageService(fileName: string, source: string) { - const languageServiceHost = createLanguageServiceHost(fileName, source); - return ts.createLanguageService(languageServiceHost); -} diff --git a/src/language/rule/abstractRule.ts b/src/language/rule/abstractRule.ts index 6942b4e00e0..a31168c1d81 100644 --- a/src/language/rule/abstractRule.ts +++ b/src/language/rule/abstractRule.ts @@ -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()); diff --git a/src/language/rule/optionallyTypedRule.ts b/src/language/rule/optionallyTypedRule.ts index cf5b8f6bed7..d2aad11d4b4 100644 --- a/src/language/rule/optionallyTypedRule.ts +++ b/src/language/rule/optionallyTypedRule.ts @@ -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[]; } diff --git a/src/language/rule/rule.ts b/src/language/rule/rule.ts index d253a92e7ae..1e841769cad 100644 --- a/src/language/rule/rule.ts +++ b/src/language/rule/rule.ts @@ -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 { diff --git a/src/language/rule/typedRule.ts b/src/language/rule/typedRule.ts index 1cc7d55a273..30e8cb5afe7 100644 --- a/src/language/rule/typedRule.ts +++ b/src/language/rule/typedRule.ts @@ -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[]; } diff --git a/src/language/utils.ts b/src/language/utils.ts index 039ddea4d78..3fdc4ba8292 100644 --- a/src/language/utils.ts +++ b/src/language/utils.ts @@ -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[]) { diff --git a/src/linter.ts b/src/linter.ts index 547cc19a35f..15ba445112e 100644 --- a/src/linter.ts +++ b/src/linter.ts @@ -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"; @@ -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. @@ -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 { @@ -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)) { @@ -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) { diff --git a/src/rules/awaitPromiseRule.ts b/src/rules/awaitPromiseRule.ts index 179ab17bccb..7197af22942 100644 --- a/src/rules/awaitPromiseRule.ts +++ b/src/rules/awaitPromiseRule.ts @@ -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)); } } diff --git a/src/rules/completedDocsRule.ts b/src/rules/completedDocsRule.ts index fa3847f250e..635d4a6eea9 100644 --- a/src/rules/completedDocsRule.ts +++ b/src/rules/completedDocsRule.ts @@ -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)); diff --git a/src/rules/matchDefaultExportNameRule.ts b/src/rules/matchDefaultExportNameRule.ts index 1c49fd9b8bb..16839488348 100644 --- a/src/rules/matchDefaultExportNameRule.ts +++ b/src/rules/matchDefaultExportNameRule.ts @@ -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)); } } diff --git a/src/rules/noBooleanLiteralCompareRule.ts b/src/rules/noBooleanLiteralCompareRule.ts index 6cc43df13bd..73df31152ab 100644 --- a/src/rules/noBooleanLiteralCompareRule.ts +++ b/src/rules/noBooleanLiteralCompareRule.ts @@ -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)); } } diff --git a/src/rules/noFloatingPromisesRule.ts b/src/rules/noFloatingPromisesRule.ts index 6f029e8d523..15b7cf8ccd1 100644 --- a/src/rules/noFloatingPromisesRule.ts +++ b/src/rules/noFloatingPromisesRule.ts @@ -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); diff --git a/src/rules/noForInArrayRule.ts b/src/rules/noForInArrayRule.ts index 8d060519247..3893655600c 100644 --- a/src/rules/noForInArrayRule.ts +++ b/src/rules/noForInArrayRule.ts @@ -49,9 +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, langSvc: ts.LanguageService): Lint.RuleFailure[] { - const noForInArrayWalker = new NoForInArrayWalker(sourceFile, this.getOptions(), langSvc.getProgram()); - return this.applyWithWalker(noForInArrayWalker); + public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + return this.applyWithWalker(new NoForInArrayWalker(sourceFile, this.getOptions(), program)); } } diff --git a/src/rules/noInferredEmptyObjectTypeRule.ts b/src/rules/noInferredEmptyObjectTypeRule.ts index 0976c0c7b7a..63f488ea5a8 100644 --- a/src/rules/noInferredEmptyObjectTypeRule.ts +++ b/src/rules/noInferredEmptyObjectTypeRule.ts @@ -36,8 +36,8 @@ export class Rule extends Lint.Rules.TypedRule { public static EMPTY_INTERFACE_INSTANCE = "Explicit type parameter needs to be provided to the constructor"; public static EMPTY_INTERFACE_FUNCTION = "Explicit type parameter needs to be provided to the function call"; - public applyWithProgram(srcFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] { - return this.applyWithWalker(new NoInferredEmptyObjectTypeRule(srcFile, this.getOptions(), langSvc.getProgram())); + public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + return this.applyWithWalker(new NoInferredEmptyObjectTypeRule(sourceFile, this.getOptions(), program)); } } diff --git a/src/rules/noUnboundMethodRule.ts b/src/rules/noUnboundMethodRule.ts index 556597491ff..b5b64f02599 100644 --- a/src/rules/noUnboundMethodRule.ts +++ b/src/rules/noUnboundMethodRule.ts @@ -34,8 +34,8 @@ export class Rule extends Lint.Rules.TypedRule { public static FAILURE_STRING = "Avoid referencing unbound methods which may cause unintentional scoping of 'this'."; - public applyWithProgram(srcFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] { - return this.applyWithWalker(new Walker(srcFile, this.getOptions(), langSvc.getProgram())); + public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + return this.applyWithWalker(new Walker(sourceFile, this.getOptions(), program)); } } diff --git a/src/rules/noUnnecessaryQualifierRule.ts b/src/rules/noUnnecessaryQualifierRule.ts index 8d4e4232c7a..6fdefff3d2f 100644 --- a/src/rules/noUnnecessaryQualifierRule.ts +++ b/src/rules/noUnnecessaryQualifierRule.ts @@ -40,8 +40,8 @@ export class Rule extends Lint.Rules.TypedRule { return `Qualifier is unnecessary since '${name}' is in scope.`; } - 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)); } } diff --git a/src/rules/noUnsafeAnyRule.ts b/src/rules/noUnsafeAnyRule.ts index e3a8d4dfc66..d5cfc992c7a 100644 --- a/src/rules/noUnsafeAnyRule.ts +++ b/src/rules/noUnsafeAnyRule.ts @@ -38,8 +38,8 @@ export class Rule extends Lint.Rules.TypedRule { public static FAILURE_STRING = "Unsafe use of expression of type 'any'."; - public applyWithProgram(sourceFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] { - return this.applyWithFunction(sourceFile, (ctx) => walk(ctx, langSvc.getProgram().getTypeChecker())); + public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + return this.applyWithFunction(sourceFile, (ctx) => walk(ctx, program.getTypeChecker())); } } diff --git a/src/rules/noUnusedVariableRule.ts b/src/rules/noUnusedVariableRule.ts index 95b88c0c47f..d897af528cf 100644 --- a/src/rules/noUnusedVariableRule.ts +++ b/src/rules/noUnusedVariableRule.ts @@ -15,19 +15,15 @@ * limitations under the License. */ +import * as utils from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; -const OPTION_REACT = "react"; const OPTION_CHECK_PARAMETERS = "check-parameters"; +const OPTION_IGNORE_PATTERN = "ignore-pattern"; -const REACT_MODULES = ["react", "react/addons"]; -const REACT_NAMESPACE_IMPORT_NAME = "React"; - -const MODULE_SPECIFIER_MATCH = /^["'](.+)['"]$/; - -export class Rule extends Lint.Rules.AbstractRule { +export class Rule extends Lint.Rules.TypedRule { /* tslint:disable:object-literal-sort-keys */ public static metadata: Lint.IRuleMetadata = { ruleName: "no-unused-variable", @@ -41,10 +37,6 @@ export class Rule extends Lint.Rules.AbstractRule { * \`"check-parameters"\` disallows unused function and constructor parameters. * NOTE: this option is experimental and does not work with classes that use abstract method declarations, among other things. - * \`"react"\` relaxes the rule for a namespace import named \`React\` - (from either the module \`"react"\` or \`"react/addons"\`). - Any JSX expression in the file will be treated as a usage of \`React\` - (because it expands to \`React.createElement \`). * \`{"ignore-pattern": "pattern"}\` where pattern is a case-sensitive regexp. Variable names that match the pattern will be ignored.`, options: { @@ -70,397 +62,319 @@ export class Rule extends Lint.Rules.AbstractRule { }; /* tslint:enable:object-literal-sort-keys */ - public static FAILURE_TYPE_FUNC = "function"; - public static FAILURE_TYPE_IMPORT = "import"; - public static FAILURE_TYPE_METHOD = "method"; - public static FAILURE_TYPE_PARAM = "parameter"; - public static FAILURE_TYPE_PROP = "property"; - public static FAILURE_TYPE_VAR = "variable"; - public static FAILURE_STRING_FACTORY = (type: string, name: string) => { - return `Unused ${type}: '${name}'`; - } + public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + const x = program.getCompilerOptions(); + if (x.noUnusedLocals && x.noUnusedParameters) { + console.warn("WARNING: 'no-unused-variable' lint rule does not need to be set if " + + "the 'no-unused-locals' and 'no-unused-parameters' compiler options are enabled."); + } - public apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): Lint.RuleFailure[] { - return this.applyWithWalker(new NoUnusedVariablesWalker(sourceFile, this.getOptions(), languageService)); + return this.applyWithFunction(sourceFile, (ctx) => walk(ctx, program, parseOptions(this.getOptions().ruleArguments))); } } -interface Failure { - start: number; - width: number; - message: string; - fix?: Lint.Fix; +interface Options { + checkParameters: boolean; + ignorePattern: RegExp | undefined; } - -class NoUnusedVariablesWalker extends Lint.RuleWalker { - private skipBindingElement: boolean; - private skipParameterDeclaration: boolean; - private skipVariableDeclaration: boolean; - - private hasSeenJsxElement: boolean; - private ignorePattern: RegExp; - private isReactUsed: boolean; - private reactImport: ts.NamespaceImport; - private possibleFailures: Failure[] = []; - - constructor(sourceFile: ts.SourceFile, options: Lint.IOptions, - private languageService: ts.LanguageService) { - super(sourceFile, options); - this.skipVariableDeclaration = false; - this.skipParameterDeclaration = false; - this.hasSeenJsxElement = false; - this.isReactUsed = false; - - const ignorePatternOption = this.getOptions().filter((option: any) => { - return typeof option === "object" && option["ignore-pattern"] != null; - })[0]; - if (ignorePatternOption != null) { - this.ignorePattern = new RegExp(ignorePatternOption["ignore-pattern"]); +function parseOptions(options: any[]): Options { + const checkParameters = options.indexOf(OPTION_CHECK_PARAMETERS) !== -1; + + let ignorePattern: RegExp | undefined; + for (const o of options) { + if (typeof o === "object" && o[OPTION_IGNORE_PATTERN] != null) { + ignorePattern = new RegExp(o[OPTION_IGNORE_PATTERN]); + break; } } - public visitSourceFile(node: ts.SourceFile) { - super.visitSourceFile(node); - - /* - * After super.visitSourceFile() is completed, this.reactImport will be set to a NamespaceImport iff: - * - * - a react option has been provided to the rule and - * - an import of a module that matches one of OPTION_REACT_MODULES is found, to a - * NamespaceImport named OPTION_REACT_NAMESPACE_IMPORT_NAME - * - * e.g. - * - * import * as React from "react/addons"; - * - * If reactImport is defined when a walk is completed, we need to have: - * - * a) seen another usage of React and/or - * b) seen a JSX identifier - * - * otherwise a a variable usage failure will will be reported - */ - if (this.hasOption(OPTION_REACT) - && this.reactImport != null - && !this.isReactUsed - && !this.hasSeenJsxElement) { - const nameText = this.reactImport.name.getText(); - if (!this.isIgnored(nameText)) { - const start = this.reactImport.name.getStart(); - const msg = Rule.FAILURE_STRING_FACTORY(Rule.FAILURE_TYPE_IMPORT, nameText); - this.possibleFailures.push({ start, width: nameText.length, message: msg }); - } - } + return { checkParameters, ignorePattern }; +} - let someFixBrokeIt = false; - // Performance optimization: type-check the whole file before verifying individual fixes - if (this.possibleFailures.some((f) => f.fix !== undefined)) { - const newText = Lint.Fix.applyAll(this.getSourceFile().getFullText(), - this.possibleFailures.map((f) => f.fix!).filter((f) => f !== undefined)); +function walk(ctx: Lint.WalkContext, program: ts.Program, { checkParameters, ignorePattern }: Options): void { + const { sourceFile } = ctx; + const unusedCheckedProgram = getUnusedCheckedProgram(program, checkParameters); + const diagnostics = ts.getPreEmitDiagnostics(unusedCheckedProgram, sourceFile); + const checker = unusedCheckedProgram.getTypeChecker(); // Doesn't matter which program is used for this. - // 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; - } + // If all specifiers in an import are unused, we elide the entire import. + const importSpecifierFailures = new Map(); + + for (const diag of diagnostics) { + const kind = getUnusedDiagnostic(diag); + if (kind === undefined) { + continue; } - this.possibleFailures.forEach((f) => { - if (!someFixBrokeIt || f.fix === undefined) { - this.addFailureAt(f.start, f.width, f.message, f.fix); - } else { - const newText = f.fix.apply(this.getSourceFile().getFullText()); - if (Lint.checkEdit(this.languageService, this.getSourceFile(), newText).length === 0) { - this.addFailureAt(f.start, f.width, f.message, f.fix); - } - } - }); - } + const failure = ts.flattenDiagnosticMessageText(diag.messageText, "\n"); - public visitBindingElement(node: ts.BindingElement) { - const isSingleVariable = node.name.kind === ts.SyntaxKind.Identifier; + if (kind === UnusedKind.VARIABLE_OR_PARAMETER) { + const importName = findImport(diag.start, sourceFile); + if (importName !== undefined) { + if (isImportUsed(importName, sourceFile, checker)) { + continue; + } - if (isSingleVariable && !this.skipBindingElement) { - const variableIdentifier = node.name as ts.Identifier; - this.validateReferencesForVariable(Rule.FAILURE_TYPE_VAR, variableIdentifier.text, variableIdentifier.getStart()); + if (importSpecifierFailures.has(importName)) { + throw new Error("Should not get 2 errors for the same import."); + } + importSpecifierFailures.set(importName, failure); + continue; + } } - super.visitBindingElement(node); - } - - public visitCatchClause(node: ts.CatchClause) { - // don't visit the catch clause variable declaration, just visit the block - // the catch clause variable declaration needs to be there but doesn't need to be used - this.visitBlock(node.block); - } - - // skip exported and declared functions - public visitFunctionDeclaration(node: ts.FunctionDeclaration) { - if (!Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword, ts.SyntaxKind.DeclareKeyword) && node.name !== undefined) { - const variableName = node.name.text; - this.validateReferencesForVariable(Rule.FAILURE_TYPE_FUNC, variableName, node.name.getStart()); + if (ignorePattern) { + const varName = /'(.*)'/.exec(failure)![1]; + if (ignorePattern.test(varName)) { + continue; + } } - super.visitFunctionDeclaration(node); + ctx.addFailureAt(diag.start, diag.length, failure); } - public visitFunctionType(node: ts.FunctionOrConstructorTypeNode) { - this.skipParameterDeclaration = true; - super.visitFunctionType(node); - this.skipParameterDeclaration = false; + if (importSpecifierFailures.size) { + addImportSpecifierFailures(ctx, importSpecifierFailures, sourceFile); } +} - public visitImportDeclaration(node: ts.ImportDeclaration) { - const importClause = node.importClause; +/** + * Handle import-specifier failures separately. + * - If all of the import specifiers in an import are unused, add a combined failure for them all. + * - Unused imports are fixable. + */ +function addImportSpecifierFailures(ctx: Lint.WalkContext, failures: Map, sourceFile: ts.SourceFile) { + forEachImport(sourceFile, (importNode) => { + if (importNode.kind === ts.SyntaxKind.ImportEqualsDeclaration) { + tryRemoveAll(importNode.name); + return; + } - // If the imports are exported, they may be used externally - if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword) || - // importClause will be null for bare imports - importClause == null) { - super.visitImportDeclaration(node); + if (!importNode.importClause) { + // Error node return; } - // Two passes: first collect what's unused, then produce failures. This allows the fix to lookahead. - let usesDefaultImport = false; - let usedNamedImports: boolean[] = []; - if (importClause.name != null) { - const variableIdentifier = importClause.name; - usesDefaultImport = this.isUsed(variableIdentifier.text, variableIdentifier.getStart()); + const { name: defaultName, namedBindings } = importNode.importClause; + if (namedBindings && namedBindings.kind === ts.SyntaxKind.NamespaceImport) { + tryRemoveAll(namedBindings.name); + return; } - if (importClause.namedBindings != null) { - if (importClause.namedBindings.kind === ts.SyntaxKind.NamedImports && node.importClause !== undefined) { - const imports = node.importClause.namedBindings as ts.NamedImports; - usedNamedImports = imports.elements.map((e) => this.isUsed(e.name.text, e.name.getStart())); - } - // Avoid deleting the whole statement if there's an import * inside - if (importClause.namedBindings.kind === ts.SyntaxKind.NamespaceImport) { - usesDefaultImport = true; + + const allNamedBindingsAreFailures = !namedBindings || namedBindings.elements.every((e) => failures.has(e.name)); + if (namedBindings && allNamedBindingsAreFailures) { + for (const e of namedBindings.elements) { + failures.delete(e.name); } } - // Delete the entire import statement if named and default imports all unused - if (!usesDefaultImport && usedNamedImports.every((e) => !e)) { - this.fail(Rule.FAILURE_TYPE_IMPORT, node.getText(), node.getStart(), this.deleteImportStatement(node)); - super.visitImportDeclaration(node); + if ((!defaultName || failures.has(defaultName)) && allNamedBindingsAreFailures) { + if (defaultName) { failures.delete(defaultName); } + removeAll(importNode, "All imports are unused."); return; } - // Delete the default import and trailing comma if unused - if (importClause.name != null && !usesDefaultImport && importClause.namedBindings !== undefined) { - // There must be some named imports or we would have been in case 1 - const end = importClause.namedBindings.getStart(); - this.fail(Rule.FAILURE_TYPE_IMPORT, importClause.name.text, importClause.name.getStart(), [ - this.deleteText(importClause.name.getStart(), end - importClause.name.getStart()), - ]); + if (defaultName) { + const failure = tryDelete(defaultName); + if (failure !== undefined) { + const start = defaultName.getStart(); + const end = namedBindings ? namedBindings.getStart() : importNode.moduleSpecifier.getStart(); + const fix = ctx.createFix(Lint.Replacement.deleteFromTo(start, end)); + ctx.addFailureAtNode(defaultName, failure, fix); + } } - if (importClause.namedBindings != null && - importClause.namedBindings.kind === ts.SyntaxKind.NamedImports) { - // Delete the entire named imports if all unused, including curly braces. - if (usedNamedImports.every((e) => !e)) { - const start = importClause.name != null ? importClause.name.getEnd() : importClause.namedBindings.getStart(); - this.fail(Rule.FAILURE_TYPE_IMPORT, importClause.namedBindings.getText(), importClause.namedBindings.getStart(), [ - this.deleteText(start, importClause.namedBindings.getEnd() - start), - ]); + + if (namedBindings) { + if (allNamedBindingsAreFailures) { + const start = defaultName ? defaultName.getEnd() : namedBindings.getStart(); + const fix = ctx.createFix(Lint.Replacement.deleteFromTo(start, namedBindings.getEnd())); + const failure = "All named bindings are unused."; + ctx.addFailureAtNode(namedBindings, failure, fix); } else { - const imports = importClause.namedBindings as ts.NamedImports; - let priorElementUsed = false; - for (let idx = 0; idx < imports.elements.length; idx++) { - const namedImport = imports.elements[idx]; - if (usedNamedImports[idx]) { - priorElementUsed = true; - } else { - const isLast = idx === imports.elements.length - 1; - // Before the first used import, consume trailing commas. - // Afterward, consume leading commas instead. - const start = priorElementUsed ? imports.elements[idx - 1].getEnd() : namedImport.getStart(); - const end = priorElementUsed || isLast ? namedImport.getEnd() : imports.elements[idx + 1].getStart(); - this.fail(Rule.FAILURE_TYPE_IMPORT, namedImport.name.text, namedImport.name.getStart(), [ - this.deleteText(start, end - start), - ]); + const { elements } = namedBindings; + for (let i = 0; i < elements.length; i++) { + const element = elements[i]; + const failure = tryDelete(element.name); + if (failure === undefined) { + continue; } + + const prevElement = elements[i - 1]; + const nextElement = elements[i + 1]; + const start = prevElement ? prevElement.getEnd() : element.getStart(); + const end = nextElement && !prevElement ? nextElement.getStart() : element.getEnd(); + const fix = ctx.createFix(Lint.Replacement.deleteFromTo(start, end)); + ctx.addFailureAtNode(element.name, failure, fix); } } } - // import x = 'y' & import * as x from 'y' handled by other walker methods - // because they only have one identifier that might be unused - super.visitImportDeclaration(node); - } - - public visitImportEqualsDeclaration(node: ts.ImportEqualsDeclaration) { - if (!Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword)) { - const name = node.name; - this.validateReferencesForVariable(Rule.FAILURE_TYPE_IMPORT, name.text, name.getStart(), this.deleteImportStatement(node)); + function tryRemoveAll(name: ts.Identifier): void { + const failure = tryDelete(name); + if (failure !== undefined) { + removeAll(name, failure); + } } - super.visitImportEqualsDeclaration(node); - } - // skip parameters in index signatures (stuff like [key: string]: string) - public visitIndexSignatureDeclaration(node: ts.IndexSignatureDeclaration) { - this.skipParameterDeclaration = true; - super.visitIndexSignatureDeclaration(node); - this.skipParameterDeclaration = false; - } + function removeAll(errorNode: ts.Node, failure: string): void { + const fix = ctx.createFix(Lint.Replacement.deleteFromTo(importNode.getStart(), importNode.getEnd())); + ctx.addFailureAtNode(errorNode, failure, fix); + } + }); - // skip parameters in interfaces - public visitInterfaceDeclaration(node: ts.InterfaceDeclaration) { - this.skipParameterDeclaration = true; - super.visitInterfaceDeclaration(node); - this.skipParameterDeclaration = false; + if (failures.size) { + throw new Error("Should have revisited all import specifier failures."); } - public visitJsxElement(node: ts.JsxElement) { - this.hasSeenJsxElement = true; - super.visitJsxElement(node); + function tryDelete(name: ts.Identifier): string | undefined { + const failure = failures.get(name); + if (failure !== undefined) { + failures.delete(name); + return failure; + } + return undefined; } +} - public visitJsxSelfClosingElement(node: ts.JsxSelfClosingElement) { - this.hasSeenJsxElement = true; - super.visitJsxSelfClosingElement(node); +/** + * Ignore this import if it's used as an implicit type somewhere. + * Workround for https://github.com/Microsoft/TypeScript/issues/9944 + */ +function isImportUsed(importSpecifier: ts.Identifier, sourceFile: ts.SourceFile, checker: ts.TypeChecker): boolean { + let symbol = checker.getSymbolAtLocation(importSpecifier); + if (!symbol) { + return false; } - // check private member functions - public visitMethodDeclaration(node: ts.MethodDeclaration) { - if (node.name != null && node.name.kind === ts.SyntaxKind.Identifier) { - const modifiers = node.modifiers; - const variableName = (node.name as ts.Identifier).text; - - if (Lint.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword)) { - this.validateReferencesForVariable(Rule.FAILURE_TYPE_METHOD, variableName, node.name.getStart()); - } - } - - // abstract methods can't have a body so their parameters are always unused - if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.AbstractKeyword)) { - this.skipParameterDeclaration = true; - } - super.visitMethodDeclaration(node); - this.skipParameterDeclaration = false; + symbol = checker.getAliasedSymbol(symbol); + if (!Lint.isSymbolFlagSet(symbol, ts.SymbolFlags.Type)) { + return false; } - public visitNamespaceImport(node: ts.NamespaceImport) { - if (node.parent !== undefined) { - const importDeclaration = node.parent.parent as ts.ImportDeclaration; - const moduleSpecifier = importDeclaration.moduleSpecifier.getText(); - - // extract the unquoted module being imported - const moduleNameMatch = moduleSpecifier.match(MODULE_SPECIFIER_MATCH); - const isReactImport = (moduleNameMatch != null) && (REACT_MODULES.indexOf(moduleNameMatch[1]) !== -1); - - if (this.hasOption(OPTION_REACT) && isReactImport && node.name.text === REACT_NAMESPACE_IMPORT_NAME) { - this.reactImport = node; - const fileName = this.getSourceFile().fileName; - const position = node.name.getStart(); - const highlights = this.languageService.getDocumentHighlights(fileName, position, [fileName]); - if (highlights != null && highlights[0].highlightSpans.length > 1) { - this.isReactUsed = true; - } - } else { - this.validateReferencesForVariable(Rule.FAILURE_TYPE_IMPORT, node.name.text, node.name.getStart(), - this.deleteImportStatement(importDeclaration)); - } + return ts.forEachChild(sourceFile, function cb(child): boolean { + if (isImportLike(child)) { + return false; } - super.visitNamespaceImport(node); - } - public visitParameterDeclaration(node: ts.ParameterDeclaration) { - const isSingleVariable = node.name.kind === ts.SyntaxKind.Identifier; - const isPropertyParameter = Lint.hasModifier( - node.modifiers, - ts.SyntaxKind.PublicKeyword, - ts.SyntaxKind.PrivateKeyword, - ts.SyntaxKind.ProtectedKeyword, - ); - - if (!isSingleVariable && isPropertyParameter) { - // tsc error: a parameter property may not be a binding pattern - this.skipBindingElement = true; + const type = getImplicitType(child, checker); + // TODO: checker.typeEquals https://github.com/Microsoft/TypeScript/issues/13502 + if (type && checker.typeToString(type) === checker.symbolToString(symbol)) { + return true; } - if (this.hasOption(OPTION_CHECK_PARAMETERS) - && isSingleVariable - && !this.skipParameterDeclaration - && !Lint.hasModifier(node.modifiers, ts.SyntaxKind.PublicKeyword)) { - const nameNode = node.name as ts.Identifier; - this.validateReferencesForVariable(Rule.FAILURE_TYPE_PARAM, nameNode.text, node.name.getStart()); - } + return ts.forEachChild(child, cb); + }); +} - super.visitParameterDeclaration(node); - this.skipBindingElement = false; +function getImplicitType(node: ts.Node, checker: ts.TypeChecker): ts.Type | undefined { + if ((utils.isPropertyDeclaration(node) || utils.isVariableDeclaration(node)) && !node.type) { + return checker.getTypeAtLocation(node); + } else if (utils.isSignatureDeclaration(node) && !node.type) { + return checker.getSignatureFromDeclaration(node).getReturnType(); + } else { + return undefined; } +} - // check private member variables - public visitPropertyDeclaration(node: ts.PropertyDeclaration) { - if (node.name != null && node.name.kind === ts.SyntaxKind.Identifier) { - const modifiers = node.modifiers; - const variableName = (node.name as ts.Identifier).text; +type ImportLike = ts.ImportDeclaration | ts.ImportEqualsDeclaration; +function isImportLike(node: ts.Node): node is ImportLike { + return node.kind === ts.SyntaxKind.ImportDeclaration || node.kind === ts.SyntaxKind.ImportEqualsDeclaration; +} - // check only if an explicit 'private' modifier is specified - if (Lint.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword)) { - this.validateReferencesForVariable(Rule.FAILURE_TYPE_PROP, variableName, node.name.getStart()); +function forEachImport(sourceFile: ts.SourceFile, f: (i: ImportLike) => T | undefined): T | undefined { + return ts.forEachChild(sourceFile, (child) => { + if (isImportLike(child)) { + const res = f(child); + if (res !== undefined) { + return res; } } + return undefined; + }); +} - super.visitPropertyDeclaration(node); - } - - public visitVariableDeclaration(node: ts.VariableDeclaration) { - const isSingleVariable = node.name.kind === ts.SyntaxKind.Identifier; - - if (isSingleVariable && !this.skipVariableDeclaration) { - const variableIdentifier = node.name as ts.Identifier; - this.validateReferencesForVariable(Rule.FAILURE_TYPE_VAR, variableIdentifier.text, variableIdentifier.getStart()); - } +function findImport(pos: number, sourceFile: ts.SourceFile): ts.Identifier | undefined { + return forEachImport(sourceFile, (i) => { + if (i.kind === ts.SyntaxKind.ImportEqualsDeclaration) { + if (i.name.getStart() === pos) { + return i.name; + } + } else { + if (!i.importClause) { + // Error node + return undefined; + } - super.visitVariableDeclaration(node); - } + const { name: defaultName, namedBindings } = i.importClause; + if (namedBindings && namedBindings.kind === ts.SyntaxKind.NamespaceImport) { + const { name } = namedBindings; + if (name.getStart() === pos) { + return name; + } + return undefined; + } - // skip exported and declared variables - public visitVariableStatement(node: ts.VariableStatement) { - if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword, ts.SyntaxKind.DeclareKeyword)) { - this.skipBindingElement = true; - this.skipVariableDeclaration = true; + if (defaultName && defaultName.getStart() === pos) { + return defaultName; + } else if (namedBindings) { + for (const { name } of namedBindings.elements) { + if (name.getStart() === pos) { + return name; + } + } + } } + return undefined; + }); +} - super.visitVariableStatement(node); - this.skipBindingElement = false; - this.skipVariableDeclaration = false; - } - - /** - * Delete the statement along with leading trivia. - * BUT since imports are typically at the top of the file, the leading trivia is often a license. - * So when the leading trivia includes a block comment, delete the statement without leading trivia instead. - */ - private deleteImportStatement(node: ts.Statement): Lint.Replacement[] { - if (node.getFullText().substr(0, node.getLeadingTriviaWidth()).indexOf("/*") >= 0) { - return [this.deleteText(node.getStart(), node.getWidth())]; - } - return [this.deleteText(node.getFullStart(), node.getFullWidth())]; +const enum UnusedKind { + VARIABLE_OR_PARAMETER, + PROPERTY, +} +function getUnusedDiagnostic(diag: ts.Diagnostic): UnusedKind | undefined { + switch (diag.code) { + case 6133: + return UnusedKind.VARIABLE_OR_PARAMETER; // "'{0}' is declared but never used. + case 6138: + return UnusedKind.PROPERTY; // "Property '{0}' is declared but never used." + default: + return undefined; } +} - private validateReferencesForVariable(type: string, name: string, position: number, replacements?: Lint.Replacement[]) { - if (!this.isUsed(name, position)) { - this.fail(type, name, position, replacements); - } - } +const programToUnusedCheckedProgram = new WeakMap(); - private isUsed(name: string, position: number): boolean { - const fileName = this.getSourceFile().fileName; - const highlights = this.languageService.getDocumentHighlights(fileName, position, [fileName]); - return (highlights != null && highlights[0].highlightSpans.length > 1) || this.isIgnored(name); +function getUnusedCheckedProgram(program: ts.Program, checkParameters: boolean): ts.Program { + // Assuming checkParameters will always have the same value, so only lookup by program. + let checkedProgram = programToUnusedCheckedProgram.get(program); + if (checkedProgram) { + return checkedProgram; } - private fail(type: string, name: string, position: number, replacements?: Lint.Replacement[]) { - let fix: Lint.Fix | undefined; - if (replacements && replacements.length) { - fix = new Lint.Fix(Rule.metadata.ruleName, replacements); - } - this.possibleFailures.push({ start: position, width: name.length, message: Rule.FAILURE_STRING_FACTORY(type, name), fix }); - } + checkedProgram = makeUnusedCheckedProgram(program, checkParameters); + programToUnusedCheckedProgram.set(program, checkedProgram); + return checkedProgram; +} - private isIgnored(name: string) { - return this.ignorePattern != null && this.ignorePattern.test(name); - } +function makeUnusedCheckedProgram(program: ts.Program, checkParameters: boolean): ts.Program { + const options = { ...program.getCompilerOptions(), noUnusedLocals: true, ...(checkParameters ? { noUnusedParameters: true } : null) }; + const sourceFilesByName = new Map(program.getSourceFiles().map<[string, ts.SourceFile]>((s) => [s.fileName, s])); + // tslint:disable object-literal-sort-keys + return ts.createProgram(Array.from(sourceFilesByName.keys()), options, { + fileExists: (f) => sourceFilesByName.has(f), + readFile(f) { + const s = sourceFilesByName.get(f)!; + return s.text; + }, + getSourceFile: (f) => sourceFilesByName.get(f)!, + getDefaultLibFileName: () => ts.getDefaultLibFileName(options), + writeFile: () => {}, // tslint:disable-line no-empty + getCurrentDirectory: () => "", + getDirectories: () => [], + getCanonicalFileName: (f) => f, + useCaseSensitiveFileNames: () => true, + getNewLine: () => "\n", + }); + // tslint:enable object-literal-sort-keys } diff --git a/src/rules/noUseBeforeDeclareRule.ts b/src/rules/noUseBeforeDeclareRule.ts index 1dcdbbff6f7..8d0d438ef12 100644 --- a/src/rules/noUseBeforeDeclareRule.ts +++ b/src/rules/noUseBeforeDeclareRule.ts @@ -19,7 +19,7 @@ import * as ts from "typescript"; import * as Lint from "../index"; -export class Rule extends Lint.Rules.AbstractRule { +export class Rule extends Lint.Rules.TypedRule { /* tslint:disable:object-literal-sort-keys */ public static metadata: Lint.IRuleMetadata = { ruleName: "no-use-before-declare", @@ -35,110 +35,55 @@ export class Rule extends Lint.Rules.AbstractRule { }; /* tslint:enable:object-literal-sort-keys */ - public static FAILURE_STRING_PREFIX = "variable '"; - public static FAILURE_STRING_POSTFIX = "' used before declaration"; - - public apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): Lint.RuleFailure[] { - return this.applyWithWalker(new NoUseBeforeDeclareWalker(sourceFile, this.getOptions(), languageService)); - } -} - -type VisitedVariables = Set; - -class NoUseBeforeDeclareWalker extends Lint.ScopeAwareRuleWalker { - private importedPropertiesPositions: number[] = []; - - constructor(sourceFile: ts.SourceFile, options: Lint.IOptions, private languageService: ts.LanguageService) { - super(sourceFile, options); + public static FAILURE_STRING(name: string) { + return `variable '${name}' used before declaration`; } - public createScope(): VisitedVariables { - return new Set(); + public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + return this.applyWithFunction(sourceFile, (ctx) => walk(ctx, program.getTypeChecker())); } +} - public visitBindingElement(node: ts.BindingElement) { - const isSingleVariable = node.name.kind === ts.SyntaxKind.Identifier; - const isBlockScoped = Lint.isBlockScopedBindingElement(node); - - // use-before-declare errors for block-scoped vars are caught by tsc - if (isSingleVariable && !isBlockScoped) { - const variableName = (node.name as ts.Identifier).text; - this.validateUsageForVariable(variableName, node.name.getStart()); +function walk(ctx: Lint.WalkContext, checker: ts.TypeChecker): void { + return ts.forEachChild(ctx.sourceFile, function recur(node: ts.Node): void { + switch (node.kind) { + case ts.SyntaxKind.TypeReference: + // Ignore types. + return; + case ts.SyntaxKind.PropertyAccessExpression: + // Ignore `y` in `x.y`, but recurse to `x`. + return recur((node as ts.PropertyAccessExpression).expression); + case ts.SyntaxKind.Identifier: + return checkIdentifier(node as ts.Identifier, checker.getSymbolAtLocation(node)); + case ts.SyntaxKind.ExportSpecifier: + return checkIdentifier( + (node as ts.ExportSpecifier).name, + checker.getExportSpecifierLocalTargetSymbol(node as ts.ExportSpecifier)); + default: + return ts.forEachChild(node, recur); } + }); - super.visitBindingElement(node); - } - - public visitImportDeclaration(node: ts.ImportDeclaration) { - const importClause = node.importClause; - - // named imports & namespace imports handled by other walker methods - // importClause will be null for bare imports - if (importClause != null && importClause.name != null) { - const variableIdentifier = importClause.name; - this.validateUsageForVariable(variableIdentifier.text, variableIdentifier.getStart()); + function checkIdentifier(node: ts.Identifier, symbol: ts.Symbol | undefined): void { + const declarations = symbol && symbol.declarations; + if (!declarations) { + return; } - super.visitImportDeclaration(node); - } - - public visitImportEqualsDeclaration(node: ts.ImportEqualsDeclaration) { - const name = node.name as ts.Identifier; - this.validateUsageForVariable(name.text, name.getStart()); - - super.visitImportEqualsDeclaration(node); - } - - public visitNamedImports(node: ts.NamedImports) { - for (const namedImport of node.elements) { - if (namedImport.propertyName != null) { - this.saveImportedPropertiesPositions(namedImport.propertyName.getStart()); + const declaredBefore = declarations.some((decl) => { + switch (decl.kind) { + case ts.SyntaxKind.FunctionDeclaration: + // Functions may be declared later. + return true; + default: + // Use `<=` in case this *is* the declaration. + // If it's a global declared in a different file, OK. + return decl.pos <= node.pos || decl.getSourceFile() !== ctx.sourceFile; } - this.validateUsageForVariable(namedImport.name.text, namedImport.name.getStart()); - } - super.visitNamedImports(node); - } - - public visitNamespaceImport(node: ts.NamespaceImport) { - this.validateUsageForVariable(node.name.text, node.name.getStart()); - super.visitNamespaceImport(node); - } - - public visitVariableDeclaration(node: ts.VariableDeclaration) { - const isSingleVariable = node.name.kind === ts.SyntaxKind.Identifier; - const variableName = (node.name as ts.Identifier).text; - const currentScope = this.getCurrentScope(); + }); - // only validate on the first variable declaration within the current scope - if (isSingleVariable && !currentScope.has(variableName)) { - this.validateUsageForVariable(variableName, node.getStart()); + if (!declaredBefore) { + ctx.addFailureAtNode(node, Rule.FAILURE_STRING(node.text)); } - - currentScope.add(variableName); - super.visitVariableDeclaration(node); - } - - private validateUsageForVariable(name: string, position: number) { - const fileName = this.getSourceFile().fileName; - const highlights = this.languageService.getDocumentHighlights(fileName, position, [fileName]); - if (highlights != null) { - for (const highlight of highlights) { - for (const highlightSpan of highlight.highlightSpans) { - const referencePosition = highlightSpan.textSpan.start; - if (referencePosition < position && !this.isImportedPropertyName(referencePosition)) { - const failureString = Rule.FAILURE_STRING_PREFIX + name + Rule.FAILURE_STRING_POSTFIX; - this.addFailureAt(referencePosition, name.length, failureString); - } - } - } - } - } - - private saveImportedPropertiesPositions(position: number): void { - this.importedPropertiesPositions.push(position); - } - - private isImportedPropertyName(position: number): boolean { - return this.importedPropertiesPositions.indexOf(position) !== -1; } } diff --git a/src/rules/noVoidExpressionRule.ts b/src/rules/noVoidExpressionRule.ts index c27063e2a2c..134459e410d 100644 --- a/src/rules/noVoidExpressionRule.ts +++ b/src/rules/noVoidExpressionRule.ts @@ -35,8 +35,8 @@ export class Rule extends Lint.Rules.TypedRule { public static FAILURE_STRING = "Expression has type `void`. Put it on its own line as a statement."; - 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)); } } diff --git a/src/rules/promiseFunctionAsyncRule.ts b/src/rules/promiseFunctionAsyncRule.ts index 5303df0b02f..8cf252af111 100644 --- a/src/rules/promiseFunctionAsyncRule.ts +++ b/src/rules/promiseFunctionAsyncRule.ts @@ -40,8 +40,8 @@ export class Rule extends Lint.Rules.TypedRule { public static FAILURE_STRING = "functions that return promises must be async"; - public applyWithProgram(sourceFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] { - return this.applyWithWalker(new PromiseAsyncWalker(sourceFile, this.getOptions(), langSvc.getProgram())); + public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + return this.applyWithWalker(new PromiseAsyncWalker(sourceFile, this.getOptions(), program)); } } diff --git a/src/rules/restrictPlusOperandsRule.ts b/src/rules/restrictPlusOperandsRule.ts index 6f303d1fa48..ed10ed6610f 100644 --- a/src/rules/restrictPlusOperandsRule.ts +++ b/src/rules/restrictPlusOperandsRule.ts @@ -36,8 +36,8 @@ export class Rule extends Lint.Rules.TypedRule { public static INVALID_TYPES_ERROR = "Operands of '+' operation must either be both strings or both numbers"; - public applyWithProgram(sourceFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] { - return this.applyWithWalker(new RestrictPlusOperandsWalker(sourceFile, this.getOptions(), langSvc.getProgram())); + public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + return this.applyWithWalker(new RestrictPlusOperandsWalker(sourceFile, this.getOptions(), program)); } } diff --git a/src/rules/strictBooleanExpressionsRule.ts b/src/rules/strictBooleanExpressionsRule.ts index 3fc7d5a0ffc..9be4407f07d 100644 --- a/src/rules/strictBooleanExpressionsRule.ts +++ b/src/rules/strictBooleanExpressionsRule.ts @@ -94,8 +94,8 @@ export class Rule extends Lint.Rules.TypedRule { } } - public applyWithProgram(srcFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] { - return this.applyWithWalker(new Walker(srcFile, this.getOptions(), langSvc.getProgram())); + public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + return this.applyWithWalker(new Walker(sourceFile, this.getOptions(), program)); } } diff --git a/src/rules/strictTypePredicatesRule.ts b/src/rules/strictTypePredicatesRule.ts index b2372d0b6e1..da81584edae 100644 --- a/src/rules/strictTypePredicatesRule.ts +++ b/src/rules/strictTypePredicatesRule.ts @@ -49,8 +49,8 @@ export class Rule extends Lint.Rules.TypedRule { return `Use '${isPositive ? "===" : "!=="} ${value}' instead.`; } - public applyWithProgram(srcFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] { - return this.applyWithWalker(new Walker(srcFile, this.getOptions(), langSvc.getProgram())); + public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + return this.applyWithWalker(new Walker(sourceFile, this.getOptions(), program)); } } diff --git a/src/runner.ts b/src/runner.ts index 825376a5880..484e24231dd 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -147,7 +147,7 @@ export class Runner { console.error("Invalid option for project: " + this.options.project); return onComplete(1); } - program = Linter.createProgram(this.options.project, path.dirname(this.options.project)); + program = Linter.createProgram(this.options.project); if (files.length === 0) { files = Linter.getFileNames(program); } diff --git a/src/test.ts b/src/test.ts index d53fc776961..22bb95af0c2 100644 --- a/src/test.ts +++ b/src/test.ts @@ -254,9 +254,9 @@ function displayDiffResults(diffResults: diff.IDiffResult[], extension: string) for (const diffResult of diffResults) { let color = colors.grey; if (diffResult.added) { - color = colors.green; + color = colors.green.underline; } else if (diffResult.removed) { - color = colors.red; + color = colors.red.underline; } process.stdout.write(color(diffResult.value)); } diff --git a/src/test/lines.ts b/src/test/lines.ts index 0f9aeae6422..d4aeab81a06 100644 --- a/src/test/lines.ts +++ b/src/test/lines.ts @@ -90,6 +90,10 @@ export function printLine(line: Line, code?: string): string | null { return `${leadingSpaces}${tildes}`; } else if (line instanceof EndErrorLine) { let tildes = "~".repeat(line.endCol - line.startCol); + if (code.length < line.endCol) { + // Better than crashing in String.repeat + throw new Error("Bad error marker at " + JSON.stringify(line)); + } let endSpaces = " ".repeat(code.length - line.endCol); if (tildes.length === 0) { tildes = ZERO_LENGTH_ERROR; diff --git a/test/ruleLoaderTests.ts b/test/ruleLoaderTests.ts index cbdcfeb77fc..9366ae95e11 100644 --- a/test/ruleLoaderTests.ts +++ b/test/ruleLoaderTests.ts @@ -123,23 +123,4 @@ describe("Rule Loader", () => { assert.isFalse(testFailed, "List of rules doesn't match list of tests"); }); - - it("ensures that `.ts` files in `rules/` end in `.test.ts` to avoid being linted", () => { - walkSync(testRulesDir, (filename) => { - if (/\.ts$/.test(filename)) { - assert.match(filename, /\.test\.ts$/); - } - }); - }); - - const walkSync = (dir: string, cb: (filename: string) => void) => { - fs.readdirSync(dir).forEach((file) => { - const fullPath = path.join(dir, file); - if (fs.statSync(path.join(dir, file)).isDirectory()) { - walkSync(fullPath, cb); - } else { - cb(fullPath); - } - }); - }; }); diff --git a/test/rules/no-unused-variable/check-parameters/test.ts.lint b/test/rules/no-unused-variable/check-parameters/test.ts.lint index e8a66eed72e..50845d47d72 100644 --- a/test/rules/no-unused-variable/check-parameters/test.ts.lint +++ b/test/rules/no-unused-variable/check-parameters/test.ts.lint @@ -1,15 +1,17 @@ +[typescript]: >= 2.1.0 + export function func1(x: number, y: number, ...args: number[]) { - ~~~~ [Unused parameter: 'args'] + ~~~~ ['args' is declared but never used.] return x + y; } export function func2(x: number, y: number, ...args: number[]) { - ~ [Unused parameter: 'y'] + ~ ['y' is declared but never used.] return x + args[0]; } export function func3(x?: number, y?: number) { - ~ [Unused parameter: 'y'] + ~ ['y' is declared but never used.] return x; } @@ -19,7 +21,7 @@ export interface ITestInterface { export class ABCD { constructor(private x: number, public y: number, private z: number) { - ~ [Unused parameter: 'x'] + ~ [Property 'x' is declared but never used.] } func5() { @@ -40,7 +42,7 @@ export function func7(f: (x: number) => number) { } export function func8([x, y]: [number, number]) { - ~ [Unused variable: 'y'] + ~ ['y' is declared but never used.] return x; } @@ -49,22 +51,23 @@ export class DestructuringTests { } public func9({a, b}) { - ~ [Unused variable: 'b'] + ~ ['b' is declared but never used.] return a; } public func10([a, b]) { - ~ [Unused variable: 'b'] + ~ ['b' is declared but never used.] return [a]; } // destructuring with default value public func11([x = 0]) { - ~ [Unused variable: 'x'] + ~ ['x' is declared but never used.] return; } } abstract class AbstractTest { + ~~~~~~~~~~~~ ['AbstractTest' is declared but never used.] abstract foo(x); -} \ No newline at end of file +} diff --git a/test/rules/no-unused-variable/check-parameters/tslint.json b/test/rules/no-unused-variable/check-parameters/tslint.json index ab3b45be1f5..62b8abc06c2 100644 --- a/test/rules/no-unused-variable/check-parameters/tslint.json +++ b/test/rules/no-unused-variable/check-parameters/tslint.json @@ -1,4 +1,7 @@ { + "linterOptions": { + "typeCheck": true + }, "rules": { "no-unused-variable": [true, "check-parameters"] } diff --git a/test/rules/no-unused-variable/default/class.ts.lint b/test/rules/no-unused-variable/default/class.ts.lint index 478611183ee..4d0422c6acd 100644 --- a/test/rules/no-unused-variable/default/class.ts.lint +++ b/test/rules/no-unused-variable/default/class.ts.lint @@ -1,6 +1,6 @@ class ABCD { private z2: number; - ~~ [Unused property: 'z2'] + ~~ ['z2' is declared but never used.] constructor() { } @@ -17,7 +17,7 @@ class ABCD { } private mfunc4() { - ~~~~~~ [Unused method: 'mfunc4'] + ~~~~~~ ['mfunc4' is declared but never used.] // } static stillPublic: number; diff --git a/test/rules/no-unused-variable/default/false-positives.ts.lint b/test/rules/no-unused-variable/default/false-positives.ts.lint index bfd2b0ac19d..3c9a527dbd2 100644 --- a/test/rules/no-unused-variable/default/false-positives.ts.lint +++ b/test/rules/no-unused-variable/default/false-positives.ts.lint @@ -2,6 +2,7 @@ const fs = require("fs"); module Foo { + ~~~ ['Foo' is declared but never used.] const path = require("path"); console.log(fs); @@ -23,6 +24,7 @@ hello.sayHello(); import Bar = whatever.that.Foo; module some.module.blah { + ~~~~ ['some' is declared but never used.] export class bar { private bar: Bar; constructor() { @@ -41,12 +43,12 @@ interface MyDateTimeOpts extends DateTimeOpts { let opts: MyDateTimeOpts; console.log(opts.timezoneOffset - 1); -import * as myLib from 'myLib'; +import * as myLib from 'a'; export { myLib }; -import foo from 'foo'; +import foo from 'a'; const bar = {foo}; myFunc(bar); -import a from "module"; +import a from "a"; export { a }; diff --git a/test/rules/no-unused-variable/default/function.ts.lint b/test/rules/no-unused-variable/default/function.ts.lint index 15c9a0e5acc..7a578f87ade 100644 --- a/test/rules/no-unused-variable/default/function.ts.lint +++ b/test/rules/no-unused-variable/default/function.ts.lint @@ -3,12 +3,12 @@ function func1(x: number, y: number) { } var func2 = () => { - ~~~~~ [Unused variable: 'func2'] + ~~~~~ ['func2' is declared but never used.] // }; function func3() { - ~~~~~ [Unused function: 'func3'] + ~~~~~ ['func3' is declared but never used.] return func1(1, 2); } @@ -17,6 +17,7 @@ export function func4() { } declare function func5(): any; + ~~~~~ ['func5' is declared but never used.] export default function () { return 0; diff --git a/test/rules/no-unused-variable/default/import.ts.fix b/test/rules/no-unused-variable/default/import.ts.fix index 081f3e24ca0..2156e83c509 100644 --- a/test/rules/no-unused-variable/default/import.ts.fix +++ b/test/rules/no-unused-variable/default/import.ts.fix @@ -3,17 +3,20 @@ * This text will not be deleted. */ -import $ = require("jquery"); -import _ = require("underscore"); -import {a2 as aa2} from "modA"; +import $ = require("a"); +import _ = require("a"); +import {a2 as aa2} from "a"; aa2; -import {a3 as aa3} from "modA"; +import {a3 as aa3} from "a"; aa3; -import {a8} from "modA"; +// This import statement is unused and will be deleted along with this comment. + + +import {a8} from "a"; a8; -import {a13} from "modA"; +import {a13} from "a"; a13; -import {a14, a16} from "modA"; +import {a14, a16} from "a"; a14; a16; @@ -26,18 +29,21 @@ $(_.xyz()); module S { var template = ""; } -import * as bar from "libB"; -import baz from "libC"; -import { namedExport } from "libD"; -import d3 from "libD"; + + +import * as bar from "a"; +import baz from "a"; +import { namedExport } from "a"; + +import d3 from "a"; d3; bar.someFunc(); baz(); namedExport(); -import "jquery"; +import "a"; -import abc = require('abc'); +import abc = require('a'); import def = abc.someVar; console.log(def); diff --git a/test/rules/no-unused-variable/default/import.ts.lint b/test/rules/no-unused-variable/default/import.ts.lint index 539dde7c3e1..b0c2450c72a 100644 --- a/test/rules/no-unused-variable/default/import.ts.lint +++ b/test/rules/no-unused-variable/default/import.ts.lint @@ -2,31 +2,31 @@ * Some license that appears in the leading trivia of a statement that will be deleted. * This text will not be deleted. */ -import xyz = require("xyz"); - ~~~ [Unused import: 'xyz'] -import $ = require("jquery"); -import _ = require("underscore"); -import {a1 as aa1, a2 as aa2} from "modA"; - ~~~ [Unused import: 'aa1'] +import xyz = require("a"); + ~~~ ['xyz' is declared but never used.] +import $ = require("a"); +import _ = require("a"); +import {a1 as aa1, a2 as aa2} from "a"; + ~~~ ['aa1' is declared but never used.] aa2; -import {a3 as aa3, a4 as aa4} from "modA"; - ~~~ [Unused import: 'aa4'] +import {a3 as aa3, a4 as aa4} from "a"; + ~~~ ['aa4' is declared but never used.] aa3; // This import statement is unused and will be deleted along with this comment. -import {a5, a6} from "modA"; -~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Unused import: 'import {a5, a6} from "modA";'] -import {a7} from "modA"; -~~~~~~~~~~~~~~~~~~~~~~~~ [Unused import: 'import {a7} from "modA";'] -import {a8, a9, a10} from "modA"; - ~~ [Unused import: 'a9'] - ~~~ [Unused import: 'a10'] +import {a5, a6} from "a"; +~~~~~~~~~~~~~~~~~~~~~~~~~ [All imports are unused.] +import {a7} from "a"; +~~~~~~~~~~~~~~~~~~~~~ [All imports are unused.] +import {a8, a9, a10} from "a"; + ~~ ['a9' is declared but never used.] + ~~~ ['a10' is declared but never used.] a8; -import {a11, a12, a13} from "modA"; - ~~~ [Unused import: 'a11'] - ~~~ [Unused import: 'a12'] +import {a11, a12, a13} from "a"; + ~~~ ['a11' is declared but never used.] + ~~~ ['a12' is declared but never used.] a13; -import {a14, a15, a16} from "modA"; - ~~~ [Unused import: 'a15'] +import {a14, a15, a16} from "a"; + ~~~ ['a15' is declared but never used.] a14; a16; @@ -37,28 +37,29 @@ $(_.xyz()); /// module S { + ~ ['S' is declared but never used.] var template = ""; - ~~~~~~~~ [Unused variable: 'template'] + ~~~~~~~~ ['template' is declared but never used.] } -import * as foo from "libA"; - ~~~ [Unused import: 'foo'] -import * as bar from "libB"; -import baz from "libC"; -import defaultExport, { namedExport } from "libD"; - ~~~~~~~~~~~~~ [Unused import: 'defaultExport'] -import d1, { d2 } from "libD"; -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Unused import: 'import d1, { d2 } from "libD";'] -import d3, { d4 } from "libD"; - ~~~~~~ [Unused import: '{ d4 }'] +import * as foo from "a"; + ~~~ ['foo' is declared but never used.] +import * as bar from "a"; +import baz from "a"; +import defaultExport, { namedExport } from "a"; + ~~~~~~~~~~~~~ ['defaultExport' is declared but never used.] +import d1, { d2 } from "a"; +~~~~~~~~~~~~~~~~~~~~~~~~~~~ [All imports are unused.] +import d3, { d4 } from "a"; + ~~~~~~ [All named bindings are unused.] d3; bar.someFunc(); baz(); namedExport(); -import "jquery"; +import "a"; -import abc = require('abc'); +import abc = require('a'); import def = abc.someVar; console.log(def); diff --git a/test/rules/no-unused-variable/default/node_modules/a.ts b/test/rules/no-unused-variable/default/node_modules/a.ts new file mode 100644 index 00000000000..85e5dcf1ed9 --- /dev/null +++ b/test/rules/no-unused-variable/default/node_modules/a.ts @@ -0,0 +1,2 @@ +declare var x: any; +export = x; diff --git a/test/rules/no-unused-variable/default/node_modules/react.ts b/test/rules/no-unused-variable/default/node_modules/react.ts new file mode 100644 index 00000000000..6c1838c3425 --- /dev/null +++ b/test/rules/no-unused-variable/default/node_modules/react.ts @@ -0,0 +1 @@ +export default x = 0; \ No newline at end of file diff --git a/test/rules/no-unused-variable/default/node_modules/react/addons.ts b/test/rules/no-unused-variable/default/node_modules/react/addons.ts new file mode 100644 index 00000000000..d6d31ab1196 --- /dev/null +++ b/test/rules/no-unused-variable/default/node_modules/react/addons.ts @@ -0,0 +1 @@ +export const x = 0; \ No newline at end of file diff --git a/test/rules/no-unused-variable/default/react-addons1.tsx.lint b/test/rules/no-unused-variable/default/react-addons1.tsx.lint index 247e4d22361..262c73a24d4 100644 --- a/test/rules/no-unused-variable/default/react-addons1.tsx.lint +++ b/test/rules/no-unused-variable/default/react-addons1.tsx.lint @@ -1,4 +1,5 @@ +[typescript]: >= 2.1.0 + import * as React from "react/addons"; - ~~~~~ [Unused import: 'React'] console.log(
); diff --git a/test/rules/no-unused-variable/default/react-addons3.tsx.lint b/test/rules/no-unused-variable/default/react-addons3.tsx.lint index fa8d065a4fb..23f968bc76e 100644 --- a/test/rules/no-unused-variable/default/react-addons3.tsx.lint +++ b/test/rules/no-unused-variable/default/react-addons3.tsx.lint @@ -1,2 +1,2 @@ import * as React from "react/addons"; - ~~~~~ [Unused import: 'React'] + ~~~~~ ['React' is declared but never used.] diff --git a/test/rules/no-unused-variable/default/react1.tsx.lint b/test/rules/no-unused-variable/default/react1.tsx.lint index afe2ccddd22..57924397649 100644 --- a/test/rules/no-unused-variable/default/react1.tsx.lint +++ b/test/rules/no-unused-variable/default/react1.tsx.lint @@ -1,4 +1,5 @@ +[typescript]: >= 2.1.0 + import * as React from "react"; - ~~~~~ [Unused import: 'React'] console.log(
); diff --git a/test/rules/no-unused-variable/default/react3.tsx.lint b/test/rules/no-unused-variable/default/react3.tsx.lint index 3dd18ddaa18..44c3a5ebe06 100644 --- a/test/rules/no-unused-variable/default/react3.tsx.lint +++ b/test/rules/no-unused-variable/default/react3.tsx.lint @@ -1,2 +1,2 @@ import * as React from "react"; - ~~~~~ [Unused import: 'React'] + ~~~~~ ['React' is declared but never used.] diff --git a/test/rules/no-unused-variable/default/react4.tsx.lint b/test/rules/no-unused-variable/default/react4.tsx.lint index 41a115bb2da..44c3a5ebe06 100644 --- a/test/rules/no-unused-variable/default/react4.tsx.lint +++ b/test/rules/no-unused-variable/default/react4.tsx.lint @@ -1,4 +1,2 @@ import * as React from "react"; - ~~~~~ [Unused import: 'React'] - -console.log(); + ~~~~~ ['React' is declared but never used.] diff --git a/test/rules/no-unused-variable/default/tslint.json b/test/rules/no-unused-variable/default/tslint.json index b1d833a0963..e81cc662cf1 100644 --- a/test/rules/no-unused-variable/default/tslint.json +++ b/test/rules/no-unused-variable/default/tslint.json @@ -1,4 +1,7 @@ { + "linterOptions": { + "typeCheck": true + }, "rules": { "no-unused-variable": true } diff --git a/test/rules/no-unused-variable/default/var.ts.lint b/test/rules/no-unused-variable/default/var.ts.lint index 84a173b47da..6734eb7b31c 100644 --- a/test/rules/no-unused-variable/default/var.ts.lint +++ b/test/rules/no-unused-variable/default/var.ts.lint @@ -1,12 +1,13 @@ var x = 3; var y = x; - ~ [Unused variable: 'y'] + ~ ['y' is declared but never used.] var z; export var abcd = 3; class ABCD { + ~~~~ ['ABCD' is declared but never used.] constructor() { z = 3; } @@ -19,16 +20,17 @@ try { } declare var tmp: any; + ~~~ ['tmp' is declared but never used.] export function testDestructuring() { var [a, b] = [1, 2]; - ~ [Unused variable: 'a'] - ~ [Unused variable: 'b'] + ~ ['a' is declared but never used.] + ~ ['b' is declared but never used.] var [c] = [3]; var {d, e} = { d: 1, e: 2 }; - ~ [Unused variable: 'd'] - ~ [Unused variable: 'e'] + ~ ['d' is declared but never used.] + ~ ['e' is declared but never used.] var {f} = { f: 3 }; return c * f; @@ -38,7 +40,7 @@ export var [foo, bar] = [1, 2]; export function testUnusedSpread() { var a = [1, 2]; - ~ [Unused variable: 'a'] + ~ ['a' is declared but never used.] var b = [3, 4]; var c = [...b, 5]; // make sure we see that b is being used @@ -47,14 +49,14 @@ export function testUnusedSpread() { } for(let e of [1,2,3]) { - ~ [Unused variable: 'e'] + ~ ['e' is declared but never used.] } export function testRenamingDestructure() { var a = 2; let {a: b} = {a: 4}; - ~ [Unused variable: 'b'] + ~ ['b' is declared but never used.] let {x: y} = {x: 7}; // false positive return a + y; } diff --git a/test/rules/no-unused-variable/ignore-pattern/react.tsx.lint b/test/rules/no-unused-variable/ignore-pattern/react.tsx.lint deleted file mode 100644 index 832fcf6505b..00000000000 --- a/test/rules/no-unused-variable/ignore-pattern/react.tsx.lint +++ /dev/null @@ -1,3 +0,0 @@ -import * as React from "react"; - -console.log(
); diff --git a/test/rules/no-unused-variable/ignore-pattern/tslint.json b/test/rules/no-unused-variable/ignore-pattern/tslint.json index 6e81221de24..b8d027ab315 100644 --- a/test/rules/no-unused-variable/ignore-pattern/tslint.json +++ b/test/rules/no-unused-variable/ignore-pattern/tslint.json @@ -1,4 +1,7 @@ { + "linterOptions": { + "typeCheck": true + }, "rules": { "no-unused-variable": [true, {"ignore-pattern": "^[_R]"}] } diff --git a/test/rules/no-unused-variable/ignore-pattern/var.ts.lint b/test/rules/no-unused-variable/ignore-pattern/var.ts.lint index 098b4492528..451eebe5fce 100644 --- a/test/rules/no-unused-variable/ignore-pattern/var.ts.lint +++ b/test/rules/no-unused-variable/ignore-pattern/var.ts.lint @@ -1,13 +1,14 @@ var x = 3; var y = x; - ~ [Unused variable: 'y'] + ~ ['y' is declared but never used.] var _y = x; var z; export var abcd = 3; class ABCD { + ~~~~ ['ABCD' is declared but never used.] constructor() { z = 3; } @@ -20,17 +21,18 @@ try { } declare var tmp: any; + ~~~ ['tmp' is declared but never used.] export function testDestructuring() { var [a, b] = [1, 2]; - ~ [Unused variable: 'a'] - ~ [Unused variable: 'b'] + ~ ['a' is declared but never used.] + ~ ['b' is declared but never used.] var [_a, _b] = [1, 2]; var [c] = [3]; var {d, e} = { d: 1, e: 2 }; - ~ [Unused variable: 'd'] - ~ [Unused variable: 'e'] + ~ ['d' is declared but never used.] + ~ ['e' is declared but never used.] var {d: _d, e: _e} = { d: 1, e: 2 }; var {f} = { f: 3 }; @@ -41,7 +43,7 @@ export var [foo, bar] = [1, 2]; export function testUnusedSpread() { var a = [1, 2]; - ~ [Unused variable: 'a'] + ~ ['a' is declared but never used.] var _a = [1, 2]; var b = [3, 4]; var c = [...b, 5]; // make sure we see that b is being used @@ -51,7 +53,7 @@ export function testUnusedSpread() { } for(let e of [1,2,3]) { - ~ [Unused variable: 'e'] + ~ ['e' is declared but never used.] } @@ -62,7 +64,7 @@ for(let _e of [1,2,3]) { export function testRenamingDestructure() { var a = 2; let {a: b} = {a: 4}; - ~ [Unused variable: 'b'] + ~ ['b' is declared but never used.] let {a: _b} = {a: 4}; let {x: y} = {x: 7}; // false positive return a + y; diff --git a/test/rules/no-unused-variable/react/react-addons1.tsx.lint b/test/rules/no-unused-variable/react/react-addons1.tsx.lint deleted file mode 100644 index 4fc6872f28c..00000000000 --- a/test/rules/no-unused-variable/react/react-addons1.tsx.lint +++ /dev/null @@ -1,3 +0,0 @@ -import * as React from "react/addons"; - -console.log(
); diff --git a/test/rules/no-unused-variable/react/react-addons2.tsx.lint b/test/rules/no-unused-variable/react/react-addons2.tsx.lint deleted file mode 100644 index 259677ba1b1..00000000000 --- a/test/rules/no-unused-variable/react/react-addons2.tsx.lint +++ /dev/null @@ -1,3 +0,0 @@ -import * as React from "react/addons"; - -export class MyComponent extends React.Component<{}, {}> {} diff --git a/test/rules/no-unused-variable/react/react-addons3.tsx.lint b/test/rules/no-unused-variable/react/react-addons3.tsx.lint deleted file mode 100644 index fa8d065a4fb..00000000000 --- a/test/rules/no-unused-variable/react/react-addons3.tsx.lint +++ /dev/null @@ -1,2 +0,0 @@ -import * as React from "react/addons"; - ~~~~~ [Unused import: 'React'] diff --git a/test/rules/no-unused-variable/react/react1.tsx.lint b/test/rules/no-unused-variable/react/react1.tsx.lint deleted file mode 100644 index 832fcf6505b..00000000000 --- a/test/rules/no-unused-variable/react/react1.tsx.lint +++ /dev/null @@ -1,3 +0,0 @@ -import * as React from "react"; - -console.log(
); diff --git a/test/rules/no-unused-variable/react/react2.tsx.lint b/test/rules/no-unused-variable/react/react2.tsx.lint deleted file mode 100644 index 0045f6b2065..00000000000 --- a/test/rules/no-unused-variable/react/react2.tsx.lint +++ /dev/null @@ -1,3 +0,0 @@ -import * as React from "react"; - -export class MyComponent extends React.Component<{}, {}> {} diff --git a/test/rules/no-unused-variable/react/react3.tsx.lint b/test/rules/no-unused-variable/react/react3.tsx.lint deleted file mode 100644 index 3dd18ddaa18..00000000000 --- a/test/rules/no-unused-variable/react/react3.tsx.lint +++ /dev/null @@ -1,2 +0,0 @@ -import * as React from "react"; - ~~~~~ [Unused import: 'React'] diff --git a/test/rules/no-unused-variable/react/react4.tsx.lint b/test/rules/no-unused-variable/react/react4.tsx.lint deleted file mode 100644 index 2e342fcc080..00000000000 --- a/test/rules/no-unused-variable/react/react4.tsx.lint +++ /dev/null @@ -1,3 +0,0 @@ -import * as React from "react"; - -console.log(); \ No newline at end of file diff --git a/test/rules/no-unused-variable/react/tslint.json b/test/rules/no-unused-variable/react/tslint.json deleted file mode 100644 index e6e2a72813c..00000000000 --- a/test/rules/no-unused-variable/react/tslint.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "rules": { - "no-unused-variable": [true, "react"] - } -} diff --git a/test/rules/no-use-before-declare/$.ts b/test/rules/no-use-before-declare/$.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/rules/no-use-before-declare/ImportAliasSecond.ts b/test/rules/no-use-before-declare/ImportAliasSecond.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/rules/no-use-before-declare/ImportAliasWithComment.ts b/test/rules/no-use-before-declare/ImportAliasWithComment.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/rules/no-use-before-declare/ImportRegularAlias.ts b/test/rules/no-use-before-declare/ImportRegularAlias.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/rules/no-use-before-declare/ImportWithLineBreaks.ts b/test/rules/no-use-before-declare/ImportWithLineBreaks.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/rules/no-use-before-declare/InterfaceFile.ts b/test/rules/no-use-before-declare/InterfaceFile.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/rules/no-use-before-declare/lib.ts b/test/rules/no-use-before-declare/lib.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/rules/no-use-before-declare/test.js.lint b/test/rules/no-use-before-declare/test.js.lint index 8c51be3ba61..bfdf4948de0 100644 --- a/test/rules/no-use-before-declare/test.js.lint +++ b/test/rules/no-use-before-declare/test.js.lint @@ -1,6 +1,6 @@ $.x = 3; ~ [variable '$' used before declaration] -import $ from "$"; +import $ from "./$"; var vara = varb, varb; ~~~~ [variable 'varb' used before declaration] @@ -34,6 +34,8 @@ if (something) { var defined; } +testUndeclaredImports(); // Function early reference OK + function testUndeclaredImports() { console.log(foo1); ~~~~ [variable 'foo1' used before declaration] @@ -45,11 +47,11 @@ function testUndeclaredImports() { ~~~ [variable 'map' used before declaration] } -import { default as foo1 } from "lib"; -import foo2 from "lib"; -import _, { map, foldl } from "underscore"; -import * as foo3 from "lib"; -import "lib"; +import { default as foo1 } from "./lib"; +import foo2 from "./lib"; +import _, { map, foldl } from "./underscore"; +import * as foo3 from "./lib"; +import "./lib"; export default function () { // @@ -58,8 +60,10 @@ export default function () { export { undeclaredA, ~~~~~~~~~~~ [variable 'undeclaredA' used before declaration] - undeclaredB, // tsc error - undeclaredC, // tsc error + undeclaredB, + ~~~~~~~~~~~ [variable 'undeclaredB' used before declaration] + undeclaredC, + ~~~~~~~~~~~ [variable 'undeclaredC' used before declaration] testUndeclaredImports }; diff --git a/test/rules/no-use-before-declare/test.ts.lint b/test/rules/no-use-before-declare/test.ts.lint index d98b31ddf96..f6effa2de63 100644 --- a/test/rules/no-use-before-declare/test.ts.lint +++ b/test/rules/no-use-before-declare/test.ts.lint @@ -1,6 +1,6 @@ $.x = 3; ~ [variable '$' used before declaration] -import $ = require("$"); +import $ = require("./$"); var vara = varb, varb; ~~~~ [variable 'varb' used before declaration] @@ -34,6 +34,8 @@ if (something) { var defined; } +testUndeclaredImports(); // Function early reference OK + function testUndeclaredImports() { console.log(foo1); ~~~~ [variable 'foo1' used before declaration] @@ -45,11 +47,11 @@ function testUndeclaredImports() { ~~~ [variable 'map' used before declaration] } -import { default as foo1 } from "lib"; -import foo2 from "lib"; -import _, { map, foldl } from "underscore"; -import * as foo3 from "lib"; -import "lib"; +import { default as foo1 } from "./lib"; +import foo2 from "./lib"; +import _, { map, foldl } from "./underscore"; +import * as foo3 from "./lib"; +import "./lib"; export default function () { // @@ -58,8 +60,10 @@ export default function () { export { undeclaredA, ~~~~~~~~~~~ [variable 'undeclaredA' used before declaration] - undeclaredB, // tsc error - undeclaredC, // tsc error + undeclaredB, + ~~~~~~~~~~~ [variable 'undeclaredB' used before declaration] + undeclaredC, + ~~~~~~~~~~~ [variable 'undeclaredC' used before declaration] testUndeclaredImports }; @@ -81,3 +85,10 @@ import { import {First, ITest as ITest3 } from './ImportAliasSecond'; import ITest from './InterfaceFile'; + +class Greeter { + greet(options: { str: string }) { + var {str} = options; + return str; + } +} diff --git a/test/rules/no-use-before-declare/tslint.json b/test/rules/no-use-before-declare/tslint.json index 113653e7cd5..1b2660b466e 100644 --- a/test/rules/no-use-before-declare/tslint.json +++ b/test/rules/no-use-before-declare/tslint.json @@ -1,4 +1,7 @@ { + "linterOptions": { + "typeCheck": true + }, "rules": { "no-use-before-declare": true }, diff --git a/test/rules/no-use-before-declare/underscore.ts b/test/rules/no-use-before-declare/underscore.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/tsconfig.json b/test/tsconfig.json index 685d2f2fb98..cdc452339dc 100644 --- a/test/tsconfig.json +++ b/test/tsconfig.json @@ -17,6 +17,7 @@ "**/*" ], "exclude": [ - "./files" + "./files", + "./rules" ] } diff --git a/tslint.json b/tslint.json index 2e7d0a00e60..ec934571111 100644 --- a/tslint.json +++ b/tslint.json @@ -1,4 +1,7 @@ { + "lintOptions": { + "typeCheck": true + }, "extends": "tslint:latest", "rules": { "callable-types": true,