Change Request: no-loop-func should ignore variables declared outside the loop #17382
Description
ESLint version
8.45.0
What problem do you want to solve?
At the moment, no-loop-func will highlight any variable reference that is declared outside the loop, but is mutated inside or after the loop as being "unsafe".
e.g. the test cases:
{
code: "let a; for (let i=0; i<l; i++) { a = 1; (function() { a; });}",
parserOptions: { ecmaVersion: 6 },
errors: [{ messageId: "unsafeRefs", data: { varNames: "'a'" }, type: "FunctionExpression" }]
},
{
code: "let a; for (let i in {}) { (function() { a; }); a = 1; }",
parserOptions: { ecmaVersion: 6 },
errors: [{ messageId: "unsafeRefs", data: { varNames: "'a'" }, type: "FunctionExpression" }]
},
{
code: "let a; for (let i of {}) { (function() { a; }); } a = 1; ",
parserOptions: { ecmaVersion: 6 },
errors: [{ messageId: "unsafeRefs", data: { varNames: "'a'" }, type: "FunctionExpression" }]
},
{
code: "let a; for (let i=0; i<l; i++) { (function() { (function() { a; }); }); a = 1; }",
parserOptions: { ecmaVersion: 6 },
errors: [{ messageId: "unsafeRefs", data: { varNames: "'a'" }, type: "FunctionExpression" }]
},
{
code: "let a; for (let i in {}) { a = 1; function foo() { (function() { a; }); } }",
parserOptions: { ecmaVersion: 6 },
errors: [{ messageId: "unsafeRefs", data: { varNames: "'a'" }, type: "FunctionDeclaration" }]
},
{
code: "let a; for (let i of {}) { (() => { (function() { a; }); }); } a = 1;",
parserOptions: { ecmaVersion: 6 },
errors: [{ messageId: "unsafeRefs", data: { varNames: "'a'" }, type: "ArrowFunctionExpression" }]
},
{
code: "var a; for (let x of xs) { a = 1; (function() { a; }); }",
parserOptions: { ecmaVersion: 6 },
errors: [{ messageId: "unsafeRefs", data: { varNames: "'a'" }, type: "FunctionExpression" }]
},
{
code: "var a; for (let x of xs) { (function() { a; }); a = 1; }",
parserOptions: { ecmaVersion: 6 },
errors: [{ messageId: "unsafeRefs", data: { varNames: "'a'" }, type: "FunctionExpression" }]
},
{
code: "let a; function foo() { a = 10; } for (let x of xs) { (function() { a; }); } foo();",
parserOptions: { ecmaVersion: 6 },
errors: [{ messageId: "unsafeRefs", data: { varNames: "'a'" }, type: "FunctionExpression" }]
},
{
code: "let a; function foo() { a = 10; for (let x of xs) { (function() { a; }); } } foo();",
parserOptions: { ecmaVersion: 6 },
errors: [{ messageId: "unsafeRefs", data: { varNames: "'a'" }, type: "FunctionExpression" }]
}Reading the docs for the rule, my understanding is that the rule is trying to prevent the very specific scenario of a function creating a closure over a loop variable that has a wider scope than the user might expect, and therefore that that loop variable is shared across all iterations.
i.e. it is trying to point out to the user that for (var a in {}) { ... } is equivalent to var a; for(a in {}) { ... }, and that capturing a is probably not going to do what they expect.
But in the case that the user has already declared a variable outside of the loop, I don't think the scope of that variable is at all unclear, at least in regards to loop iterations.
Unless the user fundamentally misunderstands scoping, I don't think any reasonable user would think the variable isn't shared by all iterations of the loop already, and therefore mutating it in/after the loop and capturing it isn't a problem.
What do you think is the correct solution?
I think the rule needs to ignore any variable references to variables that are declared outside the loop.
The rule should solely be detecting the case where the scope of a variable declared by the loop statement is wider than a single loop iteration, since that is the case when capturing it is counter-intuitive/"unsafe".
Participation
- I am willing to submit a pull request for this change.
Additional comments
Metadata
Assignees
Labels
Type
Projects
Status
Complete
Activity