Rule Change: Add no-loop-func option to ignore known immediate execution calls

[matwilko]

What rule do you want to change?

no-loop-func

What change to do you want to make?

Generate fewer warnings

How do you think the change should be implemented?

A new option

Example code

const filterKeys = {
    'a': [],
    'b': []
    'c': []
};
const values = [ ['a', 5], ['b', 6], ['c', 3], ['a', 3]];

for (var key in filterKeys) {
    filterKeys[key] = values.filter(([k,v]) => k === key).map(([k,v]) => v);
}

What does the rule currently do for this code?

no-loop-func will highlight the use of key in filter as unsafe. Naively, this is true, because it's capturing the unsafe loop variable. But the closure doesn't escape the loop, because filter is known to evaluate eagerly and immediately, and so the closure is already dropped by the time filter returns.

What will the rule do after it's changed?

I'd propose adding an extra option to no-loop-func to allow ignoring the use of otherwise unsafe variables in functions when the function is being used in methods well-known to be eager and immediately executing, so we know the unsafe closure won't escape beyond that function invocation.

This analysis obviously doesn't apply to variable use inside nested functions within these function calls, because those inner closures can escape the loop (e.g. by being embedded in an array by map)

Why as a new option? Unlike in #16902, where we can analyse the immediate use and discard of the function syntactically, we can't check that a call to filter or map is guaranteed to be the call we think it is (i.e. Array.prototype.xxx). I think this fundamentally weakens the otherwise quite strict checking of this rule, and enables unsafe captures in inappropriate situations (e.g. a random object with a map method). Therefore it should be up to users to intentionally weaken the analysis here if they wish to.

(This kind of analysis would be possible in typescript-eslint when type info is available however.)

I'd propose to begin with, all the methods on Array that do eager + immediate work on an array based on a passed function be allow-listed, so: every, filter, find, findIndex, findLast, findLastIndex, flatMap, forEach, Array.from, Array.fromAsync, group, groupToMap, map, reduce, reduceRight, some, sort, toSorted.

Participation

• I am willing to submit a pull request to implement this change.

Additional comments

I'm happy to take a stab at implementation, though writing more complex rule scenarios like this is new to me, so it might be quicker/easier for someone more knowledgeable to implement.

[nzakas]

Thanks for the suggestion. We try not to create rule or rule options that rely on the name of the method being called to determine what to do. Concretely, we can't know that because a method is called filter() that it's being called on an array, so it's not safe to make assumptions about that.

This rule is intentionally a bit dumb -- when you come across special cases, you can always use a disable comment for that.