sindresorhus / eslint-plugin-unicorn

More than 100 powerful ESLint rules
MIT License
4.25k stars 367 forks source link

`no-array-for-each`: autofix uses an already existing variable name in method chaining #1093

Open noftaly opened 3 years ago

noftaly commented 3 years ago

I had this code before, with an Array#forEach:

topics.results
  .map(topic => /* ... */)
  .filter(topic => /* ... */)
  .forEach((topic) => {
    // ...
  });

The new no-array-for-each auto-fixed it to:

for (const topic of topics.results
  .map(topic => /* ... */)
  .filter(topic => /* ... */) {
    // ...
  }

But it used the name topic for the for loop, which is already used in the .map and .filter methods. This triggered the no-shadow rule. I don't know if unicorn can do anything about it though, appart from extracting the array to a variable before the loop, or using a suggestion rather than a fix.

One thing I though of, is to add an option to disable this rule when using method chaining, because I think it looks better when using a forEach with method chaining, than using method chaining in a for of.

fisker commented 3 years ago

Don't you think manually fix no-shadow is easier than complicated forEach case?

In you case above I would move .filter out and assign to a new variable. Easy fix.

sindresorhus commented 3 years ago

The rule is doing the correct thing here.

sindresorhus commented 3 years ago

One thing I though of, is to add an option to disable this rule when using method chaining, because I think it looks better when using a forEach with method chaining, than using method chaining in a for of.

👎 The solution is to just:

In you case above I would move .filter out and assign to a new variable. Easy fix.