space-before-function-paren: Rewrite

src/rules/spaceBeforeFunctionParenRule.ts

@@ -61,130 +61,66 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static MISSING_WHITESPACE_ERROR = "Missing whitespace before function parens";
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithWalker(new FunctionWalker(sourceFile, this.getOptions()));
+        return this.applyWithFunction(sourceFile, walk, parseOptions(this.ruleArguments[0] as Option | Options | undefined));
     }
 }
 
 type OptionName = "anonymous" | "asyncArrow" | "constructor" | "method" | "named";
-
+const optionNames: OptionName[] = ["anonymous", "asyncArrow", "constructor", "method", "named"];
 type Option = "always" | "never";
+type Options = Partial<Record<OptionName, Option>>;
 
-interface CachedOptions {
-    anonymous?: Option;
-    asyncArrow?: Option;
-    constructor?: Option;
-    method?: Option;
-    named?: Option;
-}
-
-class FunctionWalker extends Lint.RuleWalker {
-    private scanner: ts.Scanner;
-    // assign constructor now to avoid typescript assuming its a function type
-    private cachedOptions: CachedOptions = {constructor: undefined};
-
-    constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
-        super(sourceFile, options);
-        this.scanner = ts.createScanner(ts.ScriptTarget.ES5, false, ts.LanguageVariant.Standard, sourceFile.text);
-        this.cacheOptions();
-    }
-
-    protected visitArrowFunction(node: ts.ArrowFunction): void {
-        const option = this.getOption("asyncArrow");
-        const syntaxList = Lint.childOfKind(node, ts.SyntaxKind.SyntaxList)!;
-        const isAsyncArrow = syntaxList.getStart() === node.getStart() && syntaxList.getText() === "async";
-        const openParen = isAsyncArrow ? Lint.childOfKind(node, ts.SyntaxKind.OpenParenToken) : undefined;
-        this.evaluateRuleAt(openParen, option);
-
-        super.visitArrowFunction(node);
-    }
-
-    protected visitConstructorDeclaration(node: ts.ConstructorDeclaration): void {
-        const option = this.getOption("constructor");
-        const openParen = Lint.childOfKind(node, ts.SyntaxKind.OpenParenToken);
-        this.evaluateRuleAt(openParen, option);
-
-        super.visitConstructorDeclaration(node);
-    }
-
-    protected visitFunctionDeclaration(node: ts.FunctionDeclaration): void {
-        this.visitFunction(node);
-        super.visitFunctionDeclaration(node);
-    }
-
-    protected visitFunctionExpression(node: ts.FunctionExpression): void {
-        this.visitFunction(node);
-        super.visitFunctionExpression(node);
-    }
-
-    protected visitMethodDeclaration(node: ts.MethodDeclaration): void {
-        this.visitMethod(node);
-        super.visitMethodDeclaration(node);
+function parseOptions(json: Option | Options | undefined): Options {
+    // Need to specify constructor or it will be Object
+    const options: Options = { constructor: undefined };
+    for (const optionName of optionNames) {
+        options[optionName] = typeof json === "object" ? json[optionName] : json === undefined ? "always" : json;
     }
+    return options;
+}
 
-    protected visitMethodSignature(node: ts.SignatureDeclaration): void {
-        this.visitMethod(node);
-        super.visitMethodSignature(node);
-    }
-
-    private cacheOptions(): void {
-        const options = (this.getOptions() as any[])[0] as Option | { [K in OptionName]: Option } | undefined;
-        const optionNames: OptionName[] = ["anonymous", "asyncArrow", "constructor", "method", "named"];
-
-        optionNames.forEach((optionName) => {
-            switch (options) {
-                case undefined:
-                case "always":
-                    this.cachedOptions[optionName] = "always";
-                    break;
-
-                case "never":
-                    this.cachedOptions[optionName] = "never";
-                    break;
-
-                default:
-                    this.cachedOptions[optionName] = options[optionName];
+function walk(ctx: Lint.WalkContext<Options>): void {
+    const { options, sourceFile } = ctx;
+    ts.forEachChild(sourceFile, function cb(node: ts.Node): void {
+        const option = getOption(node, options);
+        if (option !== undefined) {
+            const openParen = Lint.childOfKind(node, ts.SyntaxKind.OpenParenToken)!;
+            const hasSpace = ts.isWhiteSpaceLike(sourceFile.text.charCodeAt(openParen.end - 2));
+
+            if (hasSpace && option === "never") {
+                const pos = openParen.getStart() - 1;
+                ctx.addFailureAt(pos, 1, Rule.INVALID_WHITESPACE_ERROR, Lint.Replacement.deleteText(pos, 1));
+            } else if (!hasSpace && option === "always") {
+                const pos = openParen.getStart();
+                ctx.addFailureAt(pos, 1, Rule.MISSING_WHITESPACE_ERROR, Lint.Replacement.appendText(pos, " "));
             }
-        });
-    }
+        }
 
-    private getOption(optionName: OptionName): Option | undefined {
-        return this.cachedOptions[optionName];
-    }
+        ts.forEachChild(node, cb);
+    });
+}
 
-    private evaluateRuleAt(openParen?: ts.Node, option?: Option): void {
-        if (openParen === undefined || option === undefined) {
-            return;
-        }
+function getOption(node: ts.Node, options: Options): Option | undefined {
+    switch (node.kind) {
+        case ts.SyntaxKind.ArrowFunction:
+            return Lint.hasModifier(node.modifiers, ts.SyntaxKind.AsyncKeyword) ? options.asyncArrow : undefined;
 
-        const hasSpace = this.isSpaceAt(openParen.getStart() - 1);
+        case ts.SyntaxKind.Constructor:
+            return options.constructor;
 
-        if (hasSpace && option === "never") {
-            const pos = openParen.getStart() - 1;
-            this.addFailureAt(pos, 1, Rule.INVALID_WHITESPACE_ERROR, this.deleteText(pos, 1));
-        } else if (!hasSpace && option === "always") {
-            const pos = openParen.getStart();
-            this.addFailureAt(pos, 1, Rule.MISSING_WHITESPACE_ERROR, this.appendText(pos, " "));
-        }
-    }
+        case ts.SyntaxKind.FunctionDeclaration:
+            return options.named;
 
-    private isSpaceAt(textPos: number): boolean {
-        this.scanner.setTextPos(textPos);
-        const prevTokenKind = this.scanner.scan();
-        return prevTokenKind === ts.SyntaxKind.WhitespaceTrivia;
-    }
+        case ts.SyntaxKind.FunctionExpression:
+            return (node as ts.FunctionExpression).name !== undefined ? options.named : options.anonymous;
 
-    private visitFunction(node: ts.Node): void {
-        const identifier = Lint.childOfKind(node, ts.SyntaxKind.Identifier);
-        const hasIdentifier = identifier !== undefined && (identifier.getEnd() !== identifier.getStart());
-        const optionName = hasIdentifier ? "named" : "anonymous";
-        const option = this.getOption(optionName);
-        const openParen = Lint.childOfKind(node, ts.SyntaxKind.OpenParenToken);
-        this.evaluateRuleAt(openParen, option);
-    }
+        case ts.SyntaxKind.MethodDeclaration:
+        case ts.SyntaxKind.MethodSignature:
+        case ts.SyntaxKind.GetAccessor:
+        case ts.SyntaxKind.SetAccessor:
+            return options.method;
 
-    private visitMethod(node: ts.Node): void {
-        const option = this.getOption("method");
-        const openParen = Lint.childOfKind(node, ts.SyntaxKind.OpenParenToken);
-        this.evaluateRuleAt(openParen, option);
+        default:
+            return undefined;
     }
 }

test/rules/space-before-function-paren/mixed/test.ts.fix

@@ -1,9 +1,4 @@
 // Anonymous
-function () {}
-function (): void {
-}
-function (a: string, cb: ()=>{}): void {}
-
 var f = function () {};
 var f = function (): void {
 };

test/rules/space-before-function-paren/mixed/test.ts.lint

@@ -1,12 +1,4 @@
 // Anonymous
-function() {}
-        ~  [missing-space]
-function(): void {
-        ~  [missing-space]
-}
-function(a: string, cb: ()=>{}): void {}
-        ~  [missing-space]
-
 var f = function() {};
                 ~  [missing-space]
 var f = function(): void {

test/rules/space-before-function-paren/never/test.ts.fix

@@ -42,6 +42,9 @@ class MyClass {
         two(): void {
         }
         three(a: string, cb: ()=>{}): void {}
+
+        get a() {}
+        set a() {}
 }
 
 

test/rules/space-before-function-paren/never/test.ts.lint

@@ -1,37 +1,37 @@
 // Anonymous
 function () {}
-        ~  [space-before-function-paren]
+        ~ [0]
 function (): void {
-        ~  [space-before-function-paren]
+        ~ [0]
 }
 function (a: string, cb: ()=>{}): void {}
-        ~  [space-before-function-paren]
+        ~ [0]
 
 var f = function () {};
-                ~  [space-before-function-paren]
+                ~ [0]
 var f = function (): void {
-                ~  [space-before-function-paren]
+                ~ [0]
 };
 var f = function (a: string, cb: ()=>{}): void {};
-                ~  [space-before-function-paren]
+                ~ [0]
 
 
 // Named
 function foobar (){}
-               ~  [space-before-function-paren]
+               ~ [0]
 function foobar (): void{
-               ~  [space-before-function-paren]
+               ~ [0]
 }
 function foobar (a: string, cb: ()=>{}): void{}
-               ~  [space-before-function-paren]
+               ~ [0]
 
 var f = function foobar (){};
-                       ~  [space-before-function-paren]
+                       ~ [0]
 var f = function foobar (): void{
-                       ~  [space-before-function-paren]
+                       ~ [0]
 };
 var f = function foobar (a: string, cb: ()=>{}): void{};
-                       ~  [space-before-function-paren]
+                       ~ [0]
 
 
 // Async Arrow
@@ -40,44 +40,49 @@ var f = function foobar (a: string, cb: ()=>{}): void{};
 var arrow = () => {};
 
 async () => {};
-     ~ [space-before-function-paren]
+     ~ [0]
 var arrow = async () => {};
-                 ~ [space-before-function-paren]
+                 ~ [0]
 
 
 // Method
 interface IMyInterface {
         one ();
-           ~  [space-before-function-paren]
+           ~ [0]
         two (): void;
-           ~  [space-before-function-paren]
+           ~ [0]
         three (a: string, cb: ()=>{}): void;
-             ~  [space-before-function-paren]
+             ~ [0]
 }
 class MyClass {
         one () {}
-           ~  [space-before-function-paren]
+           ~ [0]
         two (): void {
-           ~  [space-before-function-paren]
+           ~ [0]
         }
         three (a: string, cb: ()=>{}): void {}
-             ~  [space-before-function-paren]
+             ~ [0]
+
+        get a () {}
+             ~ [0]
+        set a () {}
+             ~ [0]
 }
 
 
 // Constructor
 class MyClass {
         constructor () {}
-                   ~  [space-before-function-paren]
+                   ~ [0]
 }
 class MyClass {
         constructor (): void {
-                   ~  [space-before-function-paren]
+                   ~ [0]
         }
 }
 class MyClass {
         constructor (a: string, cb: ()=>{}): void {}
-                   ~  [space-before-function-paren]
+                   ~ [0]
 }
 
-[space-before-function-paren]: Spaces before function parens are disallowed
+[0]: Spaces before function parens are disallowed