A case not handled by `no-for-loop`

[chiawendt]

/* eslint unicorn/no-for-loop: 2 */

const arr = [];

for (let i = 0, j = arr.length; i < j; i += 1) {
  const element = arr[i];
  console.log(element);
}

no-for-loop should report on this but it does not.

This slightly unusual pattern is used all over the place in eslint-plugin-react.

This issue is a subtask of #250.

IssueHunt Summary

Sponsors (Total: $40.00)

• issuehunt ($40.00)

Become a sponsor now!

Or submit a pull request to get the deposits!

Tips

• Checkout the Issuehunt explorer to discover more funded issues.
• Need some help from other developers? Add your repositories on IssueHunt to raise funds.

[sindresorhus]

@golopot This was not handled by #297, right?

[chiawendt]

It is not.

[IssueHuntBot]

@IssueHunt has funded $40.00 to this issue.

---

• Submit pull request via IssueHunt to receive this reward.
• Want to contribute? Chip in to this issue via IssueHunt.
• Checkout the IssueHunt Issue Explorer to see more funded issues.
• Need help from developers? Add your repository on IssueHunt to raise funds.

[MrHen]

I took a quick look at this and think this should be a separate rule that auto-fixes this shape into a normal for loop. no-for-loop can then convert the normal for loop since it already handles that.

Original:

for (let i = 0, j = arr.length; i < j; i += 1) {
  const element = arr[i];
  console.log(element);
}

Fixed:

for (let i = 0; i < arr.length; i += 1) {
  const element = arr[i];
  console.log(element);
}

---

Original:

for (let i = 0, j = arr.length; i < j; i += 1) {
  const element = arr[i];
  console.log(element);
  console.log(j);
}

Fixed:

for (let i = 0; i < arr.length; i += 1) {
  const element = arr[i];
  console.log(element);
  console.log(arr.length);
}

[sindresorhus]

@MrHen Are we sure we can depend on the fixer output of other rules (the order of fixes)? I don't fully remember how fixers work. How about a rule named simplify-for-loop?

Personally, I don't want for-loops at all, so I would prefer just keeping the logic in this rule, as I don't really see the point of simplifying a for-loop if it can just be removed altogether.

[MrHen]

@sindresorhus auto-fix will repeatedly attempt to validate / fix after it applies a fixing. I'm not sure what happens if two things want to fix the same code but I don't think it would matter here because the fixers are not going to immediately overlap.

The main reason I'd want this check in a different rule is because it would make the rule logic and test cases easier to understand and maintain. The end result will be:

for (let i = 0, j = arr.length; i < j; i += 1) {
  const element = arr[i];
  console.log(element);
}

Will be auto-fixed to:

for (let i = 0; i < arr.length; i += 1) {
  const element = arr[i];
  console.log(element);
}

Which is then auto fixed to:

for (const [i, element] of arr) {
  console.log(element);
}

The end result will be exactly what you want.

[sindresorhus]

Alright, let's do it as a separate rule then.

[jmoore914]

I can take this one!

[fisker]

@jmoore914 I thought this is fixed by #476 , but apparently ArrayPattern is missed, I just send a follow up PR #489 fix this.

@sindresorhus @MrHen I missed this issue, when I review #476, we already do destructuring in the for...of.

Update: #489 didn't fix this

[snowteamer]

Alright, let's do it as a separate rule then.

What about restricting the scope of this proposed new separate rule to only address array length caching ?

• it would still address the specific OP issue
• simpler than checking for every possible way of declaring an extra variable
• array length caching was once a possible performance optimization, but nowadays modern browsers automatically cache it in such for loops, making manual caching an obsolete practice unless targeting old browsers
• all proposed pass/fail exemples in Add no-for-loop-extra-variable rule #492 already concern a "length" variable anyway
• maybe also the possibility to name the new rule no-array-length-caching instead of no-for-loop-extra-variable