fix(pluginutils): attach scope object to for-loop (#616)

* fix(pluginutils): attach scope object to for-loop

* fix: types, add estree to devdependencies

* refactor: move estree import to the top

packages/pluginutils/package.json

@@ -48,14 +48,14 @@
     "rollup": "^1.20.0||^2.0.0"
   },
   "dependencies": {
-    "@types/estree": "0.0.45",
     "estree-walker": "^2.0.1",
     "picomatch": "^2.2.2"
   },
   "devDependencies": {
     "@rollup/plugin-commonjs": "^14.0.0",
     "@rollup/plugin-node-resolve": "^8.4.0",
     "@rollup/plugin-typescript": "^5.0.2",
+    "@types/estree": "0.0.45",
     "@types/node": "^14.0.26",
     "@types/picomatch": "^2.2.1",
     "acorn": "^8.0.4",

packages/pluginutils/src/attachScopes.ts

@@ -1,3 +1,6 @@
+// eslint-disable-next-line import/no-unresolved
+import * as estree from 'estree';
+
 import { walk } from 'estree-walker';
 
 import { AttachedScope, AttachScopes } from '../types';
@@ -16,7 +19,7 @@ const blockDeclarations: BlockDeclaration = {
 interface ScopeOptions {
   parent?: AttachedScope;
   block?: boolean;
-  params?: import('estree').Node[];
+  params?: estree.Node[];
 }
 
 class Scope implements AttachedScope {
@@ -39,7 +42,7 @@ class Scope implements AttachedScope {
     }
   }
 
-  addDeclaration(node: import('estree').Node, isBlockDeclaration: boolean, isVar: boolean): void {
+  addDeclaration(node: estree.Node, isBlockDeclaration: boolean, isVar: boolean): void {
     if (!isBlockDeclaration && this.isBlockScope) {
       // it's a `var` or function node, and this
       // is a block scope, so we need to go up
@@ -61,7 +64,7 @@ const attachScopes: AttachScopes = function attachScopes(ast, propertyName = 'sc
 
   walk(ast, {
     enter(n, parent) {
-      const node = n as import('estree').Node;
+      const node = n as estree.Node;
       // function foo () {...}
       // class Foo {...}
       if (/(Function|Class)Declaration/.test(node.type)) {
@@ -72,20 +75,16 @@ const attachScopes: AttachScopes = function attachScopes(ast, propertyName = 'sc
       if (node.type === 'VariableDeclaration') {
         const { kind } = node;
         const isBlockDeclaration = blockDeclarations[kind];
-        // don't add const/let declarations in the body of a for loop #113
-        const parentType = parent ? parent.type : '';
-        if (!(isBlockDeclaration && /ForOfStatement/.test(parentType))) {
-          node.declarations.forEach((declaration) => {
-            scope.addDeclaration(declaration, isBlockDeclaration, true);
-          });
-        }
+        node.declarations.forEach((declaration) => {
+          scope.addDeclaration(declaration, isBlockDeclaration, true);
+        });
       }
 
       let newScope: AttachedScope | undefined;
 
       // create new function scope
       if (/Function/.test(node.type)) {
-        const func = node as import('estree').Function;
+        const func = node as estree.Function;
         newScope = new Scope({
           parent: scope,
           block: false,
@@ -99,6 +98,14 @@ const attachScopes: AttachScopes = function attachScopes(ast, propertyName = 'sc
         }
       }
 
+      // create new for scope
+      if (/For(In|Of)?Statement/.test(node.type)) {
+        newScope = new Scope({
+          parent: scope,
+          block: true
+        });
+      }
+
       // create new block scope
       if (node.type === 'BlockStatement' && !/Function/.test(parent.type)) {
         newScope = new Scope({
@@ -126,7 +133,7 @@ const attachScopes: AttachScopes = function attachScopes(ast, propertyName = 'sc
       }
     },
     leave(n) {
-      const node = n as import('estree').Node & Record<string, any>;
+      const node = n as estree.Node & Record<string, any>;
       if (node[propertyName]) scope = scope.parent!;
     }
   });

packages/pluginutils/test/attachScopes.ts

@@ -1,7 +1,10 @@
+// eslint-disable-next-line import/no-unresolved
+import * as estree from 'estree';
+
 import test from 'ava';
 import { parse } from 'acorn';
 
-import { attachScopes } from '../';
+import { attachScopes, AttachedScope } from '../';
 
 test('attaches a scope to the top level', (t) => {
   const ast = parse('var foo;', { ecmaVersion: 2020, sourceType: 'module' });
@@ -70,3 +73,76 @@ test('supports catch without a parameter', (t) => {
     attachScopes(ast, 'scope');
   });
 });
+
+test('supports ForStatement', (t) => {
+  const ast = (parse(
+    `
+    for (let a = 0; a < 10; a++) {
+      console.log(a);
+      let b = 10;
+    }
+  `,
+    { ecmaVersion: 2020, sourceType: 'module' }
+  ) as unknown) as estree.Program;
+
+  const scope = attachScopes(ast, 'scope');
+  t.falsy(scope.contains('a'));
+  t.falsy(scope.contains('b'));
+
+  const forLoop = ast.body[0] as estree.ForStatement & { scope: AttachedScope };
+
+  t.truthy(forLoop.scope.contains('a'));
+  t.falsy(forLoop.scope.contains('b'));
+
+  const forBody = forLoop.body as estree.BlockStatement & { scope: AttachedScope };
+  t.truthy(forBody.scope.contains('a'));
+  t.truthy(forBody.scope.contains('b'));
+});
+
+test('supports ForOfStatement', (t) => {
+  const ast = (parse(
+    `
+    for (const a of [1, 2, 3]) {
+      console.log(a);
+      let b = 10;
+    }
+  `,
+    { ecmaVersion: 2020, sourceType: 'module' }
+  ) as unknown) as estree.Program;
+
+  const scope = attachScopes(ast, 'scope');
+  t.falsy(scope.contains('a'));
+  t.falsy(scope.contains('b'));
+
+  const forLoop = ast.body[0] as estree.ForOfStatement & { scope: AttachedScope };
+  t.truthy(forLoop.scope.contains('a'));
+  t.falsy(forLoop.scope.contains('b'));
+
+  const forBody = forLoop.body as estree.BlockStatement & { scope: AttachedScope };
+  t.truthy(forBody.scope.contains('a'));
+  t.truthy(forBody.scope.contains('b'));
+});
+
+test('supports ForInStatement', (t) => {
+  const ast = (parse(
+    `
+    for (let a in [1, 2, 3]) {
+      console.log(a);
+      let b = 10;
+    }
+  `,
+    { ecmaVersion: 2020, sourceType: 'module' }
+  ) as unknown) as estree.Program;
+
+  const scope = attachScopes(ast, 'scope');
+  t.falsy(scope.contains('a'));
+  t.falsy(scope.contains('b'));
+
+  const forLoop = ast.body[0] as estree.ForInStatement & { scope: AttachedScope };
+  t.truthy(forLoop.scope.contains('a'));
+  t.falsy(forLoop.scope.contains('b'));
+
+  const forBody = forLoop.body as estree.BlockStatement & { scope: AttachedScope };
+  t.truthy(forBody.scope.contains('a'));
+  t.truthy(forBody.scope.contains('b'));
+});