diff --git a/lib/rules/method.js b/lib/rules/method.js index 45c5020..4533381 100644 --- a/lib/rules/method.js +++ b/lib/rules/method.js @@ -61,6 +61,35 @@ function checkImport(ruleHelper, importExpr) { ruleHelper.checkMethod(fakeCall); } +// A bound function is only worth re-checking when we can tell which +// function is being bound. Rewriting any other expression would report its +// receiver as an unsupported callee, or duplicate a diagnostic that another +// visitor already emits for it. +const BINDABLE_CALLEES = ["Identifier", "MemberExpression", "CallExpression"]; + +/** + * Returns the function being bound by an expression that looks like a call to + * `Function.prototype.bind`, i.e., `foo.bar` for the `foo.bar.bind(foo)` part + * of `foo.bar.bind(foo)(baz)`. + * + * @param {object} callExpr The CallExpression to inspect + * @returns {object|undefined} The bound function, if we can name one + */ +function getBoundFunction(callExpr) { + const callee = callExpr.callee; + if ( + callee.type !== "MemberExpression" || + callee.computed || + callee.property.type !== "Identifier" || + callee.property.name !== "bind" + ) { + return undefined; + } + return BINDABLE_CALLEES.includes(callee.object.type) + ? callee.object + : undefined; +} + /** * Run ruleHelper.checkMethod for all but irrelevant callees (FunctionExpression, etc.) * @@ -129,6 +158,36 @@ function checkCallExpression(ruleHelper, callExpr, node) { break; } + case "CallExpression": { + // Calling the result of a call is only interesting when that call + // is a bind(): `foo.insertAdjacentHTML.bind(foo)(pos, bar)` runs + // the very same method as `foo.insertAdjacentHTML(pos, bar)`. + // So, we create a new mock CallExpression that calls the bound + // function directly and check that one instead. Issue #115. + const bound = getBoundFunction(node); + if (!bound) { + break; + } + + // bind() prepends its own arguments, all but `thisArg`, to the + // ones of the eventual call. A SpreadElement in the `thisArg` + // position may well stand for prepended arguments too and we + // cannot tell how many, so we leave those calls alone. + const [thisArg, ...prependedArguments] = node.arguments; + if (thisArg && thisArg.type === "SpreadElement") { + break; + } + + const newCallExpr = Object.assign({}, callExpr); + newCallExpr.callee = bound; + newCallExpr.arguments = [ + ...prependedArguments, + ...callExpr.arguments, + ]; + checkCallExpression(ruleHelper, newCallExpr, bound); + break; + } + case "TSAsExpression": break; @@ -138,7 +197,6 @@ function checkCallExpression(ruleHelper, callExpr, node) { case "ArrowFunctionExpression": case "FunctionExpression": case "Super": - case "CallExpression": case "ThisExpression": case "NewExpression": case "TSTypeAssertion": diff --git a/tests/rules/method.js b/tests/rules/method.js index 4d62e4a..2ac3693 100644 --- a/tests/rules/method.js +++ b/tests/rules/method.js @@ -408,6 +408,35 @@ eslintTester.run("method", rule, { // # 232: disallow setHTMLUnsafe, but OK with static string. code: "foo.setHTMLUnsafe('static string')", }, + { + // #115: `baz` is called on the bound function, and is not a sink. + code: "foo.bind(bar).baz()", + }, + { + code: 'document.body.insertAdjacentHTML.bind(document.body)("afterend", "harmless")', + }, + { + // #115: bind() may also provide the arguments. + code: 'document.body.insertAdjacentHTML.bind(document.body, "afterend", "harmless")()', + }, + { + // #115: the result of a call that is not a bind() stays unknown. + code: "getInserter()('afterend', evil)", + }, + { + // #115: a spread `thisArg` may carry prepended arguments as well, + // so we cannot tell where `evil` ends up. Like #214, we allow it. + code: "document.body.insertAdjacentHTML.bind(...l)('afterend', evil);", + ...ECMA_VERSION_2020_ONLY_OPTIONS, + }, + { + // #115: binding a callee we cannot name must stay quiet, rather + // than report it as an unsupported callee. + code: "(class {}).bind(null)()", + }, + { + code: "({ f: 1 }).bind(null)()", + }, ], // Examples of code that should trigger the rule @@ -967,5 +996,65 @@ eslintTester.run("method", rule, { }, ], }, + { + // #115: bound calls are checked like direct calls. + code: 'document.body.insertAdjacentHTML.bind(document.body)("afterend", foo)', + errors: [ + { + message: + /Unsafe call to document.body.insertAdjacentHTML for argument 1/, + }, + ], + }, + { + // #115: bind() may also provide the arguments. + code: 'document.body.insertAdjacentHTML.bind(document.body, "afterend")(foo)', + errors: [ + { + message: + /Unsafe call to document.body.insertAdjacentHTML for argument 1/, + }, + ], + }, + { + // #115: objectMatches still applies to the bound method. + code: "document.write.bind(document)(foo)", + errors: [ + { + message: /Unsafe call to document.write for argument 0/, + }, + ], + }, + { + // #115: binding an already bound function. + code: 'document.body.insertAdjacentHTML.bind(a).bind(b)("afterend", foo)', + errors: [ + { + message: + /Unsafe call to document.body.insertAdjacentHTML for argument 1/, + }, + ], + }, + { + // #115: a trailing spread does not hide the arguments before it. + code: "document.body.insertAdjacentHTML.bind(document.body, 'afterend', foo, ...rest)()", + errors: [ + { + message: + /Unsafe call to document.body.insertAdjacentHTML for argument 1/, + }, + ], + ...ECMA_VERSION_2020_ONLY_OPTIONS, + }, + { + // #115: binding an already reported call reports it just once. + code: "(document.body.insertAdjacentHTML`afterend${foo}`).bind(null)()", + errors: [ + { + message: + /Unsafe call to document.body.insertAdjacentHTML for argument 1/, + }, + ], + }, ], });