Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 59 additions & 1 deletion lib/rules/method.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.)
*
Expand Down Expand Up @@ -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;

Expand All @@ -138,7 +197,6 @@ function checkCallExpression(ruleHelper, callExpr, node) {
case "ArrowFunctionExpression":
case "FunctionExpression":
case "Super":
case "CallExpression":
case "ThisExpression":
case "NewExpression":
case "TSTypeAssertion":
Expand Down
89 changes: 89 additions & 0 deletions tests/rules/method.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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/,
},
],
},
],
});