sindresorhus / eslint-plugin-unicorn

More than 100 powerful ESLint rules
MIT License
4.13k stars 360 forks source link

consistent-function-scoping false positive in loops #792

Open klimashkin opened 4 years ago

klimashkin commented 4 years ago

Hi!

consistent-function-scoping is complaining about a function that is being created in a loop, and which depends on a dynamic variable inside that loop.

export const myMap = new Map();

for (const value of values) {
  const regexp = new RegExp(`^${value}\\/?$`);
  const match = href => regexp.test(href);

  myMap.set(value, {value, regexp, match});
}
ESLint: Move arrow function `match` to the outer scope.(unicorn/consistent-function-scoping)
fisker commented 4 years ago

I tried to fix this issue, but seems not easy as I thought.

Other broken cases

            for (const value in values) {
                const a = 1;
                const match = () => a;
            }
            for (let i = 0; i < 9; i ++) {
                const a = 1;
                const match = () => a;
            }
GuestrixErik commented 2 months ago

I can confirm this is a problem.

My (sadly less minimal) repro:

export const foo = (foos: readonly string[]): void => {
  for (const blockScopedConstant of foos) {
    // ✅ Nothing is suggested here, and everything is fine. ✅
    const fineClosure = (): void => {
      // 👇️ `fineClosure()` captures `blockScopedConstant` (and so couldn't move up).
      console.log(blockScopedConstant);
    };

    // 👇️ This renaming *shouldn't* make any difference (but it does).
    const sameBlockScopedConstant = blockScopedConstant;

    // ❌ Impossible suggestion: `Move arrow function 'buggyClosure' to the outer scope.` ❌
    const buggyClosure = (): void => {
      // 👇️ `buggyClosure()` captures `sameBlockScopedConstant` (and so couldn't move up).
      console.log(sameBlockScopedConstant);
    };
  }
};

Perhaps the only thing it contributes is the simple observation that aliasing blockScopedConstant triggers the bug (in this example).