💅 lint/complexity/noForEach suggests an impossible code change when using the index parameter.

[lgarron]

Environment information

Version: 1.3.0

What happened?

Consider the following code:

const data = ["hi", "hello"];
data.forEach((entry, idx) => {
  console.log(`Entry at idx ${idx}: ${entry}`);
});
console.log(Object.entries(data));

Biome shows an error, stating:

Prefer for...of instead of Array.forEach

I'm not aware of any way to rewrite the code this way, given that the second parameter (array index) is used.

for (const [idx, entry] of Object.entries) { … } sort of works, but idx changes from a number to a string in the inner loop (and also the relative positions of the two parameters is reversed). If someone assumes that this is a recommended rewrite, this may cause subtle bugs — ones that may not show up immediately (since they can be masked by type coercion), but at some point in the future.

For a more real-world example, see:
https://github.com/cubing/cubing.js/blob/340f18f5824b79d432af238ce2379b601d089d80/src/cubing/bluetooth/smart-puzzle/gan.ts#L187-L196

Expected result

No error when the second parameter is actually used.

Code of Conduct

• I agree to follow Biome's Code of Conduct

[Conaclos]

@lgarron

You could use Array.proptotype.entries() as follows:

for (const [idx, entry] of data.entries()) {
    console.log(`Entry at idx ${idx}: ${entry}`);
}

[lgarron]

You could use Array.proptotype.entries() as follows:

Ah, interesting, thanks! What a language. 😆

The description for this linting item is:

forEach could lead to performance issues when working with large arrays. When combined with functions like .filter or .map, this causes multiple iterations over the same type.

When not chained, wouldn't Array.prototype.entries(…) be worse for performance, since it results in an unnecessary array construction?

[Conaclos]

When not chained, wouldn't Array.prototype.entries(…) be worse for performance, since it results in an unnecessary array construction?

I am unsure about this. Because these arrays are short-lived, their cost could be very low. Some engines could even have some shortcut to avoid the cost. The best way to know is to benchmark that.

[Conaclos]

To fix this issue, I propose adding a code fix that turns forEach into for of. The code fix should take the use of indexes into account. If indexes are used, the code fix could suggest for const [index, value] of array.entries() { ... }.

[abustany]

There's actually a difference between using forEach and entries with sparse arrays: forEach will skip them, while entries will include them:

> const a = [];
undefined
> a[0] = 1
1
> a[10] = 2
2
> for (const [i, x] of a.entries()) { console.log(i, x); }
0 1
1 undefined
2 undefined
3 undefined
4 undefined
5 undefined
6 undefined
7 undefined
8 undefined
9 undefined
10 2
undefined
> a.forEach((x, i) => { console.log(i, x); })
0 1
10 2
undefined
> 

I'm using forEach instead for an explicit for (let i = 0; i < arr.length; ++i) because I use Typescript with noUncheckedIndexedAccess. Interestingly, Array.prototype.entries seems to be another place where the soundness of TS's typesystem is broken 😬