Refactor some rules (#2359)

Refactored some of the rather simple rules to WalkContext or AbstractWalker.
no-trailing-whitespace is the only rule where the logic changes: now it looks for trailing whitespace in the source text and iff there are lines found, it searches the AST for template spans and comments.
[rule-change] no-trailing-whitespace now checks template strings by default. Use the new options ignore-template-strings to restore the old behavior.
[new-rule-option] added ignore-template-strings to no-trailing-whitespace
[enhancement] fixer of quotemark unescapes original quotemark (e.g. '\'' -> "'")

src/rules/commentFormatRule.ts

@@ -45,11 +45,11 @@ export class Rule extends Lint.Rules.AbstractRule {
                 * note that comments starting with \`///\` are also allowed, for things such as \`///<reference>\`
             * \`"check-lowercase"\` requires that the first non-whitespace character of a comment must be lowercase, if applicable.
             * \`"check-uppercase"\` requires that the first non-whitespace character of a comment must be uppercase, if applicable.
-            
+
             Exceptions to \`"check-lowercase"\` or \`"check-uppercase"\` can be managed with object that may be passed as last argument.
-            
+
             One of two options can be provided in this object:
-                
+
                 * \`"ignore-words"\`  - array of strings - words that will be ignored at the beginning of the comment.
                 * \`"ignore-pattern"\` - string - RegExp pattern that will be ignored at the beginning of the comment.
             `,

src/rules/maxFileLineCountRule.ts

@@ -44,14 +44,7 @@ export class Rule extends Lint.Rules.AbstractRule {
     }
 
     public isEnabled(): boolean {
-        const ruleArguments = this.getOptions().ruleArguments;
-        if (super.isEnabled()) {
-            const option = ruleArguments[0];
-            if (typeof option === "number" && option > 0) {
-                return true;
-            }
-        }
-        return false;
+        return super.isEnabled() && this.ruleArguments[0] > 0;
     }
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {

src/rules/maxLineLengthRule.ts

@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import { getLineRanges } from "tsutils";
 import * as ts from "typescript";
 
 import * as Lint from "../index";
@@ -44,40 +45,19 @@ export class Rule extends Lint.Rules.AbstractRule {
     }
 
     public isEnabled(): boolean {
-        const ruleArguments = this.getOptions().ruleArguments;
-        if (super.isEnabled()) {
-            const option = ruleArguments[0];
-            if (typeof option === "number" && option > 0) {
-                return true;
-            }
-        }
-        return false;
+        return super.isEnabled() && this.ruleArguments[0] > 0;
     }
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        const ruleFailures: Lint.RuleFailure[] = [];
-        const ruleArguments = this.getOptions().ruleArguments;
-        const lineLimit = ruleArguments[0];
-        const lineStarts = sourceFile.getLineStarts();
-        const errorString = Rule.FAILURE_STRING_FACTORY(lineLimit);
-        const disabledIntervals = this.getOptions().disabledIntervals;
-        const source = sourceFile.getFullText();
+        return this.applyWithFunction(sourceFile, walk, this.ruleArguments[0]);
+    }
+}
 
-        for (let i = 0; i < lineStarts.length - 1; ++i) {
-            const from = lineStarts[i];
-            const to = lineStarts[i + 1];
-            if ((to - from - 1) > lineLimit && !((to - from - 2) === lineLimit && source[to - 2] === "\r")) {
-                // first condition above is whether the line (minus the newline) is larger than the line limit
-                // second two check for windows line endings, that is, check to make sure it is not the case
-                // that we are only over by the limit by exactly one and that the character we are over the
-                // limit by is a '\r' character which does not count against the limit
-                // (and thus we are not actually over the limit).
-                const ruleFailure = new Lint.RuleFailure(sourceFile, from, to - 1, errorString, this.getOptions().ruleName);
-                if (!Lint.doesIntersect(ruleFailure, disabledIntervals)) {
-                    ruleFailures.push(ruleFailure);
-                }
-            }
+function walk(ctx: Lint.WalkContext<number>) {
+    const limit = ctx.options;
+    for (const line of getLineRanges(ctx.sourceFile)) {
+        if (line.contentLength > limit) {
+            ctx.addFailureAt(line.pos, line.contentLength, Rule.FAILURE_STRING_FACTORY(limit));
         }
-        return ruleFailures;
     }
 }

src/rules/newParensRule.ts

@@ -36,16 +36,15 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static FAILURE_STRING = "Parentheses are required when invoking a constructor";
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        const newParensWalker = new NewParensWalker(sourceFile, this.getOptions());
-        return this.applyWithWalker(newParensWalker);
+        return this.applyWithFunction(sourceFile, walk);
     }
 }
 
-class NewParensWalker extends Lint.RuleWalker {
-    public visitNewExpression(node: ts.NewExpression) {
-        if (node.arguments === undefined) {
-            this.addFailureAtNode(node, Rule.FAILURE_STRING);
+function walk(ctx: Lint.WalkContext<void>) {
+    return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
+        if (node.kind === ts.SyntaxKind.NewExpression && (node as ts.NewExpression).arguments === undefined) {
+            ctx.addFailureAtNode(node, Rule.FAILURE_STRING);
         }
-        super.visitNewExpression(node);
-    }
+        return ts.forEachChild(node, cb);
+    });
 }

src/rules/noAngleBracketTypeAssertionRule.ts

@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import { isTypeAssertion } from "tsutils";
 import * as ts from "typescript";
 
 import * as Lint from "../index";
@@ -40,20 +41,19 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static FAILURE_STRING = "Type assertion using the '<>' syntax is forbidden. Use the 'as' syntax instead.";
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithWalker(new NoAngleBracketTypeAssertionWalker(sourceFile, this.getOptions()));
+        return this.applyWithFunction(sourceFile, walk);
     }
 }
 
-class NoAngleBracketTypeAssertionWalker extends Lint.RuleWalker {
-    public visitTypeAssertionExpression(node: ts.TypeAssertion) {
-        const { expression, type } = node;
-        const fix = this.createFix(
-            // add 'as' syntax at end
-            this.createReplacement(node.getEnd(), 0, ` as ${type.getText()}`),
-            // delete the angle bracket assertion
-            this.createReplacement(node.getStart(), expression.getStart() - node.getStart(), ""),
-        );
-        this.addFailureAtNode(node, Rule.FAILURE_STRING, fix);
-        super.visitTypeAssertionExpression(node);
-    }
+function walk(ctx: Lint.WalkContext<void>) {
+    return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
+        if (isTypeAssertion(node)) {
+            const start = node.getStart(ctx.sourceFile);
+            ctx.addFailure(start, node.end, Rule.FAILURE_STRING, ctx.createFix(
+                Lint.Replacement.appendText(node.end, ` as ${ node.type.getText(ctx.sourceFile) }`),
+                Lint.Replacement.deleteFromTo(start, node.expression.getStart(ctx.sourceFile)),
+            ));
+        }
+        return ts.forEachChild(node, cb);
+    });
 }

src/rules/noAnyRule.ts

@@ -39,16 +39,18 @@ export class Rule extends Lint.Rules.AbstractRule {
         "or suppress this occurrence.";
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithWalker(new NoAnyWalker(sourceFile, this.getOptions()));
+        return this.applyWithFunction(sourceFile, walk);
     }
 }
 
-class NoAnyWalker extends Lint.RuleWalker {
-    public visitAnyKeyword(node: ts.Node) {
-        const fix = this.createFix(
-            this.createReplacement(node.getStart(), node.getWidth(), "{}"),
-        );
-        this.addFailureAtNode(node, Rule.FAILURE_STRING, fix);
-        super.visitAnyKeyword(node);
-    }
+function walk(ctx: Lint.WalkContext<void>) {
+    return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
+        if (node.kind === ts.SyntaxKind.AnyKeyword) {
+            const start = node.end - 3;
+            return ctx.addFailure(start, node.end, Rule.FAILURE_STRING, ctx.createFix(
+                new Lint.Replacement(start, 3, "{}"),
+            ));
+        }
+        return ts.forEachChild(node, cb);
+    });
 }

src/rules/noArgRule.ts

@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import { isPropertyAccessExpression } from "tsutils";
 import * as ts from "typescript";
 
 import * as Lint from "../index";
@@ -39,22 +40,17 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static FAILURE_STRING = "Access to arguments.callee is forbidden";
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithWalker(new NoArgWalker(sourceFile, this.getOptions()));
+        return this.applyWithFunction(sourceFile, walk);
     }
 }
 
-class NoArgWalker extends Lint.RuleWalker {
-    public visitPropertyAccessExpression(node: ts.PropertyAccessExpression) {
-        const expression = node.expression;
-        const name = node.name;
-
-        if (expression.kind === ts.SyntaxKind.Identifier && name.text === "callee") {
-            const identifierExpression = expression as ts.Identifier;
-            if (identifierExpression.text === "arguments") {
-                this.addFailureAtNode(expression, Rule.FAILURE_STRING);
-            }
+function walk(ctx: Lint.WalkContext<void>) {
+    return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
+        if (isPropertyAccessExpression(node) &&
+            node.name.text === "callee" &&
+            node.expression.kind === ts.SyntaxKind.Identifier && (node.expression as ts.Identifier).text === "arguments") {
+            return ctx.addFailureAtNode(node, Rule.FAILURE_STRING);
         }
-
-        super.visitPropertyAccessExpression(node);
-    }
+        return ts.forEachChild(node, cb);
+    });
 }

src/rules/noBitwiseRule.ts

@@ -44,37 +44,32 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static FAILURE_STRING = "Forbidden bitwise operation";
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithWalker(new NoBitwiseWalker(sourceFile, this.getOptions()));
+        return this.applyWithFunction(sourceFile, walk);
     }
 }
 
-class NoBitwiseWalker extends Lint.RuleWalker {
-    public visitBinaryExpression(node: ts.BinaryExpression) {
-        switch (node.operatorToken.kind) {
-            case ts.SyntaxKind.AmpersandToken:
-            case ts.SyntaxKind.AmpersandEqualsToken:
-            case ts.SyntaxKind.BarToken:
-            case ts.SyntaxKind.BarEqualsToken:
-            case ts.SyntaxKind.CaretToken:
-            case ts.SyntaxKind.CaretEqualsToken:
-            case ts.SyntaxKind.LessThanLessThanToken:
-            case ts.SyntaxKind.LessThanLessThanEqualsToken:
-            case ts.SyntaxKind.GreaterThanGreaterThanToken:
-            case ts.SyntaxKind.GreaterThanGreaterThanEqualsToken:
-            case ts.SyntaxKind.GreaterThanGreaterThanGreaterThanToken:
-            case ts.SyntaxKind.GreaterThanGreaterThanGreaterThanEqualsToken:
-                this.addFailureAtNode(node, Rule.FAILURE_STRING);
-                break;
-            default:
-                break;
+function walk(ctx: Lint.WalkContext<void>) {
+    return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
+        if (node.kind === ts.SyntaxKind.BinaryExpression) {
+            switch ((node as ts.BinaryExpression).operatorToken.kind) {
+                case ts.SyntaxKind.AmpersandToken:
+                case ts.SyntaxKind.AmpersandEqualsToken:
+                case ts.SyntaxKind.BarToken:
+                case ts.SyntaxKind.BarEqualsToken:
+                case ts.SyntaxKind.CaretToken:
+                case ts.SyntaxKind.CaretEqualsToken:
+                case ts.SyntaxKind.LessThanLessThanToken:
+                case ts.SyntaxKind.LessThanLessThanEqualsToken:
+                case ts.SyntaxKind.GreaterThanGreaterThanToken:
+                case ts.SyntaxKind.GreaterThanGreaterThanEqualsToken:
+                case ts.SyntaxKind.GreaterThanGreaterThanGreaterThanToken:
+                case ts.SyntaxKind.GreaterThanGreaterThanGreaterThanEqualsToken:
+                    ctx.addFailureAtNode(node, Rule.FAILURE_STRING);
+            }
+        } else if (node.kind === ts.SyntaxKind.PrefixUnaryExpression &&
+                   (node as ts.PrefixUnaryExpression).operator === ts.SyntaxKind.TildeToken) {
+            ctx.addFailureAtNode(node, Rule.FAILURE_STRING);
         }
-        super.visitBinaryExpression(node);
-    }
-
-    public visitPrefixUnaryExpression(node: ts.PrefixUnaryExpression) {
-        if (node.operator === ts.SyntaxKind.TildeToken) {
-            this.addFailureAtNode(node, Rule.FAILURE_STRING);
-        }
-        super.visitPrefixUnaryExpression(node);
-    }
+        return ts.forEachChild(node, cb);
+    });
 }

src/rules/noConsecutiveBlankLinesRule.ts

@@ -52,9 +52,7 @@ export class Rule extends Lint.Rules.AbstractRule {
      * Disable the rule if the option is provided but non-numeric or less than the minimum.
      */
     public isEnabled(): boolean {
-        return super.isEnabled() &&
-            (!this.ruleArguments[0] ||
-             typeof this.ruleArguments[0] === "number" && this.ruleArguments[0] > 0);
+        return super.isEnabled() && (!this.ruleArguments[0] || this.ruleArguments[0] > 0);
     }
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
@@ -70,7 +68,7 @@ function walk(ctx: Lint.WalkContext<number>) {
     let consecutiveBlankLines = 0;
 
     for (const line of utils.getLineRanges(ctx.sourceFile)) {
-        if (sourceText.substring(line.pos, line.end).search(/\S/) === -1) {
+        if (line.contentLength === 0 || sourceText.substr(line.pos, line.contentLength).search(/\S/) === -1) {
             ++consecutiveBlankLines;
             if (consecutiveBlankLines === threshold) {
                 possibleFailures.push({
@@ -108,7 +106,7 @@ function getStartOfLineBreak(sourceText: string, pos: number) {
     return sourceText[pos - 2] === "\r" ? pos - 1 : pos - 1;
 }
 
-function getTemplateRanges(sourceFile: ts.SourceFile): ts.TextRange[] {
+export function getTemplateRanges(sourceFile: ts.SourceFile): ts.TextRange[] {
     const intervals: ts.TextRange[] = [];
     const cb = (node: ts.Node): void => {
         if (node.kind >= ts.SyntaxKind.FirstTemplateToken &&

src/rules/noConstructRule.ts

@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import { isNewExpression } from "tsutils";
 import * as ts from "typescript";
 
 import * as Lint from "../index";
@@ -40,25 +41,20 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static FAILURE_STRING = "Forbidden constructor, use a literal or simple function call instead";
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithWalker(new NoConstructWalker(sourceFile, this.getOptions()));
+        return this.applyWithFunction(sourceFile, walk);
     }
 }
 
-class NoConstructWalker extends Lint.RuleWalker {
-    private static FORBIDDEN_CONSTRUCTORS = [
-        "Boolean",
-        "Number",
-        "String",
-    ];
-
-    public visitNewExpression(node: ts.NewExpression) {
-        if (node.expression.kind === ts.SyntaxKind.Identifier) {
-            const identifier = node.expression as ts.Identifier;
-            const constructorName = identifier.text;
-            if (NoConstructWalker.FORBIDDEN_CONSTRUCTORS.indexOf(constructorName) !== -1) {
-                this.addFailureAt(node.getStart(), identifier.getEnd() - node.getStart(), Rule.FAILURE_STRING);
+function walk(ctx: Lint.WalkContext<void>) {
+    return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
+        if (isNewExpression(node) && node.expression.kind === ts.SyntaxKind.Identifier) {
+            switch ((node.expression as ts.Identifier).text) {
+                case "Boolean":
+                case "String":
+                case "Number":
+                    ctx.addFailure(node.getStart(ctx.sourceFile), node.expression.end, Rule.FAILURE_STRING);
             }
         }
-        super.visitNewExpression(node);
-    }
+        return ts.forEachChild(node, cb);
+    });
 }

src/rules/noDebuggerRule.ts

@@ -36,14 +36,15 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static FAILURE_STRING = "Use of debugger statements is forbidden";
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithWalker(new NoDebuggerWalker(sourceFile, this.getOptions()));
+        return this.applyWithFunction(sourceFile, walk);
     }
 }
 
-class NoDebuggerWalker extends Lint.RuleWalker {
-    public visitDebuggerStatement(node: ts.Statement) {
-        const debuggerKeywordNode = node.getChildAt(0);
-        this.addFailureAtNode(debuggerKeywordNode, Rule.FAILURE_STRING);
-        super.visitDebuggerStatement(node);
-   }
+function walk(ctx: Lint.WalkContext<void>) {
+    return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
+        if (node.kind === ts.SyntaxKind.DebuggerStatement) {
+            return ctx.addFailureAtNode(node, Rule.FAILURE_STRING);
+        }
+        return ts.forEachChild(node, cb);
+    });
 }