Add prefer-destructuring-in-parameters rule

[fisker]

Questions:

1. Should we allow rename, maybe under an option

- const foo = (bar, x) => bar.x === x
+ const foo = ({x: xOfBar}, x) => xOfBar === x

2. Not sure if we should only allow 0/1 or just make sure the fixed code not against no-unreadable-array-destructuring rule

- const foo = bar => bar[1] + bar[2]
+ const foo = ([, second, third] => second + third)

3. Should we add a limitation on numbers of properties

See https://github.com/sindresorhus/eslint-plugin-unicorn/pull/1045/files#diff-e9f53b43e2668561c178de9cab0e6745a034fee9e0c5321f93c83750b6c73553R740, this case creates a lot of variables.

---

Did not fix codebase, since the diff is big, I want keep this PR small. But you can check changes 9887f56

Fixes #954

[fisker]

Another question, currently

- const foo = (bar, baz) => bar.x === baz.x;
+ const foo = ({x}, baz) => x === baz.x;

This is a little a awkward, baz.x can't be destructed, because we created a new variable x, maybe we want ignore this case "accessing same property on different parameter".

But if we decided allow rename, this won't be a problem, we can fix to

- const foo = (bar, baz) => bar.x === baz.x;
+ const foo = ({x: xOfBar}, {x: xOfBaz}) => xOfBar === xOfBaz;

[sindresorhus]

I think this rule should have an option to only enforce it for inline arrow functions. I find it a bit opinionated to do it for all arrow functions. Constructuring many times leads to less readable code when applied to large functions.

[sindresorhus]

Should we allow rename, maybe under an option

Are you asking whether we should trigger on the case where it needs to be renamed or are you asking whether we should allow renaming at all?

Not sure if we should only allow 0/1 or just make sure the fixed code not against

Yes, just make sure the fix follows what we enforce in no-unreadable-array-destructuring.

Should we add a limitation on numbers of properties

Yes, but not sure what number makes sense. Should it be configurable?

[fregante]

Should we add a limitation on numbers of properties

When I decide whether to destructure or just use the object, I also think of:

• which one is shorter

  this is fine:

  - element => element.id
  + ({id}) => id

  this not so much:

  - user => user.isHomeDirectoryOpen
  + ({isHomeDirectoryOpen}) => isHomeDirectoryOpen

• does the property make sense on its own?

  event.name makes perfect sense on its own, while name… name of what?

While it's a nice rule, I'm not sure that it can consistently improve the readability. More like 50/50

[sindresorhus]

@fisker I'm not sure about this rule. It seems to make code more unreadable in many cases. Maybe many of the cases could be detected, but not sure all can. (#1046)

[sindresorhus]

Closing as this has been stale for 2 years.