Description

Following up on feedback while proposing a change to no-loop-func in core ESLint eslint#17382.

In the example given, the loop is conditioned on a variable declared outside the loop:

var i, arr = [];

for (i = 1; i <= 3; i++) {
    arr.push(() => i);
}

arr.map(f => f()); // expected: [1, 2, 3], actual: [4, 4, 4]

This is used as an example of "unsafe" closure behaviour. I disagree that it's unsafe because the scope of i is clear in relation to closures, but that's besides the point here.

But, I think this example highlights a different potential problem that should be addressed - using an already existing variable as a loop variable can lead to confusion.

I'd propose a new rule, no-loop-var-reuse that prohibits creating loops that use variables in the loop statement not declared by the loop itself, and that prohibits declaring variables in the loop statement or body that have a scope that is wider than a single loop iteration. The loop should also not create any variables that shadow variables in a higher scope (though this is probably covered by no-shadow already - I think it should still be an option to allow shadowing in general, but make this rule strictly disallow shadowing in loops)

Note that this doesn't affect variable references inside the loop body, but any variables declared in the loop body also should also not be permitted to have a wider scope.

This also shouldn't affect variable references in the loop statement when those references are read-only, e.g. you can use a variable reference as part of the condition, as long as that variable is not modified anywhere in the loop statement/body.

There's a few things the new rule is intended to do:

• Prevent re-using variables for different purposes between or around loops - re-use of the same variable for different purposes is asking for hard to track down bugs. It's always possible to create a new loop variable with unambiguous scope and predictable values, so you should.
• Prevent problems when two variables with the same name are technically different variables because of scope, but it would be easy to confuse the two
• Keeping loop iterations independent except for purposeful references to variables known to be outside the scope of the loop. It should be extremely clear what is solely in the scope of each loop iteration, and what are external references.
• Eliminate variables that look like they are loop-iteration scoped, but are in fact scoped outside the loop and shared by all loop iterations.
• Make looping conditions easier to reason about by limiting how they can be modified - it should be easy to work out how/when a loop terminates and how state changes between iterations. Allowing mutable state outside the loop to affect the condition can be confusing.

Potential options:

• allowVarEscapeIfNotReused (default: false) - allow declaring variables that have a scope wider than the loop, but only when those variables are not referenced after the loop. In the default mode, when this is false, for (var i = 0; i < 10; i++) {} loops are always banned, because var is scoped outside the loop. Setting to true allows this form, but only if i is never referenced outside the loop statement/body.

• ignoreWhileLoops (default: false) - by default, the rule would prevent any use of while/do..while loops, as they can't declare their own variables in the loop statement. This would allow them.

• disllowShadowing (default: false) - disallow any variable declared in the loop statement or body to shadow a variable already declared in a higher scope.

• disallowNameReuseInSameBlockScope (default: false`) - enables a stricter check that prevents two or more loops in the same block scope from declaring variables of the same name. Even though the variables in each loop are completely separate and have different scopes, it can be easy for a developer to confuse the two if they are in the same block.

Fail

var i;
for (i = 0; i < 10; i++) { }
// i is declared in a wider scope than a single loop iteration, it shouldn't be reused

let i;
for (i = 0; i < 10; i++) { }
// i is declared in a wider scope than a single loop iteration, it shouldn't be reused

for (let i = 0; i < 10; i++) {
    var g = 10;
}
// g is declared in a wider scope than a single loop iteration, it shouldn't be reused

let n = 10;
for (let i = 0; i < n; i++) {
    n = n / 2;
}
// n is declared in a wider scope than a single loop iteration, and is modified by the loop, refactor to declare a new loop-iteration scoped variable

function() {
    for (var i = 0; i < 10; i++) {}
}
// i is declared in a wider scope than a single loop iteration, it shouldn't be reused

for (var i = 0; i < 10; i++) { }
// i is declared in a wider scope than a single loop iteration, it shouldn't be reused

for (i = 0; i < 10; i++) { }
// i is declared in a wider scope than a single loop iteration, it shouldn't be reused

function() {
    for (let i = 0; i < 20; i++) {
        var a = i === 0 ? 20 : a - 1;
    }

    return a;
}
// a is declared with a wider scope than a single loop iteration, it should be limited to the scope of a single loop iteration, or explicitly placed outside of the loop to make the scope unambiguous

for (let i = 0; i < 10; i++) { } // This is fine

for (i = 0; i < 10; i++) { }
// i is declared in a wider scope than a single loop iteration, it shouldn't be reused

const i = 10;

for (let i = 0; i < 10; i++) { }
// disallowShadowing: true - while these two variables are technically different, the second i shadows the first

for (var i = 0; i < 10; i++) { }

for (let i = 0; i < 10; i++) { }
// allow-var-escape-if-not-reused: true, disallowShadowing: true - while these two variables are technically different, the second i shadows the first

function() {
    for (let i = 0; i < 10; i++) { }

    ....

    for (let i = 0; i < 10; i++) { }
    // disallowNameReuseInSameBlockScope: true - while these two i's are completely separate variables with different scopes, the reuse of the name twice in the same block scope is disallowed with this option on
}

Pass

for (let i = 0; i < 10; i++) { }
// i is scoped to each loop iteration, all good

const n = 10;
for (let i = 0; i < n; i++) { }
// n is a read-only reference, all good

let n = 128;
for (let i = 0; i < 10; i++) {
    n = n / 2;
}
// n isn't referenced in the loop statement, all good

```js
let n = 10;
for (let i = 0, limit = n; i < limit; i++) {
    limit = limit / 2;
}
// n is a read-only reference, all good

let n = 10;
for (let i = 0, limit = n; i < limit; i++) {
    n = n / 2;
}
// n is a read-only reference in the loop statement, all good

function() {
    var a = 20;
    for (let i = 0; i < 20; i++) {
        a = a - 20;
    }

    return a;
}
// a is fine, because it's explicitly scoped outside the loop

function() {
    for (let i = 0; i < 20; i++) {
        let a = i + 20;
    }
}
// a is fine, because it's scoped to each loop iteration

function() {
    for (var i = 0; i < 10; i++) {}
}
// allow-var-escape-if-not-reused: true - i is scoped outside the loop, but it isn't referenced later.

for (let i = 0; i < 10; i++) {}
for (let i = 0; i < 10; i++) {}
// The two i's are separate variables scoped to each individual iteration of their respective loops, all good

let n = 5;
for (let i = 0; i < 10; i++) {
   n += i;
}

for (let i = 0; i < 10; i++) {
   n += i;
}
// n is scoped outside of both loops, but it is not part of the loop statement, it's only modified in the body - this is considered ok by the rule because the scope of `n` isn't potentially unclear.

Additional Info

All the examples focus on for loops, but the same logic should be applied to for..in, for..of, for await..of, while and do..while loops.