Only mark vars directly inside loops

babel · Oct 7, 2022 · 0a21a22 · 0a21a22
1 parent 6404e8a
commit 0a21a22
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 38 deletions.
diff --git a/...-plugin-transform-destructuring/test/fixtures/assumption-iterableIsArray/for-in/output.js b/...-plugin-transform-destructuring/test/fixtures/assumption-iterableIsArray/for-in/output.js
@@ -5,7 +5,8 @@ for (var _ref in obj) {
 }
 
 for (var _ref2 in obj) {
-  name = _ref2[0];
-  value = _ref2[1];
+  var _ref3 = _ref2;
+  name = _ref3[0];
+  value = _ref3[1];
   print("Name: " + name + ", Value: " + value);
 }
diff --git a/packages/babel-plugin-transform-destructuring/test/fixtures/destructuring/for-in/output.js b/packages/babel-plugin-transform-destructuring/test/fixtures/destructuring/for-in/output.js
@@ -7,9 +7,11 @@ for (var _ref in obj) {
 }
 
 for (var _ref3 in obj) {
-  var _ref4 = babelHelpers.slicedToArray(_ref3, 2);
+  var _ref4 = _ref3;
 
-  name = _ref4[0];
-  value = _ref4[1];
+  var _ref5 = babelHelpers.slicedToArray(_ref4, 2);
+
+  name = _ref5[0];
+  value = _ref5[1];
   print("Name: " + name + ", Value: " + value);
 }
diff --git a/...ransform-destructuring/test/fixtures/destructuring/for-of-shadowed-block-scoped/output.js b/...ransform-destructuring/test/fixtures/destructuring/for-of-shadowed-block-scoped/output.js
@@ -13,7 +13,8 @@ for (const _ref of [O]) {
 var _;
 
 for (var _ref2 of [O]) {
-  _ = _ref2[a];
+  var _ref3 = _ref2;
+  _ = _ref3[a];
   {
     const a = "A";
   }

diff --git a/packages/babel-plugin-transform-destructuring/test/fixtures/destructuring/for-of/output.js b/packages/babel-plugin-transform-destructuring/test/fixtures/destructuring/for-of/output.js
@@ -7,10 +7,12 @@ for (var _ref of test.expectation.registers) {
 }
 
 for (var _ref3 of test.expectation.registers) {
-  var _ref4 = babelHelpers.slicedToArray(_ref3, 3);
+  var _ref4 = _ref3;
 
-  name = _ref4[0];
-  before = _ref4[1];
-  after = _ref4[2];
+  var _ref5 = babelHelpers.slicedToArray(_ref4, 3);
+
+  name = _ref5[0];
+  before = _ref5[1];
+  after = _ref5[2];
   void 0;
 }
diff --git a/packages/babel-traverse/src/scope/binding.ts b/packages/babel-traverse/src/scope/binding.ts
@@ -44,11 +44,8 @@ export default class Binding {
     this.path = path;
     this.kind = kind;
 
-    for (let node of path.getAncestry()) {
-      if (node.isLoop()) {
-        debugger;
-        this.constant = false;
-      }
+    if ((kind === "var" || kind === "hoisted") && isDeclaredInLoop(path)) {
+      this.reassign(path);
     }
 
     this.clearValue();
@@ -116,3 +113,21 @@ export default class Binding {
     this.referenced = !!this.references;
   }
 }
+
+function isDeclaredInLoop(path: NodePath) {
+  for (
+    let { parentPath, key } = path;
+    parentPath;
+    { parentPath, key } = parentPath
+  ) {
+    if (parentPath.isFunctionParent()) return false;
+    if (
+      parentPath.isWhile() ||
+      parentPath.isForXStatement() ||
+      (parentPath.isForStatement() && key === "body")
+    ) {
+      return true;
+    }
+  }
+  return false;
+}
diff --git a/packages/babel-traverse/test/scope.js b/packages/babel-traverse/test/scope.js
@@ -389,29 +389,44 @@ describe("scope", () => {
       expect(
         getPath("var a = 1; var a = 2;").scope.getBinding("a").constant,
       ).toBe(false);
-      expect(
-        getPath("for (var n of ns) { var a = 1; }").scope.getBinding("a")
-          .constant,
-      ).toBe(false);
-      expect(
-        getPath("for (var n in ns) { var a = 1; }").scope.getBinding("a")
-          .constant,
-      ).toBe(false);
-      expect(
-        getPath("for (var i = 0; i < n; i++) { var a = 1; }").scope.getBinding(
-          "a",
-        ).constant,
-      ).toBe(false);
-      expect(
-        getPath("var i = 0; while (i != 1) { var a = 1; }").scope.getBinding(
-          "a",
-        ).constant,
-      ).toBe(false);
-      expect(
-        getPath("var i = 0; do { var a = 1; } while (i != 1)").scope.getBinding(
-          "a",
-        ).constant,
-      ).toBe(false);
+    });
+
+    it("variable constantness in loops", () => {
+      let scopePath = null;
+      const isAConstant = code => {
+        let path = getPath(code);
+        if (scopePath) path = path.get(scopePath);
+        return path.scope.getBinding("a").constant;
+      };
+
+      expect(isAConstant("for (_ of ns) { var a = 1; }")).toBe(false);
+      expect(isAConstant("for (_ in ns) { var a = 1; }")).toBe(false);
+      expect(isAConstant("for (;;) { var a = 1; }")).toBe(false);
+      expect(isAConstant("while (1) { var a = 1; }")).toBe(false);
+      expect(isAConstant("do { var a = 1; } while (1)")).toBe(false);
+
+      expect(isAConstant("for (var a of ns) {}")).toBe(false);
+      expect(isAConstant("for (var a in ns) {}")).toBe(false);
+      expect(isAConstant("for (var a;;) {}")).toBe(true);
+
+      scopePath = "body.0.body.expression";
+      expect(isAConstant("for (_ of ns) () => { var a = 1; }")).toBe(true);
+      expect(isAConstant("for (_ in ns) () => { var a = 1; }")).toBe(true);
+      expect(isAConstant("for (;;) () => { var a = 1; }")).toBe(true);
+      expect(isAConstant("while (1) () => { var a = 1; }")).toBe(true);
+      expect(isAConstant("do () => { var a = 1; }; while (1)")).toBe(true);
+
+      scopePath = "body.0.body";
+      expect(isAConstant("for (_ of ns) { let a = 1; }")).toBe(true);
+      expect(isAConstant("for (_ in ns) { let a = 1; }")).toBe(true);
+      expect(isAConstant("for (;;) { let a = 1; }")).toBe(true);
+      expect(isAConstant("while (1) { let a = 1; }")).toBe(true);
+      expect(isAConstant("do { let a = 1; } while (1)")).toBe(true);
+
+      scopePath = "body.0";
+      expect(isAConstant("for (let a of ns) {}")).toBe(true);
+      expect(isAConstant("for (let a in ns) {}")).toBe(true);
+      expect(isAConstant("for (let a;;) {}")).toBe(true);
     });
 
     test("label", function () {