Whitelist option for no-unbound-method

src/rules/noUnboundMethodRule.ts

@@ -15,27 +15,77 @@
  * limitations under the License.
  */
 
-import { hasModifier, isPropertyAccessExpression } from "tsutils";
+import {
+    hasModifier,
+    isCallExpression,
+    isIdentifier,
+    isPropertyAccessExpression,
+    isTypeOfExpression,
+} from "tsutils";
 import * as ts from "typescript";
 import * as Lint from "../index";
 
 const OPTION_IGNORE_STATIC = "ignore-static";
+const OPTION_WHITELIST = "whitelist";
+const OPTION_ALLOW_TYPEOF = "allow-typeof";
+
+const OPTION_WHITELIST_EXAMPLE = [
+    true,
+    {
+        [OPTION_IGNORE_STATIC]: true,
+        [OPTION_WHITELIST]: ["expect"],
+        [OPTION_ALLOW_TYPEOF]: true,
+    },
+];
 
 interface Options {
+    allowTypeof: boolean;
     ignoreStatic: boolean;
+    whitelist: Set<string>;
+}
+
+interface OptionsInput {
+    [OPTION_ALLOW_TYPEOF]?: boolean;
+    [OPTION_IGNORE_STATIC]?: boolean;
+    [OPTION_WHITELIST]?: string[];
 }
 
 export class Rule extends Lint.Rules.TypedRule {
     /* tslint:disable:object-literal-sort-keys */
     public static metadata: Lint.IRuleMetadata = {
         ruleName: "no-unbound-method",
         description: "Warns when a method is used outside of a method call.",
-        optionsDescription: `You may optionally pass "${OPTION_IGNORE_STATIC}" to ignore static methods.`,
+        optionsDescription: Lint.Utils.dedent`
+            You may additionally pass "${OPTION_IGNORE_STATIC}" to ignore static methods, or an options object.
+
+            The object may have three properties:
+
+            * "${OPTION_IGNORE_STATIC}" - to ignore static methods.
+            * "${OPTION_ALLOW_TYPEOF}" - ignore methods referenced in a typeof expression.
+            * "${OPTION_WHITELIST}" - ignore method references in parameters of specifed function calls.
+
+            `,
         options: {
-            type: "string",
-            enum: [OPTION_IGNORE_STATIC],
+            anyOf: [
+                {
+                    type: "string",
+                    enum: [OPTION_IGNORE_STATIC],
+                },
+                {
+                    type: "object",
+                    properties: {
+                        [OPTION_ALLOW_TYPEOF]: { type: "boolean" },
+                        [OPTION_IGNORE_STATIC]: { type: "boolean" },
+                        [OPTION_WHITELIST]: {
+                            type: "array",
+                            items: { type: "string" },
+                            minLength: 1,
+                        },
+                    },
+                },
+            ],
         },
-        optionExamples: [true, [true, OPTION_IGNORE_STATIC]],
+        optionExamples: [true, [true, OPTION_IGNORE_STATIC], OPTION_WHITELIST_EXAMPLE],
         rationale: Lint.Utils.dedent`
             Class functions don't preserve the class scope when passed as standalone variables.
             For example, this code will log the global scope (\`window\`/\`global\`), not the class instance:
@@ -87,20 +137,46 @@ export class Rule extends Lint.Rules.TypedRule {
         return this.applyWithFunction(
             sourceFile,
             walk,
-            {
-                ignoreStatic: this.ruleArguments.indexOf(OPTION_IGNORE_STATIC) !== -1,
-            },
+            parseArguments(this.ruleArguments),
             program.getTypeChecker(),
         );
     }
 }
 
+function parseArguments(args: Array<string | OptionsInput>): Options {
+    const options: Options = {
+        allowTypeof: false,
+        ignoreStatic: false,
+        whitelist: new Set(),
+    };
+
+    for (const arg of args) {
+        if (typeof arg === "string") {
+            if (arg === OPTION_IGNORE_STATIC) {
+                options.ignoreStatic = true;
+            }
+        } else {
+            options.allowTypeof = arg[OPTION_ALLOW_TYPEOF] || false;
+            options.ignoreStatic = arg[OPTION_IGNORE_STATIC] || false;
+            options.whitelist = new Set(arg[OPTION_WHITELIST]);
+        }
+    }
+
+    return options;
+}
+
 function walk(ctx: Lint.WalkContext<Options>, tc: ts.TypeChecker) {
     return ts.forEachChild(ctx.sourceFile, function cb(node): void {
         if (isPropertyAccessExpression(node) && !isSafeUse(node)) {
             const symbol = tc.getSymbolAtLocation(node);
             const declaration = symbol === undefined ? undefined : symbol.valueDeclaration;
-            if (declaration !== undefined && isMethod(declaration, ctx.options.ignoreStatic)) {
+
+            const isMethodAccess =
+                declaration !== undefined && isMethod(declaration, ctx.options.ignoreStatic);
+            const shouldBeReported =
+                isMethodAccess &&
+                !isWhitelisted(node, ctx.options.whitelist, ctx.options.allowTypeof);
+            if (shouldBeReported) {
                 ctx.addFailureAtNode(node, Rule.FAILURE_STRING);
             }
         }
@@ -150,3 +226,15 @@ function isSafeUse(node: ts.Node): boolean {
             return false;
     }
 }
+
+function isWhitelisted(node: ts.Node, whitelist: Set<string>, allowTypeof: boolean): boolean {
+    if (isTypeOfExpression(node.parent)) {
+        return allowTypeof;
+    }
+    if (isCallExpression(node.parent) && isIdentifier(node.parent.expression)) {
+        const expression = node.parent.expression;
+        const callingMethodName = expression.text;
+        return whitelist.has(callingMethodName);
+    }
+    return false;
+}

test/rules/no-unbound-method/whitelist/test.tsx.lint

@@ -0,0 +1,72 @@
+class C {
+    method(x: number) {}
+    property: () => void;
+    template(strs: TemplateStringsArray, x: any) {}
+}
+
+const c = new C();
+[0].forEach(c.method);
+            ~~~~~~~~ [0]
+[0].forEach(x => c.method(x));
+[0].forEach(c.property);
+
+c.template;
+~~~~~~~~~~ [0]
+c.template`foo${0}`;
+String.raw`${c.template}`;
+             ~~~~~~~~~~ [0]
+
+expect(c.method).toHaveBeenCalled();
+typeof c.method;
+
+test(c.method);
+     ~~~~~~~~ [0]
+
+interface I {
+    foo(): void;
+    bar: () => void;
+}
+declare var i: I;
+i.foo;
+~~~~~ [0]
+i.bar;
+
+c.method === i.foo;
+
+// OK in condition
+c.method ? 1 : 2;
+1 ? c.method : c.method;
+    ~~~~~~~~ [0]
+               ~~~~~~~~ [0]
+if (c.method) {}
+while (c.method) {}
+do {} while (c.method);
+for (c.method; c.method; c.method) {}
+
+
+[0].forEach(c.method || i.foo);
+            ~~~~~~~~ [0]
+                        ~~~~~ [0]
+[0].forEach(c.method.bind(c));
+
+<button onClick={c.method}>Click me!</button>;
+                 ~~~~~~~~ [0]
+
+class Validators {
+    static required() {
+        return null;
+    }
+    static compose(...args: Function[]) {}
+}
+
+Validators.compose(Validators.required);
+                   ~~~~~~~~~~~~~~~~~~~ [0]
+
+(condition ? expectA : expectB)(c.method);
+                                ~~~~~~~~ [0]
+(await someObject)(c.method);
+                   ~~~~~~~~ [0]
+(await someMethod())(c.method);
+                     ~~~~~~~~ [0]
+
+[0]: Avoid referencing unbound methods which may cause unintentional scoping of 'this'.

test/rules/no-unbound-method/whitelist/tsconfig.json

@@ -0,0 +1,5 @@
+{
+    "compilerOptions": {
+        "module": "commonjs"
+    }
+}

test/rules/no-unbound-method/whitelist/tslint.json

@@ -0,0 +1,8 @@
+{
+  "linterOptions": {
+    "typeCheck": true
+  },
+  "rules": {
+    "no-unbound-method": [true, { "whitelist": ["expect"], "allow-typeof": true }]
+  }
+}