diff --git a/src/rules/noUnboundMethodRule.ts b/src/rules/noUnboundMethodRule.ts index ca6eb40e983..acf2203cd71 100644 --- a/src/rules/noUnboundMethodRule.ts +++ b/src/rules/noUnboundMethodRule.ts @@ -15,15 +15,40 @@ * 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; +} + +interface OptionsInput { + [OPTION_ALLOW_TYPEOF]?: boolean; + [OPTION_IGNORE_STATIC]?: boolean; + [OPTION_WHITELIST]?: string[]; } export class Rule extends Lint.Rules.TypedRule { @@ -31,12 +56,37 @@ export class Rule extends Lint.Rules.TypedRule { 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: @@ -88,20 +138,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): 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, 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); } } @@ -151,3 +227,15 @@ function isSafeUse(node: ts.Node): boolean { return false; } } + +function isWhitelisted(node: ts.Node, whitelist: Set, 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; +} diff --git a/test/rules/no-unbound-method/whitelist/test.tsx.lint b/test/rules/no-unbound-method/whitelist/test.tsx.lint new file mode 100644 index 00000000000..a2198c77981 --- /dev/null +++ b/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)); + +; + ~~~~~~~~ [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'. diff --git a/test/rules/no-unbound-method/whitelist/tsconfig.json b/test/rules/no-unbound-method/whitelist/tsconfig.json new file mode 100644 index 00000000000..f7855d670cb --- /dev/null +++ b/test/rules/no-unbound-method/whitelist/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "module": "commonjs" + } +} \ No newline at end of file diff --git a/test/rules/no-unbound-method/whitelist/tslint.json b/test/rules/no-unbound-method/whitelist/tslint.json new file mode 100644 index 00000000000..93bcf54c8b8 --- /dev/null +++ b/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 }] + } +}