From e3ff521cd71adb3a4cb6ed2497c8f3ec2054b2ca Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 22 Dec 2016 19:04:05 +0100 Subject: [PATCH 01/10] Moved is*ScopeBoundary methods to utils The methods are still available and simply delegate to the utility function to remain backwards compatible. The methods can be removed in the next major version. --- src/language/utils.ts | 33 +++++++++++++++++++ .../walker/blockScopeAwareRuleWalker.ts | 19 ++--------- src/language/walker/scopeAwareRuleWalker.ts | 22 ++----------- 3 files changed, 39 insertions(+), 35 deletions(-) diff --git a/src/language/utils.ts b/src/language/utils.ts index 1c58f7fcf64..7638a60a0b2 100644 --- a/src/language/utils.ts +++ b/src/language/utils.ts @@ -197,3 +197,36 @@ export function unwrapParentheses(node: ts.Expression) { } return node; } + +export function isScopeBoundary(node: ts.Node): boolean { + return node.kind === ts.SyntaxKind.FunctionDeclaration + || node.kind === ts.SyntaxKind.FunctionExpression + || node.kind === ts.SyntaxKind.PropertyAssignment + || node.kind === ts.SyntaxKind.ShorthandPropertyAssignment + || node.kind === ts.SyntaxKind.MethodDeclaration + || node.kind === ts.SyntaxKind.Constructor + || node.kind === ts.SyntaxKind.ModuleDeclaration + || node.kind === ts.SyntaxKind.ArrowFunction + || node.kind === ts.SyntaxKind.ParenthesizedExpression + || node.kind === ts.SyntaxKind.ClassDeclaration + || node.kind === ts.SyntaxKind.ClassExpression + || node.kind === ts.SyntaxKind.InterfaceDeclaration + || node.kind === ts.SyntaxKind.GetAccessor + || node.kind === ts.SyntaxKind.SetAccessor + || node.kind === ts.SyntaxKind.SourceFile && ts.isExternalModule(node as ts.SourceFile); +} + +export function isBlockScopeBoundary(node: ts.Node): boolean { + return isScopeBoundary(node) + || node.kind === ts.SyntaxKind.Block + || node.kind === ts.SyntaxKind.DoStatement + || node.kind === ts.SyntaxKind.WhileStatement + || node.kind === ts.SyntaxKind.ForStatement + || node.kind === ts.SyntaxKind.ForInStatement + || node.kind === ts.SyntaxKind.ForOfStatement + || node.kind === ts.SyntaxKind.WithStatement + || node.kind === ts.SyntaxKind.SwitchStatement + || node.parent !== undefined + && (node.parent.kind === ts.SyntaxKind.TryStatement + || node.parent.kind === ts.SyntaxKind.IfStatement); +} diff --git a/src/language/walker/blockScopeAwareRuleWalker.ts b/src/language/walker/blockScopeAwareRuleWalker.ts index 4f962de6e95..aa8a9df26fe 100644 --- a/src/language/walker/blockScopeAwareRuleWalker.ts +++ b/src/language/walker/blockScopeAwareRuleWalker.ts @@ -17,6 +17,7 @@ import * as ts from "typescript"; +import {isBlockScopeBoundary} from "../utils"; import {ScopeAwareRuleWalker} from "./scopeAwareRuleWalker"; /** @@ -30,7 +31,7 @@ export abstract class BlockScopeAwareRuleWalker extends ScopeAwareRuleWalk super(sourceFile, options); // initialize with global scope if file is not a module - this.blockScopeStack = this.fileIsModule ? [] : [this.createBlockScope(sourceFile)]; + this.blockScopeStack = ts.isExternalModule(sourceFile) ? [] : [this.createBlockScope(sourceFile)]; } public abstract createBlockScope(node: ts.Node): U; @@ -75,20 +76,6 @@ export abstract class BlockScopeAwareRuleWalker extends ScopeAwareRuleWalk } private isBlockScopeBoundary(node: ts.Node): boolean { - return super.isScopeBoundary(node) - || node.kind === ts.SyntaxKind.Block - || node.kind === ts.SyntaxKind.DoStatement - || node.kind === ts.SyntaxKind.WhileStatement - || node.kind === ts.SyntaxKind.ForStatement - || node.kind === ts.SyntaxKind.ForInStatement - || node.kind === ts.SyntaxKind.ForOfStatement - || node.kind === ts.SyntaxKind.WithStatement - || node.kind === ts.SyntaxKind.SwitchStatement - || isParentKind(node, ts.SyntaxKind.TryStatement) - || isParentKind(node, ts.SyntaxKind.IfStatement); + return isBlockScopeBoundary(node); } } - -function isParentKind(node: ts.Node, kind: ts.SyntaxKind) { - return node.parent != null && node.parent.kind === kind; -} diff --git a/src/language/walker/scopeAwareRuleWalker.ts b/src/language/walker/scopeAwareRuleWalker.ts index aa01d4af19e..00b20e224ae 100644 --- a/src/language/walker/scopeAwareRuleWalker.ts +++ b/src/language/walker/scopeAwareRuleWalker.ts @@ -17,19 +17,17 @@ import * as ts from "typescript"; +import {isScopeBoundary} from "../utils"; import {RuleWalker} from "./ruleWalker"; export abstract class ScopeAwareRuleWalker extends RuleWalker { - protected fileIsModule: boolean; private scopeStack: T[]; constructor(sourceFile: ts.SourceFile, options?: any) { super(sourceFile, options); - this.fileIsModule = ts.isExternalModule(sourceFile); - // initialize with global scope if file is not a module - this.scopeStack = this.fileIsModule ? [] : [this.createScope(sourceFile)]; + this.scopeStack = ts.isExternalModule(sourceFile) ? [] : [this.createScope(sourceFile)]; } public abstract createScope(node: ts.Node): T; @@ -74,20 +72,6 @@ export abstract class ScopeAwareRuleWalker extends RuleWalker { } protected isScopeBoundary(node: ts.Node): boolean { - return node.kind === ts.SyntaxKind.FunctionDeclaration - || node.kind === ts.SyntaxKind.FunctionExpression - || node.kind === ts.SyntaxKind.PropertyAssignment - || node.kind === ts.SyntaxKind.ShorthandPropertyAssignment - || node.kind === ts.SyntaxKind.MethodDeclaration - || node.kind === ts.SyntaxKind.Constructor - || node.kind === ts.SyntaxKind.ModuleDeclaration - || node.kind === ts.SyntaxKind.ArrowFunction - || node.kind === ts.SyntaxKind.ParenthesizedExpression - || node.kind === ts.SyntaxKind.ClassDeclaration - || node.kind === ts.SyntaxKind.ClassExpression - || node.kind === ts.SyntaxKind.InterfaceDeclaration - || node.kind === ts.SyntaxKind.GetAccessor - || node.kind === ts.SyntaxKind.SetAccessor - || (node.kind === ts.SyntaxKind.SourceFile && this.fileIsModule); + return isScopeBoundary(node); } } From 4e29d1b3e4d300b8194a95f1560e99a5b36cc0a7 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 22 Dec 2016 20:09:52 +0100 Subject: [PATCH 02/10] Move filtering of failures from RuleWalker to Rule AbstractRule now deduplicates and filters failures in disabled intervals --- src/language/rule/abstractRule.ts | 16 ++++++++++++++-- src/language/walker/ruleWalker.ts | 14 ++------------ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/language/rule/abstractRule.ts b/src/language/rule/abstractRule.ts index 3d7f7aed9ec..20e9e17f01e 100644 --- a/src/language/rule/abstractRule.ts +++ b/src/language/rule/abstractRule.ts @@ -17,6 +17,7 @@ import * as ts from "typescript"; +import {doesIntersect} from "../utils"; import {RuleWalker} from "../walker/ruleWalker"; import {IDisabledInterval, IOptions, IRule, IRuleMetadata, RuleFailure} from "./rule"; @@ -36,7 +37,7 @@ export abstract class AbstractRule implements IRule { return false; } - constructor(ruleName: string, private value: any, disabledIntervals: IDisabledInterval[]) { + constructor(ruleName: string, private value: any, private disabledIntervals: IDisabledInterval[]) { let ruleArguments: any[] = []; if (Array.isArray(value) && value.length > 1) { @@ -58,10 +59,21 @@ export abstract class AbstractRule implements IRule { public applyWithWalker(walker: RuleWalker): RuleFailure[] { walker.walk(walker.getSourceFile()); - return walker.getFailures(); + return this.filterFailures(walker.getFailures()); } public isEnabled(): boolean { return AbstractRule.isRuleEnabled(this.value); } + + protected filterFailures(failures: RuleFailure[]): RuleFailure[] { + const result: RuleFailure[] = []; + for (const failure of failures) { + // don't add failures for a rule if the failure intersects an interval where that rule is disabled + if (!doesIntersect(failure, this.disabledIntervals) && !result.some((f) => f.equals(failure))) { + result.push(failure); + } + } + return result; + } } diff --git a/src/language/walker/ruleWalker.ts b/src/language/walker/ruleWalker.ts index 775a3ce3dfd..58a1726da08 100644 --- a/src/language/walker/ruleWalker.ts +++ b/src/language/walker/ruleWalker.ts @@ -17,8 +17,7 @@ import * as ts from "typescript"; -import {Fix, IDisabledInterval, IOptions, Replacement, RuleFailure} from "../rule/rule"; -import {doesIntersect} from "../utils"; +import {Fix, IOptions, Replacement, RuleFailure} from "../rule/rule"; import {SyntaxWalker} from "./syntaxWalker"; export class RuleWalker extends SyntaxWalker { @@ -26,7 +25,6 @@ export class RuleWalker extends SyntaxWalker { private position: number; private options: any[]; private failures: RuleFailure[]; - private disabledIntervals: IDisabledInterval[]; private ruleName: string; constructor(private sourceFile: ts.SourceFile, options: IOptions) { @@ -36,7 +34,6 @@ export class RuleWalker extends SyntaxWalker { this.failures = []; this.options = options.ruleArguments; this.limit = this.sourceFile.getFullWidth(); - this.disabledIntervals = options.disabledIntervals; this.ruleName = options.ruleName; } @@ -79,10 +76,7 @@ export class RuleWalker extends SyntaxWalker { } public addFailure(failure: RuleFailure) { - // don't add failures for a rule if the failure intersects an interval where that rule is disabled - if (!this.existsFailure(failure) && !doesIntersect(failure, this.disabledIntervals)) { - this.failures.push(failure); - } + this.failures.push(failure); } /** Add a failure with any arbitrary span. Prefer `addFailureAtNode` if possible. */ @@ -111,8 +105,4 @@ export class RuleWalker extends SyntaxWalker { public deleteText(start: number, length: number): Replacement { return this.createReplacement(start, length, ""); } - - private existsFailure(failure: RuleFailure) { - return this.failures.some((f) => f.equals(failure)); - } } From d27e78ee340425955eb45134af5f8d8fc0bbe63c Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 22 Dec 2016 20:11:07 +0100 Subject: [PATCH 03/10] Remove unnecessary skip method and position --- src/language/walker/ruleWalker.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/language/walker/ruleWalker.ts b/src/language/walker/ruleWalker.ts index 58a1726da08..f4b03268174 100644 --- a/src/language/walker/ruleWalker.ts +++ b/src/language/walker/ruleWalker.ts @@ -22,7 +22,6 @@ import {SyntaxWalker} from "./syntaxWalker"; export class RuleWalker extends SyntaxWalker { private limit: number; - private position: number; private options: any[]; private failures: RuleFailure[]; private ruleName: string; @@ -30,7 +29,6 @@ export class RuleWalker extends SyntaxWalker { constructor(private sourceFile: ts.SourceFile, options: IOptions) { super(); - this.position = 0; this.failures = []; this.options = options.ruleArguments; this.limit = this.sourceFile.getFullWidth(); @@ -65,10 +63,6 @@ export class RuleWalker extends SyntaxWalker { } } - public skip(node: ts.Node) { - this.position += node.getFullWidth(); - } - public createFailure(start: number, width: number, failure: string, fix?: Fix): RuleFailure { const from = (start > this.limit) ? this.limit : start; const to = ((start + width) > this.limit) ? this.limit : (start + width); From ef652cb2a5eab2042019a6325ec832cd20a13e3f Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 22 Dec 2016 20:12:27 +0100 Subject: [PATCH 04/10] Don't create a new array in getAllScopes --- src/language/walker/scopeAwareRuleWalker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/language/walker/scopeAwareRuleWalker.ts b/src/language/walker/scopeAwareRuleWalker.ts index 00b20e224ae..d7ab2d002d3 100644 --- a/src/language/walker/scopeAwareRuleWalker.ts +++ b/src/language/walker/scopeAwareRuleWalker.ts @@ -38,7 +38,7 @@ export abstract class ScopeAwareRuleWalker extends RuleWalker { // get all scopes available at this depth public getAllScopes(): T[] { - return this.scopeStack.slice(); + return this.scopeStack; } public getCurrentDepth(): number { From 38335e5502f4def193183deafbf8ac51d3dcf882 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 22 Dec 2016 20:14:42 +0100 Subject: [PATCH 05/10] Add helper method to easily create a Fix --- src/language/walker/ruleWalker.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/language/walker/ruleWalker.ts b/src/language/walker/ruleWalker.ts index f4b03268174..61d62704eb1 100644 --- a/src/language/walker/ruleWalker.ts +++ b/src/language/walker/ruleWalker.ts @@ -99,4 +99,12 @@ export class RuleWalker extends SyntaxWalker { public deleteText(start: number, length: number): Replacement { return this.createReplacement(start, length, ""); } + + public getRuleName(): string { + return this.ruleName; + } + + public createFix(replacements: Replacement[]): Fix { + return new Fix(this.ruleName, replacements); + } } From c56fbdd3110b5c7c13e0a2a1f0e84e5d19671737 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 22 Dec 2016 20:58:45 +0100 Subject: [PATCH 06/10] Pass SourceFile to getStart and getWidth --- src/language/walker/ruleWalker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/language/walker/ruleWalker.ts b/src/language/walker/ruleWalker.ts index 61d62704eb1..c2262c57db2 100644 --- a/src/language/walker/ruleWalker.ts +++ b/src/language/walker/ruleWalker.ts @@ -85,7 +85,7 @@ export class RuleWalker extends SyntaxWalker { /** Add a failure using a node's span. */ public addFailureAtNode(node: ts.Node, failure: string, fix?: Fix) { - this.addFailureAt(node.getStart(), node.getWidth(), failure, fix); + this.addFailureAt(node.getStart(this.sourceFile), node.getWidth(this.sourceFile), failure, fix); } public createReplacement(start: number, length: number, text: string): Replacement { From 16acad50de49143749750498ae3af4edaa505d57 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 22 Dec 2016 20:56:18 +0100 Subject: [PATCH 07/10] Add IWalker interface --- src/language/rule/abstractRule.ts | 4 ++-- src/language/rule/rule.ts | 4 ++-- src/language/walker/index.ts | 1 + src/language/walker/ruleWalker.ts | 3 ++- src/language/walker/walker.ts | 26 ++++++++++++++++++++++++++ 5 files changed, 33 insertions(+), 5 deletions(-) create mode 100644 src/language/walker/walker.ts diff --git a/src/language/rule/abstractRule.ts b/src/language/rule/abstractRule.ts index 20e9e17f01e..7dc381b03bb 100644 --- a/src/language/rule/abstractRule.ts +++ b/src/language/rule/abstractRule.ts @@ -18,7 +18,7 @@ import * as ts from "typescript"; import {doesIntersect} from "../utils"; -import {RuleWalker} from "../walker/ruleWalker"; +import {IWalker} from "../walker/walker"; import {IDisabledInterval, IOptions, IRule, IRuleMetadata, RuleFailure} from "./rule"; export abstract class AbstractRule implements IRule { @@ -57,7 +57,7 @@ export abstract class AbstractRule implements IRule { public abstract apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[]; - public applyWithWalker(walker: RuleWalker): RuleFailure[] { + public applyWithWalker(walker: IWalker): RuleFailure[] { walker.walk(walker.getSourceFile()); return this.filterFailures(walker.getFailures()); } diff --git a/src/language/rule/rule.ts b/src/language/rule/rule.ts index f4df1a2ab69..29762cb9ba2 100644 --- a/src/language/rule/rule.ts +++ b/src/language/rule/rule.ts @@ -17,7 +17,7 @@ import * as ts from "typescript"; -import {RuleWalker} from "../walker/ruleWalker"; +import {IWalker} from "../walker/walker"; export interface IRuleMetadata { /** @@ -96,7 +96,7 @@ export interface IRule { getOptions(): IOptions; isEnabled(): boolean; apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[]; - applyWithWalker(walker: RuleWalker): RuleFailure[]; + applyWithWalker(walker: IWalker): RuleFailure[]; } export class Replacement { diff --git a/src/language/walker/index.ts b/src/language/walker/index.ts index b762aa289ed..bf1ab5df58b 100644 --- a/src/language/walker/index.ts +++ b/src/language/walker/index.ts @@ -21,3 +21,4 @@ export * from "./ruleWalker"; export * from "./scopeAwareRuleWalker"; export * from "./skippableTokenAwareRuleWalker"; export * from "./syntaxWalker"; +export * from "./walker"; diff --git a/src/language/walker/ruleWalker.ts b/src/language/walker/ruleWalker.ts index c2262c57db2..21578b6e7a1 100644 --- a/src/language/walker/ruleWalker.ts +++ b/src/language/walker/ruleWalker.ts @@ -19,8 +19,9 @@ import * as ts from "typescript"; import {Fix, IOptions, Replacement, RuleFailure} from "../rule/rule"; import {SyntaxWalker} from "./syntaxWalker"; +import {IWalker} from "./walker"; -export class RuleWalker extends SyntaxWalker { +export class RuleWalker extends SyntaxWalker implements IWalker { private limit: number; private options: any[]; private failures: RuleFailure[]; diff --git a/src/language/walker/walker.ts b/src/language/walker/walker.ts new file mode 100644 index 00000000000..d72fc485f1c --- /dev/null +++ b/src/language/walker/walker.ts @@ -0,0 +1,26 @@ +/** + * @license + * Copyright 2013 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 {RuleFailure} from "../rule/rule"; + +export interface IWalker { + getSourceFile(): ts.SourceFile; + walk(sourceFile: ts.SourceFile): void; + getFailures(): RuleFailure[]; +} From 7b7d62006743b99fd88418bff5841bb18789f8cd Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Sun, 1 Jan 2017 15:14:31 +0100 Subject: [PATCH 08/10] Address comments --- src/language/rule/abstractRule.ts | 2 +- src/language/rule/rule.ts | 2 +- src/language/walker/walker.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/language/rule/abstractRule.ts b/src/language/rule/abstractRule.ts index 7dc381b03bb..242562560d3 100644 --- a/src/language/rule/abstractRule.ts +++ b/src/language/rule/abstractRule.ts @@ -18,7 +18,7 @@ import * as ts from "typescript"; import {doesIntersect} from "../utils"; -import {IWalker} from "../walker/walker"; +import {IWalker} from "../walker"; import {IDisabledInterval, IOptions, IRule, IRuleMetadata, RuleFailure} from "./rule"; export abstract class AbstractRule implements IRule { diff --git a/src/language/rule/rule.ts b/src/language/rule/rule.ts index 29762cb9ba2..2f961556695 100644 --- a/src/language/rule/rule.ts +++ b/src/language/rule/rule.ts @@ -17,7 +17,7 @@ import * as ts from "typescript"; -import {IWalker} from "../walker/walker"; +import {IWalker} from "../walker"; export interface IRuleMetadata { /** diff --git a/src/language/walker/walker.ts b/src/language/walker/walker.ts index d72fc485f1c..5db93adfd47 100644 --- a/src/language/walker/walker.ts +++ b/src/language/walker/walker.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2013 Palantir Technologies, Inc. + * Copyright 2017 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. From 913e7e66ec03059ce60623a72d66b60593c0e378 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Sun, 1 Jan 2017 15:17:49 +0100 Subject: [PATCH 09/10] Readd skip method --- src/language/walker/ruleWalker.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/language/walker/ruleWalker.ts b/src/language/walker/ruleWalker.ts index 21578b6e7a1..2a63d342115 100644 --- a/src/language/walker/ruleWalker.ts +++ b/src/language/walker/ruleWalker.ts @@ -64,6 +64,10 @@ export class RuleWalker extends SyntaxWalker implements IWalker { } } + public skip(_node: ts.Node) { + return; // TODO remove this method in next major version + } + public createFailure(start: number, width: number, failure: string, fix?: Fix): RuleFailure { const from = (start > this.limit) ? this.limit : start; const to = ((start + width) > this.limit) ? this.limit : (start + width); From aca5ce3738e0331977286b48b15dfdfed3501e76 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Sun, 1 Jan 2017 15:30:55 +0100 Subject: [PATCH 10/10] Fix parameter type of walk method --- src/language/walker/walker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/language/walker/walker.ts b/src/language/walker/walker.ts index 5db93adfd47..3c07585e418 100644 --- a/src/language/walker/walker.ts +++ b/src/language/walker/walker.ts @@ -21,6 +21,6 @@ import {RuleFailure} from "../rule/rule"; export interface IWalker { getSourceFile(): ts.SourceFile; - walk(sourceFile: ts.SourceFile): void; + walk(node: ts.Node): void; getFailures(): RuleFailure[]; }