eslint-plugin-better-mutation
eslint-plugin-better-mutation copied to clipboard
change how end of block works for determing let scope
We have code like this, which I believe should be valid.
function fooValid1() {
let a = 2;
[1, 2].forEach((x) => {
a = a + x;
});
[1, 2].forEach(function(x) {
a = a + x;
});
return a;
}
Since the let
is in the same "local scope" (so to speak) as the forEach
callback I believe that mutation should be allowed. To contrast, this code should not be allowed.
function doMutation(a) {
[1, 2].forEach((x) => {
a = a + x;
});
[1, 2].forEach(function (x) {
a = a + x;
});
return a;
}
function fooInvalid() {
let a = 2;
doMutation(a);
}
Doing some debugging, this appears to not work as I expect because the "upwards" recursion when looking for the let
definition stops at the callback scope. By letting the recursion continue this works exactly like I expect it should.
A lot of tests started to fail if I simply omitted the function and arrow from the isEndOfBlock
predicate though, so I have doubts that this is the fully correct solution. Perhaps it is appropriate for the let
checks and not other spots that use the function.. But I don't have enough context about all the uses to understand.
For example, this test fails:
[1,2,3].reduce((acc, x) => {
acc += x;
return acc;
}, 0);
The failure indicates that acc
is not allowed to be mutated. With that said, I'm actually a little confused that this test passes on master
, since acc
is a function argument, but I'm guessing that I'm missing something.
I could use some extra eyes on this to determine if this change is appropriate.