ban: Refactor and add new option format

src/rules/banRule.ts

@@ -14,34 +14,86 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
+import { isCallExpression, isIdentifier, isPropertyAccessExpression } from "tsutils";
 import * as ts from "typescript";
 
 import * as Lint from "../index";
 
+interface FunctionBan {
+    name: string;
+    message?: string;
+}
+interface MethodBan extends FunctionBan {
+    object: string[];
+}
+
+interface Options {
+    functions: FunctionBan[];
+    methods: MethodBan[];
+}
+
+interface OptionsInput {
+    name: string | string[];
+    message?: string;
+}
+
 export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:disable:object-literal-sort-keys */
     public static metadata: Lint.IRuleMetadata = {
         ruleName: "ban",
         description: "Bans the use of specific functions or global methods.",
         optionsDescription: Lint.Utils.dedent`
-            A list of \`['object', 'method', 'optional explanation here']\` or \`['globalMethod']\` which ban \`object.method()\`
-            or respectively \`globalMethod()\`.`,
+            A list of banned functions or methods in the following format:
+
+            * banning functions:
+              * just the name of the function: \`"functionName"\`
+              * the name of the function in an array with one element: \`["functionName"]\`
+              * an object in the following format: \`{"name": "functionName", "message": "optional explanation message"}\`
+            * banning methods:
+              * an array with the object name, method name and optional message: \`["functionName", "methodName", "optional message"]\`
+              * an object in the following format: \`{"name": ["objectName", "methodName"], "message": "optional message"}\`
+                * you can also ban deeply nested methods: \`{"name": ["foo", "bar", "baz"]}\` bans \`foo.bar.baz()\`
+                * the first element can contain a wildcard (\`*\`) that matches everything. \`{"name": ["*", "forEach"]}\` bans\
+                  \`[].forEach(...)\`, \`$(...).forEach(...)\`, \`arr.forEach(...)\`, etc.
+            `,
         options: {
             type: "list",
             listType: {
-                type: "array",
-                items: {type: "string"},
-                minLength: 1,
-                maxLength: 3,
+                anyOf: [
+                    {
+                        type: "string",
+                    },
+                    {
+                        type: "array",
+                        items: {type: "string"},
+                        minLength: 1,
+                        maxLength: 3,
+                    },
+                    {
+                        type: "object",
+                        properties: {
+                            name: {
+                                anyOf: [
+                                    {type: "string"},
+                                    {type: "array", items: {type: "string"}, minLength: 1},
+                                ],
+                            },
+                            message: {type: "string"},
+                        },
+                        required: ["name"],
+                    },
+                ],
             },
         },
         optionExamples: [
             [
                 true,
-                ["someGlobalMethod"],
-                ["someObject", "someFunction"],
-                ["someObject", "otherFunction", "Optional explanation"],
+                "eval",
+                {name: "$", message: "please don't"},
+                ["describe", "only"],
+                {name: ["it", "only"], message: "don't focus tests"},
+                {name: ["chai", "assert", "equal"], message: "Use 'strictEqual' instead."},
+                {name: ["*", "forEach"], message: "Use a regular for loop instead."},
             ],
         ],
         type: "functionality",
@@ -54,69 +106,81 @@ export class Rule extends Lint.Rules.AbstractRule {
     }
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        const options = this.getOptions();
-        const banFunctionWalker = new BanFunctionWalker(sourceFile, options);
-        const functionsToBan = options.ruleArguments as string[][];
-        if (functionsToBan !== undefined) {
-            functionsToBan.forEach((f) => banFunctionWalker.addBannedFunction(f));
-        }
-        return this.applyWithWalker(banFunctionWalker);
+        return this.applyWithWalker(new BanFunctionWalker(sourceFile, this.ruleName, parseOptions(this.ruleArguments)));
     }
 }
 
-export class BanFunctionWalker extends Lint.RuleWalker {
-    private bannedGlobalFunctions: string[] = [];
-    private bannedFunctions: string[][] = [];
-
-    public addBannedFunction(bannedFunction: string[]) {
-        if (bannedFunction.length === 1) {
-            this.bannedGlobalFunctions.push(bannedFunction[0]);
-        } else if (bannedFunction.length >= 2) {
-            this.bannedFunctions.push(bannedFunction);
+function parseOptions(args: Array<string | string[] | OptionsInput>): Options {
+    const functions: FunctionBan[] = [];
+    const methods: MethodBan[] = [];
+    for (const arg of args) {
+        if (typeof arg === "string") {
+            functions.push({name: arg});
+        } else if (Array.isArray(arg)) {
+            switch (arg.length) {
+                case 0:
+                    break;
+                case 1:
+                    functions.push({name: arg[0]});
+                    break;
+                default:
+                    methods.push({object: [arg[0]], name: arg[1], message: arg[2]});
+            }
+        } else if (!Array.isArray(arg.name)) {
+            functions.push(arg as FunctionBan);
+        } else {
+            switch (arg.name.length) {
+                case 0:
+                    break;
+                case 1:
+                    functions.push({name: arg.name[0], message: arg.message});
+                    break;
+                default:
+                    methods.push({name: arg.name[arg.name.length - 1], object: arg.name.slice(0, -1), message: arg.message});
+            }
         }
     }
+    return { functions, methods };
+}
 
-    public visitCallExpression(node: ts.CallExpression) {
-        const expression = node.expression;
-
-        this.checkForObjectMethodBan(expression);
-        this.checkForGlobalBan(expression);
-
-        super.visitCallExpression(node);
+class BanFunctionWalker extends Lint.AbstractWalker<Options> {
+    public walk(sourceFile: ts.SourceFile) {
+        const cb = (node: ts.Node): void => {
+            if (isCallExpression(node)) {
+                if (isIdentifier(node.expression)) {
+                    this.checkFunctionBan(node.expression);
+                } else if (isPropertyAccessExpression(node.expression)) {
+                    this.checkForObjectMethodBan(node.expression);
+                }
+            }
+            return ts.forEachChild(node, cb);
+        };
+        return ts.forEachChild(sourceFile, cb);
     }
 
-    private checkForObjectMethodBan(expression: ts.LeftHandSideExpression) {
-        if (expression.kind === ts.SyntaxKind.PropertyAccessExpression
-            && expression.getChildCount() >= 3) {
-
-            const firstToken = expression.getFirstToken();
-            const firstChild = expression.getChildAt(0);
-            const secondChild = expression.getChildAt(1);
-            const thirdChild = expression.getChildAt(2);
-
-            const rightSideExpression = thirdChild.getFullText();
-            const leftSideExpression = firstChild.getChildCount() > 0
-                ? firstChild.getLastToken().getText()
-                : firstToken.getText();
-
-            if (secondChild.kind === ts.SyntaxKind.DotToken) {
-                for (const bannedFunction of this.bannedFunctions) {
-                    if (leftSideExpression === bannedFunction[0] && rightSideExpression === bannedFunction[1]) {
-                        const failure = Rule.FAILURE_STRING_FACTORY(`${leftSideExpression}.${rightSideExpression}`, bannedFunction[2]);
-                        this.addFailureAtNode(expression, failure);
-                    }
-                }
+    private checkForObjectMethodBan(expression: ts.PropertyAccessExpression) {
+        for (const ban of this.options.methods) {
+            if (expression.name.text !== ban.name) { continue; }
+            let current = expression.expression;
+            for (let i = ban.object.length - 1; i > 0; --i) {
+                if (!isPropertyAccessExpression(current) || current.name.text !== ban.object[i]) { continue; }
+                current = current.expression;
+            }
+            if (ban.object[0] === "*" ||
+                isIdentifier(current) && current.text === ban.object[0]) {
+                this.addFailureAtNode(expression, Rule.FAILURE_STRING_FACTORY(`${ban.object.join(".")}.${ban.name}`, ban.message));
+                break;
             }
         }
     }
 
-    private checkForGlobalBan(expression: ts.LeftHandSideExpression) {
-        if (expression.kind === ts.SyntaxKind.Identifier) {
-            const identifierName = (expression as ts.Identifier).text;
-            if (this.bannedGlobalFunctions.indexOf(identifierName) !== -1) {
-                this.addFailureAtNode(expression, Rule.FAILURE_STRING_FACTORY(`${identifierName}`));
+    private checkFunctionBan(name: ts.Identifier) {
+        const {text} = name;
+        for (const ban of this.options.functions) {
+            if (ban.name === text) {
+                this.addFailureAtNode(name, Rule.FAILURE_STRING_FACTORY(text, ban.message));
+                break;
             }
-
         }
     }
 }

test/rules/ban/test.ts.lint

@@ -1,26 +1,37 @@
 console.time();
 window.toString();
-~~~~~~~~~~~~~~~    [Calls to 'window.toString' are not allowed.]
+~~~~~~~~~~~~~~~    [err % ('window.toString')]
 console.log();
 document.window.toString();
-~~~~~~~~~~~~~~~~~~~~~~~~    [Calls to 'window.toString' are not allowed.]
 reference.randomContainer.window.toString();
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~    [Calls to 'window.toString' are not allowed.]
 globals.getDocument().window.toString();
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~    [Calls to 'window.toString' are not allowed.]
 _.keys(obj).forEach(fun);
-_.forEach(fun);
-~~~~~~~~~    [Calls to '_.forEach' are not allowed.]
+~~~~~~~~~~~~~~~~~~~ [err % ('*.forEach', 'Use a regular for loop instead.')]
+_.map(fun, (x) => x);
+~~~~~    [err % ('_.map')]
 _.filter(array);
-~~~~~~~~    [Calls to '_.filter' are not allowed. Use the native JavaScript 'myArray.filter' instead.]
+~~~~~~~~    [err % ('_.filter', "Use the native JavaScript 'myArray.filter' instead.")]
 describe("some text", () => {});
 xdescribe("some text", () => {});
-~~~~~~~~~    [Calls to 'xdescribe' are not allowed.]
+~~~~~~~~~    [err % ('xdescribe')]
 fdescribe("some text", () => {});
-~~~~~~~~~    [Calls to 'fdescribe' are not allowed.]
+~~~~~~~~~    [err % ('fdescribe')]
 it("some text", () => {});
 xit("some text", () => {});
-~~~          [Calls to 'xit' are not allowed.]
+~~~          [err % ('xit')]
 fit("some text", () => {});
-~~~          [Calls to 'fit' are not allowed.]
-someObject.fit()
+~~~          [err % ('fit')]
+someObject.fit()
+chai.assert.equal(1, "1");
+~~~~~~~~~~~~~~~~~ [err % ('chai.assert.equal', "Use 'strictEqual' instead.")]
+assert.equal(1, "1");
+foo.assert.equal(1, "1");
+someObject.chai.assert.equal(1, "1");
+[].forEach(() => {});
+~~~~~~~~~~ [err % ('*.forEach', 'Use a regular for loop instead.')]
+arr.forEach(() => {});
+~~~~~~~~~~~ [err % ('*.forEach', 'Use a regular for loop instead.')]
+someObject.someProperty.forEach(() => {});
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [err % ('*.forEach', 'Use a regular for loop instead.')]
+
+[err]: Calls to '%s' are not allowed.

test/rules/ban/tslint.json

@@ -2,13 +2,15 @@
   "rules": {
     "ban": [
       true,
-      ["xit"],
-      ["fit"],
-      ["xdescribe"],
+      "xit",
+      {"name": "fit"},
+      {"name": ["xdescribe"]},
       ["fdescribe"],
       ["window", "toString"],
-      ["_", "forEach"],
-      ["_", "filter", "Use the native JavaScript 'myArray.filter' instead."]
+      {"name": ["_", "map"]},
+      ["_", "filter", "Use the native JavaScript 'myArray.filter' instead."],
+      {"name": ["*", "forEach"], "message": "Use a regular for loop instead."},
+      {"name": ["chai", "assert", "equal"], "message": "Use 'strictEqual' instead."}
     ]
   }
 }