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

Refactor walker #1923

Merged
merged 11 commits into from
Jan 1, 2017
20 changes: 16 additions & 4 deletions src/language/rule/abstractRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@

import * as ts from "typescript";

import {RuleWalker} from "../walker/ruleWalker";
import {doesIntersect} from "../utils";
import {IWalker} from "../walker/walker";
import {IDisabledInterval, IOptions, IRule, IRuleMetadata, RuleFailure} from "./rule";

export abstract class AbstractRule implements IRule {
Expand All @@ -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) {
Expand All @@ -56,12 +57,23 @@ 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 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;
}
}
4 changes: 2 additions & 2 deletions src/language/rule/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import * as ts from "typescript";

import {RuleWalker} from "../walker/ruleWalker";
import {IWalker} from "../walker/walker";

export interface IRuleMetadata {
/**
Expand Down Expand Up @@ -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 {
Expand Down
33 changes: 33 additions & 0 deletions src/language/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
19 changes: 3 additions & 16 deletions src/language/walker/blockScopeAwareRuleWalker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import * as ts from "typescript";

import {isBlockScopeBoundary} from "../utils";
import {ScopeAwareRuleWalker} from "./scopeAwareRuleWalker";

/**
Expand All @@ -30,7 +31,7 @@ export abstract class BlockScopeAwareRuleWalker<T, U> 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;
Expand Down Expand Up @@ -75,20 +76,6 @@ export abstract class BlockScopeAwareRuleWalker<T, U> 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;
}
1 change: 1 addition & 0 deletions src/language/walker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ export * from "./ruleWalker";
export * from "./scopeAwareRuleWalker";
export * from "./skippableTokenAwareRuleWalker";
export * from "./syntaxWalker";
export * from "./walker";
29 changes: 11 additions & 18 deletions src/language/walker/ruleWalker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,22 @@

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";
import {IWalker} from "./walker";

export class RuleWalker extends SyntaxWalker {
export class RuleWalker extends SyntaxWalker implements IWalker {
private limit: number;
private position: number;
private options: any[];
private failures: RuleFailure[];
private disabledIntervals: IDisabledInterval[];
private ruleName: string;

constructor(private sourceFile: ts.SourceFile, options: IOptions) {
super();

this.position = 0;
this.failures = [];
this.options = options.ruleArguments;
this.limit = this.sourceFile.getFullWidth();
this.disabledIntervals = options.disabledIntervals;
this.ruleName = options.ruleName;
}

Expand Down Expand Up @@ -68,21 +64,14 @@ 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);
return new RuleFailure(this.sourceFile, from, to, failure, this.ruleName, fix);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really hope nobody is using this. It looks like it implemented with a bunch of other stuff and never used

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I readded the method.
I will remove this in the followup PR with breaking changes


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. */
Expand All @@ -97,7 +86,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 {
Expand All @@ -112,7 +101,11 @@ export class RuleWalker extends SyntaxWalker {
return this.createReplacement(start, length, "");
}

private existsFailure(failure: RuleFailure) {
return this.failures.some((f) => f.equals(failure));
public getRuleName(): string {
return this.ruleName;
}

public createFix(replacements: Replacement[]): Fix {
return new Fix(this.ruleName, replacements);
}
}
24 changes: 4 additions & 20 deletions src/language/walker/scopeAwareRuleWalker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,17 @@

import * as ts from "typescript";

import {isScopeBoundary} from "../utils";
import {RuleWalker} from "./ruleWalker";

export abstract class ScopeAwareRuleWalker<T> 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;
Expand All @@ -40,7 +38,7 @@ export abstract class ScopeAwareRuleWalker<T> extends RuleWalker {

// get all scopes available at this depth
public getAllScopes(): T[] {
return this.scopeStack.slice();
return this.scopeStack;
}

public getCurrentDepth(): number {
Expand Down Expand Up @@ -74,20 +72,6 @@ export abstract class ScopeAwareRuleWalker<T> 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);
}
}
26 changes: 26 additions & 0 deletions src/language/walker/walker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* @license
* Copyright 2013 Palantir Technologies, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2016

*
* 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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we put this in walker/index.ts instead? the import from /.../walker/walker doesn't look as nice as /.../walker

getSourceFile(): ts.SourceFile;
walk(sourceFile: ts.SourceFile): void;
getFailures(): RuleFailure[];
}