Fix for binding shadowing outer var with loop closure (#15309)

* Fix for binding shadowing outer var with loop closure

Fix updating of non-captured variable with loop closure

* Rename

Author: nicolo-ribaudo

packages/babel-plugin-transform-block-scoping/src/index.ts

@@ -71,22 +71,32 @@ export default declare((api, opts: Options) => {
 
           if (headPath && isBlockScoped(headPath.node)) {
             const names = Object.keys(headPath.getBindingIdentifiers());
+            const headScope = headPath.scope;
 
-            for (const name of names) {
+            for (let name of names) {
               if (bodyScope?.hasOwnBinding(name)) continue; // shadowed
 
-              let binding = headPath.scope.getOwnBinding(name);
+              let binding = headScope.getOwnBinding(name);
               if (!binding) {
-                headPath.scope.crawl();
-                binding = headPath.scope.getOwnBinding(name);
+                headScope.crawl();
+                binding = headScope.getOwnBinding(name);
               }
               const { usages, capturedInClosure, hasConstantViolations } =
                 getUsageInBody(binding, path);
 
               if (capturedInClosure) {
                 markNeedsBodyWrap();
                 captured.push(name);
+              } else if (headScope.parent.hasBinding(name)) {
+                // If the binding is not captured, there is no need
+                // of adding it to the closure param. However, rename
+                // it if it shadows an outer binding, because the
+                // closure will be moved to an outer level.
+                const newName = headScope.generateUid(name);
+                headPath.scope.rename(name, newName);
+                name = newName;
               }
+
               if (isForStatement && hasConstantViolations) {
                 updatedBindingsUsages.set(name, usages);
               }

packages/babel-plugin-transform-block-scoping/src/loop.ts

@@ -177,23 +177,21 @@ export function wrapLoopBody(
   const callArgs = [];
   const closureParams = [];
   const updater = [];
-  for (const name of captured) {
+  for (const [name, updatedUsage] of updatedBindingsUsages) {
     callArgs.push(t.identifier(name));
 
-    const updatedUsage = updatedBindingsUsages.get(name);
-    if (!updatedUsage) {
-      // Not updated, re-use the same name
-      closureParams.push(t.identifier(name));
-      continue;
-    }
-
     const innerName = loopPath.scope.generateUid(name);
     closureParams.push(t.identifier(innerName));
     updater.push(
       t.assignmentExpression("=", t.identifier(name), t.identifier(innerName)),
     );
     for (const path of updatedUsage) path.replaceWith(t.identifier(innerName));
   }
+  for (const name of captured) {
+    if (updatedBindingsUsages.has(name)) continue; // already injected
+    callArgs.push(t.identifier(name));
+    closureParams.push(t.identifier(name));
+  }
 
   const id = loopPath.scope.generateUid("loop");
   const fn = t.functionExpression(

packages/babel-plugin-transform-block-scoping/test/fixtures/general/for-variable-update-different-captured/exec.js

@@ -0,0 +1,9 @@
+let res = [];
+
+for (let i = 1; i < 6; i++) {
+  let y = i;
+  res.push((() => y)());
+  i++;
+}
+
+expect(res).toEqual([1, 3, 5]);

packages/babel-plugin-transform-block-scoping/test/fixtures/general/for-variable-update-different-captured/input.js

@@ -0,0 +1,5 @@
+for (let i = 4; i < 6; i++) {
+  let y = i;
+  () => y;
+  i++;
+}

packages/babel-plugin-transform-block-scoping/test/fixtures/general/for-variable-update-different-captured/output.js

@@ -0,0 +1,11 @@
+var _loop = function (_i) {
+  var y = _i;
+  (function () {
+    return y;
+  });
+  _i++;
+  i = _i;
+};
+for (var i = 4; i < 6; i++) {
+  _loop(i);
+}

packages/babel-plugin-transform-block-scoping/test/fixtures/general/issue-15308-for-variable-shadow-and-capture/exec.js

@@ -0,0 +1,8 @@
+let res = [];
+
+let i = 0;
+for (let i = 4; i < 6; i++) {
+  res.push((() => i)());
+}
+
+expect(res).toEqual([4, 5]);

packages/babel-plugin-transform-block-scoping/test/fixtures/general/issue-15308-for-variable-shadow-and-capture/input.js

@@ -0,0 +1,4 @@
+let i;
+for (let i = 4; i < 6; i++) {
+  () => i;
+}

packages/babel-plugin-transform-block-scoping/test/fixtures/general/issue-15308-for-variable-shadow-and-capture/output.js

@@ -0,0 +1,9 @@
+var i;
+var _loop = function (i) {
+  (function () {
+    return i;
+  });
+};
+for (var _i = 4; _i < 6; _i++) {
+  _loop(_i);
+}

packages/babel-plugin-transform-block-scoping/test/fixtures/general/issue-15308-for-variable-shadow-and-update/exec.js

@@ -0,0 +1,10 @@
+let res = [];
+
+let i = 0;
+for (let i = 1; i < 6; i++) {
+  let y = i;
+  res.push((() => y)());
+  i++;
+}
+
+expect(res).toEqual([1, 3, 5]);

packages/babel-plugin-transform-block-scoping/test/fixtures/general/issue-15308-for-variable-shadow-and-update/input.js

@@ -0,0 +1,6 @@
+let i;
+for (let i = 4; i < 6; i++) {
+  let y = i;
+  () => y;
+  i++;
+}