False positive in prefer-rest-params when checking arguments.length

[mik01aj]

ESLint version: 2.8.0

What parser are you using?

    "parserOptions": {
        "ecmaVersion": 6,
        "sourceType": "module",
        "ecmaFeatures": {"jsx": true},
    },

When linting this code:

function makeTree(rootPath, branches) {
    console.assert(arguments.length === 2);
    console.assert(_.isArray(rootPath));
    console.assert(_.isArray(branches) && _.isArray(branches[0]));
    return _.map(branches, branch => rootPath.concat(branch));
}

I get this warning:

Use the rest parameters instead of 'arguments'  prefer-rest-params

I believe this to be a false positive. Imho just checking arguments.length should be an exception to this rule (possibly configurable).

[platinumazure]

Hmm. This is a tough one. On the one hand, it would be easy to add a rest param and assert that its length is zero; on the other hand, introducing another variable that you won't even really use just to satisfy the rule would be pretty silly.

@eslint/eslint-team Do we consider this a bug, or an enhancement request?

[nzakas]

It's an enhancement request, but one I don't think we should do. The purpose of the rule is to flag uses of arguments, which it is doing. You can replace the arguments length check by checking if branches is undefined.

[michaelficarra]

@nzakas No, that is not how you would replace the arguments.length check. Still, it can be removed using rest parameters, so this rule is working as intended. But I'd support an option to allow arguments.length since I'd prefer to read a function with two parameters in the parameter list than see a rest parameter that is later destructured.

[nzakas]

@michaelficarra fair enough, are you willing to champion this option?

[michaelficarra]

No. By support, I meant endorse. I do not plan on participating.

[mikesherov]

@mik01aj, thanks for contributing. @nzakas, I'm torn on this one. While I do think the provided example doesn't show the full breadth of situations in which arguments.length is preferable, there are lots of good use cases, especially libraries with lots of variadic functions. Looking at the jQuery source, for example, yields 19 uses of arguments.length to power heavily overloaded functions.

Now, I'm not a fan of such parameter hockey in my own APIs, but arguments.length is nifty to have around even if you don't want arguments itself.

I'm not going to champion this one but I would give it a 👍 if someone else did.

[alberto]

@eslint/eslint-team is anyone willing to champion this?

[mysticatea]

It seems reasonable to me.
Also, this is a relaxing enhancement, so this is a minor fix.

I will champion this.

[platinumazure]

Just to make sure, we are expanding this to support any member expression checks where arguments is the object and the property is non-numeric per discussion in #6835?

[mikesherov]

@platinumazure, seems correct.

[mik01aj]

@platinumazure According to docs on MDN, there are just these:

• arguments.callee
• arguments.caller
• arguments.length
• arguments[@@iterator]

I don't think that accessing the callee and caller should be allowed.

[platinumazure]

@mik01aj Regarding callee/caller, we do have a separate rule for that (no-caller).

[mikesherov]

Right, I agree @mik01aj but the rule is about prefering rest parameters over arguments, so erroring on using arguments other than as a parameter shouldn't cause an issue.

[mik01aj]

@platinumazure good point, thx. Agree.