From da3ac7d4ee879ff05d95946ef055ddcfcdfb28f9 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 19 Jan 2017 20:08:04 +0100 Subject: [PATCH 1/8] Implement AbstractWalker --- src/language/rule/rule.ts | 16 +++++++++++ src/language/walker/walker.ts | 53 +++++++++++++++++++++++++++++++++-- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/src/language/rule/rule.ts b/src/language/rule/rule.ts index f1a3035b71b..d57108ead6b 100644 --- a/src/language/rule/rule.ts +++ b/src/language/rule/rule.ts @@ -111,6 +111,22 @@ export class Replacement { return replacements.reduce((text, r) => r.apply(text), content); } + public static replaceFromTo(start: number, end: number, text: string) { + return new Replacement(start, end - start, text); + } + + public static deleteText(start: number, length: number) { + return new Replacement(start, length, ""); + } + + public static deleteFromTo(start: number, end: number) { + return new Replacement(start, end - start, ""); + } + + public static appendText(start: number, text: string) { + return new Replacement(start, 0, text); + } + constructor(private innerStart: number, private innerLength: number, private innerText: string) { } diff --git a/src/language/walker/walker.ts b/src/language/walker/walker.ts index 3c07585e418..455ab4e824f 100644 --- a/src/language/walker/walker.ts +++ b/src/language/walker/walker.ts @@ -17,10 +17,59 @@ import * as ts from "typescript"; -import {RuleFailure} from "../rule/rule"; +import {Fix, IOptions, Replacement, RuleFailure} from "../rule/rule"; +import {IWalker} from "./walker"; export interface IWalker { getSourceFile(): ts.SourceFile; - walk(node: ts.Node): void; + walk(sourceFile: ts.SourceFile): void; getFailures(): RuleFailure[]; } + +export abstract class AbstractWalker implements IWalker { + public readonly options: ReadonlyArray; + public readonly ruleName: string; + private limit: number; + private failures: RuleFailure[]; + + constructor(public readonly sourceFile: ts.SourceFile, options: IOptions) { + this.failures = []; + this.options = options.ruleArguments; + this.limit = sourceFile.getFullWidth(); + this.ruleName = options.ruleName; + } + + public abstract walk(sourceFile: ts.SourceFile): void; + + public getSourceFile() { + return this.sourceFile; + } + + public getFailures() { + return this.failures; + } + + public hasOption(option: any) { + return this.options.indexOf(option) !== -1; + } + + /** Add a failure with any arbitrary span. Prefer `addFailureAtNode` if possible. */ + public addFailureAt(start: number, width: number, failure: string, fix?: Fix) { + this.addFailure(start, start + width, failure, fix); + } + + public addFailure(start: number, end: number, failure: string, fix?: Fix) { + this.failures.push( + new RuleFailure(this.sourceFile, Math.min(start, this.limit), Math.min(end, this.limit), failure, this.ruleName, fix), + ); + } + + /** Add a failure using a node's span. */ + public addFailureAtNode(node: ts.Node, failure: string, fix?: Fix) { + this.addFailure(node.getStart(this.sourceFile), node.getEnd(), failure, fix); + } + + public createFix(...replacements: Replacement[]) { + return new Fix(this.ruleName, replacements); + } +} From 252599dc04cfad0454cddbcfba8981c714e3a121 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Mon, 23 Jan 2017 17:33:26 +0100 Subject: [PATCH 2/8] Implement WalkContext and Rule.applyWithFunction --- src/language/rule/abstractRule.ts | 10 +++++-- src/language/rule/rule.ts | 37 +++++++++++++++++++++++--- src/language/walker/walker.ts | 44 ++----------------------------- 3 files changed, 44 insertions(+), 47 deletions(-) diff --git a/src/language/rule/abstractRule.ts b/src/language/rule/abstractRule.ts index 242562560d3..822f4404f8d 100644 --- a/src/language/rule/abstractRule.ts +++ b/src/language/rule/abstractRule.ts @@ -19,7 +19,7 @@ import * as ts from "typescript"; import {doesIntersect} from "../utils"; import {IWalker} from "../walker"; -import {IDisabledInterval, IOptions, IRule, IRuleMetadata, RuleFailure} from "./rule"; +import {IDisabledInterval, IOptions, IRule, IRuleMetadata, RuleFailure, WalkContext} from "./rule"; export abstract class AbstractRule implements IRule { public static metadata: IRuleMetadata; @@ -57,11 +57,17 @@ export abstract class AbstractRule implements IRule { public abstract apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[]; - public applyWithWalker(walker: IWalker): RuleFailure[] { + protected applyWithWalker(walker: IWalker): RuleFailure[] { walker.walk(walker.getSourceFile()); return this.filterFailures(walker.getFailures()); } + protected applyWithFunction(sourceFile: ts.SourceFile, options: T, walkFn: (ctx: WalkContext) => void) { + const ctx = new WalkContext(sourceFile, this.options.ruleName, options); + walkFn(ctx); + return this.filterFailures(ctx.getFailures()); + } + public isEnabled(): boolean { return AbstractRule.isRuleEnabled(this.value); } diff --git a/src/language/rule/rule.ts b/src/language/rule/rule.ts index d57108ead6b..0b76c21e49a 100644 --- a/src/language/rule/rule.ts +++ b/src/language/rule/rule.ts @@ -17,8 +17,6 @@ import * as ts from "typescript"; -import {IWalker} from "../walker"; - export interface IRuleMetadata { /** * The kebab-case name of the rule. @@ -101,7 +99,6 @@ export interface IRule { getOptions(): IOptions; isEnabled(): boolean; apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[]; - applyWithWalker(walker: IWalker): RuleFailure[]; } export class Replacement { @@ -281,3 +278,37 @@ export class RuleFailure { return new RuleFailurePosition(position, lineAndCharacter); } } + +export class WalkContext { + private limit: number; + private failures: RuleFailure[]; + + constructor(public readonly sourceFile: ts.SourceFile, public readonly ruleName: string, public readonly options: T) { + this.failures = []; + this.limit = sourceFile.getFullWidth(); + } + + public getFailures() { + return this.failures; + } + + /** Add a failure with any arbitrary span. Prefer `addFailureAtNode` if possible. */ + public addFailureAt(start: number, width: number, failure: string, fix?: Fix) { + this.addFailure(start, start + width, failure, fix); + } + + public addFailure(start: number, end: number, failure: string, fix?: Fix) { + this.failures.push( + new RuleFailure(this.sourceFile, Math.min(start, this.limit), Math.min(end, this.limit), failure, this.ruleName, fix), + ); + } + + /** Add a failure using a node's span. */ + public addFailureAtNode(node: ts.Node, failure: string, fix?: Fix) { + this.addFailure(node.getStart(this.sourceFile), node.getEnd(), failure, fix); + } + + public createFix(...replacements: Replacement[]) { + return new Fix(this.ruleName, replacements); + } +} diff --git a/src/language/walker/walker.ts b/src/language/walker/walker.ts index 455ab4e824f..85363c0f6b6 100644 --- a/src/language/walker/walker.ts +++ b/src/language/walker/walker.ts @@ -17,7 +17,7 @@ import * as ts from "typescript"; -import {Fix, IOptions, Replacement, RuleFailure} from "../rule/rule"; +import {RuleFailure, WalkContext} from "../rule/rule"; import {IWalker} from "./walker"; export interface IWalker { @@ -26,50 +26,10 @@ export interface IWalker { getFailures(): RuleFailure[]; } -export abstract class AbstractWalker implements IWalker { - public readonly options: ReadonlyArray; - public readonly ruleName: string; - private limit: number; - private failures: RuleFailure[]; - - constructor(public readonly sourceFile: ts.SourceFile, options: IOptions) { - this.failures = []; - this.options = options.ruleArguments; - this.limit = sourceFile.getFullWidth(); - this.ruleName = options.ruleName; - } - +export abstract class AbstractWalker extends WalkContext implements IWalker { public abstract walk(sourceFile: ts.SourceFile): void; public getSourceFile() { return this.sourceFile; } - - public getFailures() { - return this.failures; - } - - public hasOption(option: any) { - return this.options.indexOf(option) !== -1; - } - - /** Add a failure with any arbitrary span. Prefer `addFailureAtNode` if possible. */ - public addFailureAt(start: number, width: number, failure: string, fix?: Fix) { - this.addFailure(start, start + width, failure, fix); - } - - public addFailure(start: number, end: number, failure: string, fix?: Fix) { - this.failures.push( - new RuleFailure(this.sourceFile, Math.min(start, this.limit), Math.min(end, this.limit), failure, this.ruleName, fix), - ); - } - - /** Add a failure using a node's span. */ - public addFailureAtNode(node: ts.Node, failure: string, fix?: Fix) { - this.addFailure(node.getStart(this.sourceFile), node.getEnd(), failure, fix); - } - - public createFix(...replacements: Replacement[]) { - return new Fix(this.ruleName, replacements); - } } From b8ffd67c275478c7b0ce1ba856c898d98b0ec6c4 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Mon, 23 Jan 2017 17:34:07 +0100 Subject: [PATCH 3/8] Refactor no-null-keyword to walk function --- src/rules/noNullKeywordRule.ts | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/rules/noNullKeywordRule.ts b/src/rules/noNullKeywordRule.ts index 44f66987331..4841502a1c3 100644 --- a/src/rules/noNullKeywordRule.ts +++ b/src/rules/noNullKeywordRule.ts @@ -40,25 +40,19 @@ export class Rule extends Lint.Rules.AbstractRule { public static FAILURE_STRING = "Use 'undefined' instead of 'null'"; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new NullWalker(sourceFile, this.getOptions())); + return this.applyWithFunction(sourceFile, undefined, walk); } } -class NullWalker extends Lint.RuleWalker { - public visitNode(node: ts.Node) { - super.visitNode(node); - if (node.kind === ts.SyntaxKind.NullKeyword && !isPartOfType(node)) { - this.addFailureAtNode(node, Rule.FAILURE_STRING); +function walk(ctx: Lint.WalkContext) { + return ts.forEachChild(ctx.sourceFile, cb); + function cb(node: ts.Node): void { + if (node.kind >= ts.SyntaxKind.FirstTypeNode && node.kind <= ts.SyntaxKind.LastTypeNode) { + return; // skip type nodes } - } -} - -function isPartOfType({ parent }: ts.Node) { - while (parent != null) { - if (ts.SyntaxKind.FirstTypeNode <= parent.kind && parent.kind <= ts.SyntaxKind.LastTypeNode) { - return true; + if (node.kind === ts.SyntaxKind.NullKeyword) { + return ctx.addFailureAtNode(node, Rule.FAILURE_STRING); } - parent = parent.parent; + return ts.forEachChild(node, cb); } - return false; } From fbe808e355f1dbc5977d2cf8866f5b4929b17174 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Mon, 23 Jan 2017 17:53:53 +0100 Subject: [PATCH 4/8] Refactor no-magic-numbers to AbstractWalker --- src/rules/noMagicNumbersRule.ts | 55 ++++++++++++--------------------- 1 file changed, 20 insertions(+), 35 deletions(-) diff --git a/src/rules/noMagicNumbersRule.ts b/src/rules/noMagicNumbersRule.ts index bfc37e2fad5..66d1d7803c8 100644 --- a/src/rules/noMagicNumbersRule.ts +++ b/src/rules/noMagicNumbersRule.ts @@ -19,8 +19,6 @@ import * as ts from "typescript"; import * as Lint from "../index"; -import { IOptions } from "../language/rule/rule"; - export class Rule extends Lint.Rules.AbstractRule { /* tslint:disable:object-literal-sort-keys */ public static metadata: Lint.IRuleMetadata = { @@ -63,44 +61,31 @@ export class Rule extends Lint.Rules.AbstractRule { public static DEFAULT_ALLOWED = [ -1, 0, 1 ]; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new NoMagicNumbersWalker(sourceFile, this.getOptions())); + const options = this.getOptions(); + const ruleArguments = options.ruleArguments; + const allowedNumbers = ruleArguments.length > 0 ? ruleArguments : Rule.DEFAULT_ALLOWED; + return this.applyWithWalker(new NoMagicNumbersWalker(sourceFile, options.ruleName, new Set(allowedNumbers.map(String)))); } } -class NoMagicNumbersWalker extends Lint.RuleWalker { - // allowed magic numbers - private allowed: Set; - constructor(sourceFile: ts.SourceFile, options: IOptions) { - super(sourceFile, options); - - const configOptions = this.getOptions(); - const allowedNumbers: number[] = configOptions.length > 0 ? configOptions : Rule.DEFAULT_ALLOWED; - this.allowed = new Set(allowedNumbers.map(String)); - } - - public visitNode(node: ts.Node) { - const num = getLiteralNumber(node); - if (num !== undefined) { - if (!Rule.ALLOWED_NODES.has(node.parent!.kind) && !this.allowed.has(num)) { - this.addFailureAtNode(node, Rule.FAILURE_STRING); +class NoMagicNumbersWalker extends Lint.AbstractWalker> { + public walk(sourceFile: ts.SourceFile) { + const cb = (node: ts.Node): void => { + if (node.kind === ts.SyntaxKind.NumericLiteral) { + this.checkNumericLiteral(node, (node as ts.NumericLiteral).text); + } else if (node.kind === ts.SyntaxKind.PrefixUnaryExpression && + (node as ts.PrefixUnaryExpression).operator === ts.SyntaxKind.MinusToken) { + this.checkNumericLiteral(node, "-" + ((node as ts.PrefixUnaryExpression).operand as ts.NumericLiteral).text); + } else { + ts.forEachChild(node, cb); } - } else { - super.visitNode(node); - } + }; + return ts.forEachChild(sourceFile, cb); } -} -/** If node is a number literal, return a string representation of that number. */ -function getLiteralNumber(node: ts.Node): string | undefined { - if (node.kind === ts.SyntaxKind.NumericLiteral) { - return (node as ts.NumericLiteral).text; - } - if (node.kind !== ts.SyntaxKind.PrefixUnaryExpression) { - return undefined; - } - const { operator, operand } = node as ts.PrefixUnaryExpression; - if (operator === ts.SyntaxKind.MinusToken && operand.kind === ts.SyntaxKind.NumericLiteral) { - return "-" + (operand as ts.NumericLiteral).text; + private checkNumericLiteral(node: ts.Node, num: string) { + if (!Rule.ALLOWED_NODES.has(node.parent!.kind) && !this.options.has(num)) { + this.addFailureAtNode(node, Rule.FAILURE_STRING); + } } - return undefined; } From eacd1a530bc2643c5d3a5d66784b8b2597223136 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Tue, 24 Jan 2017 19:20:19 +0100 Subject: [PATCH 5/8] PR feedback Also made some changes to simplify AbstractRule and added LanguageService to WalkContext --- src/language/rule/abstractRule.ts | 42 ++++++++++++++++++------------- src/language/rule/rule.ts | 11 ++++---- src/language/rule/typedRule.ts | 2 +- src/language/walker/walker.ts | 4 +++ src/rules/noMagicNumbersRule.ts | 6 ++--- src/rules/noNullKeywordRule.ts | 2 +- 6 files changed, 37 insertions(+), 30 deletions(-) diff --git a/src/language/rule/abstractRule.ts b/src/language/rule/abstractRule.ts index 822f4404f8d..c1300afb790 100644 --- a/src/language/rule/abstractRule.ts +++ b/src/language/rule/abstractRule.ts @@ -23,7 +23,7 @@ import {IDisabledInterval, IOptions, IRule, IRuleMetadata, RuleFailure, WalkCont export abstract class AbstractRule implements IRule { public static metadata: IRuleMetadata; - private options: IOptions; + protected readonly ruleArguments: any[]; public static isRuleEnabled(ruleConfigValue: any): boolean { if (typeof ruleConfigValue === "boolean") { @@ -37,22 +37,24 @@ export abstract class AbstractRule implements IRule { return false; } - constructor(ruleName: string, private value: any, private disabledIntervals: IDisabledInterval[]) { - let ruleArguments: any[] = []; - + constructor(protected readonly ruleName: string, private value: any, private disabledIntervals: IDisabledInterval[]) { if (Array.isArray(value) && value.length > 1) { - ruleArguments = value.slice(1); + this.ruleArguments = value.slice(1); + } else { + this.ruleArguments = []; } + } - this.options = { - disabledIntervals, - ruleArguments, - ruleName, + public getOptions(): IOptions { + return { + disabledIntervals: this.disabledIntervals, + ruleArguments: this.ruleArguments, + ruleName: this.ruleName, }; } - public getOptions(): IOptions { - return this.options; + public isEnabled(): boolean { + return AbstractRule.isRuleEnabled(this.value); } public abstract apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[]; @@ -62,14 +64,18 @@ export abstract class AbstractRule implements IRule { return this.filterFailures(walker.getFailures()); } - protected applyWithFunction(sourceFile: ts.SourceFile, options: T, walkFn: (ctx: WalkContext) => void) { - const ctx = new WalkContext(sourceFile, this.options.ruleName, options); + protected applyWithFunction(sourceFile: ts.SourceFile, walkFn: (ctx: WalkContext) => void): RuleFailure[]; + protected applyWithFunction(sourceFile: ts.SourceFile, + walkFn: (ctx: WalkContext) => void, + options: T, + languageService?: ts.LanguageService): RuleFailure[]; + protected applyWithFunction(sourceFile: ts.SourceFile, + walkFn: (ctx: WalkContext) => void, + options?: T, + languageService?: ts.LanguageService): RuleFailure[] { + const ctx = new WalkContext(sourceFile, this.ruleName, options, languageService); walkFn(ctx); - return this.filterFailures(ctx.getFailures()); - } - - public isEnabled(): boolean { - return AbstractRule.isRuleEnabled(this.value); + return this.filterFailures(ctx.failures); } protected filterFailures(failures: RuleFailure[]): RuleFailure[] { diff --git a/src/language/rule/rule.ts b/src/language/rule/rule.ts index 0b76c21e49a..9bacc3b0a9a 100644 --- a/src/language/rule/rule.ts +++ b/src/language/rule/rule.ts @@ -280,18 +280,17 @@ export class RuleFailure { } export class WalkContext { + public readonly failures: RuleFailure[]; private limit: number; - private failures: RuleFailure[]; - constructor(public readonly sourceFile: ts.SourceFile, public readonly ruleName: string, public readonly options: T) { + constructor(public readonly sourceFile: ts.SourceFile, + public readonly ruleName: string, + public readonly options: T, + public readonly languageService?: ts.LanguageService) { this.failures = []; this.limit = sourceFile.getFullWidth(); } - public getFailures() { - return this.failures; - } - /** Add a failure with any arbitrary span. Prefer `addFailureAtNode` if possible. */ public addFailureAt(start: number, width: number, failure: string, fix?: Fix) { this.addFailure(start, start + width, failure, fix); diff --git a/src/language/rule/typedRule.ts b/src/language/rule/typedRule.ts index 2e91c092bca..62cdaf9b516 100644 --- a/src/language/rule/typedRule.ts +++ b/src/language/rule/typedRule.ts @@ -28,7 +28,7 @@ export abstract class TypedRule extends AbstractRule { public apply(): RuleFailure[] { // if no program is given to the linter, throw an error - throw new Error(`${this.getOptions().ruleName} requires type checking`); + throw new Error(`${this.ruleName} requires type checking`); } public abstract applyWithProgram(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[]; diff --git a/src/language/walker/walker.ts b/src/language/walker/walker.ts index 85363c0f6b6..0da34d9c8a0 100644 --- a/src/language/walker/walker.ts +++ b/src/language/walker/walker.ts @@ -32,4 +32,8 @@ export abstract class AbstractWalker extends WalkContext implements IWalke public getSourceFile() { return this.sourceFile; } + + public getFailures() { + return this.failures; + } } diff --git a/src/rules/noMagicNumbersRule.ts b/src/rules/noMagicNumbersRule.ts index 66d1d7803c8..999fc3e81f5 100644 --- a/src/rules/noMagicNumbersRule.ts +++ b/src/rules/noMagicNumbersRule.ts @@ -61,10 +61,8 @@ export class Rule extends Lint.Rules.AbstractRule { public static DEFAULT_ALLOWED = [ -1, 0, 1 ]; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - const options = this.getOptions(); - const ruleArguments = options.ruleArguments; - const allowedNumbers = ruleArguments.length > 0 ? ruleArguments : Rule.DEFAULT_ALLOWED; - return this.applyWithWalker(new NoMagicNumbersWalker(sourceFile, options.ruleName, new Set(allowedNumbers.map(String)))); + const allowedNumbers = this.ruleArguments.length > 0 ? this.ruleArguments : Rule.DEFAULT_ALLOWED; + return this.applyWithWalker(new NoMagicNumbersWalker(sourceFile, this.ruleName, new Set(allowedNumbers.map(String)))); } } diff --git a/src/rules/noNullKeywordRule.ts b/src/rules/noNullKeywordRule.ts index 4841502a1c3..b3078f1efa8 100644 --- a/src/rules/noNullKeywordRule.ts +++ b/src/rules/noNullKeywordRule.ts @@ -40,7 +40,7 @@ export class Rule extends Lint.Rules.AbstractRule { public static FAILURE_STRING = "Use 'undefined' instead of 'null'"; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithFunction(sourceFile, undefined, walk); + return this.applyWithFunction(sourceFile, walk); } } From 0d7ae811427daeaf379ec30341fd97b6169e75ce Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Wed, 25 Jan 2017 16:36:30 +0100 Subject: [PATCH 6/8] Remove languageService property for now --- src/language/rule/abstractRule.ts | 12 +++--------- src/language/rule/rule.ts | 7 ++----- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/language/rule/abstractRule.ts b/src/language/rule/abstractRule.ts index c1300afb790..2a0e02e757a 100644 --- a/src/language/rule/abstractRule.ts +++ b/src/language/rule/abstractRule.ts @@ -65,15 +65,9 @@ export abstract class AbstractRule implements IRule { } protected applyWithFunction(sourceFile: ts.SourceFile, walkFn: (ctx: WalkContext) => void): RuleFailure[]; - protected applyWithFunction(sourceFile: ts.SourceFile, - walkFn: (ctx: WalkContext) => void, - options: T, - languageService?: ts.LanguageService): RuleFailure[]; - protected applyWithFunction(sourceFile: ts.SourceFile, - walkFn: (ctx: WalkContext) => void, - options?: T, - languageService?: ts.LanguageService): RuleFailure[] { - const ctx = new WalkContext(sourceFile, this.ruleName, options, languageService); + protected applyWithFunction(sourceFile: ts.SourceFile, walkFn: (ctx: WalkContext) => void, options: T): RuleFailure[]; + protected applyWithFunction(sourceFile: ts.SourceFile, walkFn: (ctx: WalkContext) => void, options?: T): RuleFailure[] { + const ctx = new WalkContext(sourceFile, this.ruleName, options); walkFn(ctx); return this.filterFailures(ctx.failures); } diff --git a/src/language/rule/rule.ts b/src/language/rule/rule.ts index 9bacc3b0a9a..281d3049619 100644 --- a/src/language/rule/rule.ts +++ b/src/language/rule/rule.ts @@ -283,12 +283,9 @@ export class WalkContext { public readonly failures: RuleFailure[]; private limit: number; - constructor(public readonly sourceFile: ts.SourceFile, - public readonly ruleName: string, - public readonly options: T, - public readonly languageService?: ts.LanguageService) { + constructor(public readonly sourceFile: ts.SourceFile, public readonly ruleName: string, public readonly options: T) { this.failures = []; - this.limit = sourceFile.getFullWidth(); + this.limit = sourceFile.end; } /** Add a failure with any arbitrary span. Prefer `addFailureAtNode` if possible. */ From 5ec78a4528f7a4558a1d7386b230b055315722f5 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Wed, 1 Feb 2017 18:47:04 +0100 Subject: [PATCH 7/8] Address review comments Reverted accidental breaking change. Move WalkContext to its own file Removed limit property and made it a local variable. --- src/language/rule/abstractRule.ts | 6 ++-- src/language/rule/rule.ts | 33 ++------------------- src/language/walker/index.ts | 1 + src/language/walker/walkContext.ts | 47 ++++++++++++++++++++++++++++++ src/language/walker/walker.ts | 3 +- 5 files changed, 56 insertions(+), 34 deletions(-) create mode 100644 src/language/walker/walkContext.ts diff --git a/src/language/rule/abstractRule.ts b/src/language/rule/abstractRule.ts index 2a0e02e757a..fb10a4e7a8d 100644 --- a/src/language/rule/abstractRule.ts +++ b/src/language/rule/abstractRule.ts @@ -18,8 +18,8 @@ import * as ts from "typescript"; import {doesIntersect} from "../utils"; -import {IWalker} from "../walker"; -import {IDisabledInterval, IOptions, IRule, IRuleMetadata, RuleFailure, WalkContext} from "./rule"; +import {IWalker, WalkContext} from "../walker"; +import {IDisabledInterval, IOptions, IRule, IRuleMetadata, RuleFailure} from "./rule"; export abstract class AbstractRule implements IRule { public static metadata: IRuleMetadata; @@ -59,7 +59,7 @@ export abstract class AbstractRule implements IRule { public abstract apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[]; - protected applyWithWalker(walker: IWalker): 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 281d3049619..d57108ead6b 100644 --- a/src/language/rule/rule.ts +++ b/src/language/rule/rule.ts @@ -17,6 +17,8 @@ import * as ts from "typescript"; +import {IWalker} from "../walker"; + export interface IRuleMetadata { /** * The kebab-case name of the rule. @@ -99,6 +101,7 @@ export interface IRule { getOptions(): IOptions; isEnabled(): boolean; apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[]; + applyWithWalker(walker: IWalker): RuleFailure[]; } export class Replacement { @@ -278,33 +281,3 @@ export class RuleFailure { return new RuleFailurePosition(position, lineAndCharacter); } } - -export class WalkContext { - public readonly failures: RuleFailure[]; - private limit: number; - - constructor(public readonly sourceFile: ts.SourceFile, public readonly ruleName: string, public readonly options: T) { - this.failures = []; - this.limit = sourceFile.end; - } - - /** Add a failure with any arbitrary span. Prefer `addFailureAtNode` if possible. */ - public addFailureAt(start: number, width: number, failure: string, fix?: Fix) { - this.addFailure(start, start + width, failure, fix); - } - - public addFailure(start: number, end: number, failure: string, fix?: Fix) { - this.failures.push( - new RuleFailure(this.sourceFile, Math.min(start, this.limit), Math.min(end, this.limit), failure, this.ruleName, fix), - ); - } - - /** Add a failure using a node's span. */ - public addFailureAtNode(node: ts.Node, failure: string, fix?: Fix) { - this.addFailure(node.getStart(this.sourceFile), node.getEnd(), failure, fix); - } - - public createFix(...replacements: Replacement[]) { - return new Fix(this.ruleName, replacements); - } -} diff --git a/src/language/walker/index.ts b/src/language/walker/index.ts index bf1ab5df58b..75e254fe056 100644 --- a/src/language/walker/index.ts +++ b/src/language/walker/index.ts @@ -21,4 +21,5 @@ export * from "./ruleWalker"; export * from "./scopeAwareRuleWalker"; export * from "./skippableTokenAwareRuleWalker"; export * from "./syntaxWalker"; +export * from "./walkContext"; export * from "./walker"; diff --git a/src/language/walker/walkContext.ts b/src/language/walker/walkContext.ts new file mode 100644 index 00000000000..3af20a74c4b --- /dev/null +++ b/src/language/walker/walkContext.ts @@ -0,0 +1,47 @@ +/** + * @license + * 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. + * 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 {Fix, Replacement, RuleFailure} from "../rule/rule"; + +export class WalkContext { + public readonly failures: RuleFailure[] = []; + + constructor(public readonly sourceFile: ts.SourceFile, public readonly ruleName: string, public readonly options: T) {} + + /** Add a failure with any arbitrary span. Prefer `addFailureAtNode` if possible. */ + public addFailureAt(start: number, width: number, failure: string, fix?: Fix) { + this.addFailure(start, start + width, failure, fix); + } + + public addFailure(start: number, end: number, failure: string, fix?: Fix) { + const fileLength = this.sourceFile.end; + this.failures.push( + new RuleFailure(this.sourceFile, Math.min(start, fileLength), Math.min(end, fileLength), failure, this.ruleName, fix), + ); + } + + /** Add a failure using a node's span. */ + public addFailureAtNode(node: ts.Node, failure: string, fix?: Fix) { + this.addFailure(node.getStart(this.sourceFile), node.getEnd(), failure, fix); + } + + public createFix(...replacements: Replacement[]) { + return new Fix(this.ruleName, replacements); + } +} diff --git a/src/language/walker/walker.ts b/src/language/walker/walker.ts index 0da34d9c8a0..2613a1a5f48 100644 --- a/src/language/walker/walker.ts +++ b/src/language/walker/walker.ts @@ -17,7 +17,8 @@ import * as ts from "typescript"; -import {RuleFailure, WalkContext} from "../rule/rule"; +import {RuleFailure} from "../rule/rule"; +import {WalkContext} from "./walkContext"; import {IWalker} from "./walker"; export interface IWalker { From 4662cf15dd5593deb60dae921e4ac23fa7a1ed72 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Wed, 1 Feb 2017 20:24:14 +0100 Subject: [PATCH 8/8] Restore location of AbstractRule#isEnabled avoid unnecessary code churn --- src/language/rule/abstractRule.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/language/rule/abstractRule.ts b/src/language/rule/abstractRule.ts index fb10a4e7a8d..7713fefd8ee 100644 --- a/src/language/rule/abstractRule.ts +++ b/src/language/rule/abstractRule.ts @@ -53,10 +53,6 @@ export abstract class AbstractRule implements IRule { }; } - public isEnabled(): boolean { - return AbstractRule.isRuleEnabled(this.value); - } - public abstract apply(sourceFile: ts.SourceFile, languageService: ts.LanguageService): RuleFailure[]; public applyWithWalker(walker: IWalker): RuleFailure[] { @@ -64,6 +60,10 @@ export abstract class AbstractRule implements IRule { return this.filterFailures(walker.getFailures()); } + public isEnabled(): boolean { + return AbstractRule.isRuleEnabled(this.value); + } + protected applyWithFunction(sourceFile: ts.SourceFile, walkFn: (ctx: WalkContext) => void): RuleFailure[]; protected applyWithFunction(sourceFile: ts.SourceFile, walkFn: (ctx: WalkContext) => void, options: T): RuleFailure[]; protected applyWithFunction(sourceFile: ts.SourceFile, walkFn: (ctx: WalkContext) => void, options?: T): RuleFailure[] {