eslint/archive-website

`no-param-reassign`: `arguments` object is mutated anyhow

Closed this issue · 8 comments

I was testing ESLint for the first time and I decided to use Airbnb style guide.

Let's use an example from the page:

function foo(bar) {
  var baz = bar;
}

It passes ESLint check.

But the main point for Disallowing Reassignment of Function Parameters is that we don't mutate arguments object unintentionally.

But I think that's only true if the bar is of primitive type. Because every time I pass a value of a complex type as the argument, arguments is mutated after var baz = bar;:

function foo(bar) {
  var baz = bar;
  baz.property1 = 5;
  return arguments[0];
}

foo({ property0: 2 }); // returns { property0: 2, property1: 5 }

I just got more curious when I read a comment with lots of points regarding the same issue, but for DOM. I tried the DOM objects too. The result is the same; arguments[0] is mutated.

Am I missing something?
If it's true only for primitive types, then that'd be great to mention something about it on the docs. If it's a general thing, what about the behavior I'm experiencing? What's the point?

@mrmowji Hi, this doesn't seem to be an issue related to our website. Do you mind making an issue in https://github.com/eslint/eslint? I agree that this example is confusing because it come with the caveat that this would not be safe if the parameter is passed by reference. We could either make it clearer in the documentation or see if we can catch this case with an enhancement to the rule (I think this would be possible).

@kaicataldo Honestly I didn't know what the problem is. I thought it's more of a website issue since there's more evidence against my reasoning. So I just wanted to see if the documentation could be clearer of what is really going on, or where my next step could be. I'll take this issue to the ESLint repo too. Thanks a lot.

It's not just about mutating the arguments object, which happens only in sloppy code. (In strict mode code, reassigning to an argument does not alter the arguments object)

It's also about the clarity of mutating and reassigning - it's easier to understand functions when the arguments are static and constant throughout the life of the function.

@ljharb By first paragraph you meant something like this?

"use strict";

function foo(bar) {
  let baz = bar;
  baz.property1 = 5;
  return arguments[0];
}

let o = { property0: 2 };
console.log(foo(o)); // { property0: 2, property1: 5 }

If that's so, it still mutates the arguments, even in strict mode.

If you meant the actual argument passed to the function, it gets mutated too:

"use strict";

function foo(bar) {
  let baz = bar;
  baz.property1 = 5;
}

let o = { property0: 2 };
foo(o);
console.log(o); // { property0: 2, property1: 5 }

Right, the rule warns on mutation and reassignment of argument values - not just the arguments object.

@ljharb Hope they'd be more specific on the doc. I believe there's a misunderstanding, for example see the comment on StackOverflow I linked to. Thanks.

Not sure what to say ¯\_(ツ)_/¯ the SO comment is wrong, because this rule covers a number of patterns that do not mutate the arguments object.

It looks like the conversation is stalled here. As this is a question rather than an action item, I'm closing the issue. If you still need help, please send a message to our mailing list or chatroom. Thanks!