fix object rest in array pattern (#10275)

* fix object rest in array pattern * update test fixtures * early return * use path.stop() at the right path
babel · Sep 10, 2019 · 8027dca · 8027dca
1 parent 8183103
commit 8027dca
Show file tree

Hide file tree

Showing 9 changed files with 179 additions and 30 deletions.
diff --git a/packages/babel-plugin-proposal-object-rest-spread/src/index.js b/packages/babel-plugin-proposal-object-rest-spread/src/index.js
@@ -30,9 +30,20 @@ export default declare((api, opts) => {
 
   function hasRestElement(path) {
     let foundRestElement = false;
-    visitRestElements(path, () => {
+    visitRestElements(path, restElement => {
       foundRestElement = true;
-      path.stop();
+      restElement.stop();
+    });
+    return foundRestElement;
+  }
+
+  function hasObjectPatternRestElement(path) {
+    let foundRestElement = false;
+    visitRestElements(path, restElement => {
+      if (restElement.parentPath.isObjectPattern()) {
+        foundRestElement = true;
+        restElement.stop();
+      }
     });
     return foundRestElement;
   }
@@ -163,17 +174,17 @@ export default declare((api, opts) => {
     ];
   }
 
-  function replaceRestElement(parentPath, paramPath, i, numParams) {
+  function replaceRestElement(parentPath, paramPath) {
     if (paramPath.isAssignmentPattern()) {
-      replaceRestElement(parentPath, paramPath.get("left"), i, numParams);
+      replaceRestElement(parentPath, paramPath.get("left"));
       return;
     }
 
     if (paramPath.isArrayPattern() && hasRestElement(paramPath)) {
       const elements = paramPath.get("elements");
 
       for (let i = 0; i < elements.length; i++) {
-        replaceRestElement(parentPath, elements[i], i, elements.length);
+        replaceRestElement(parentPath, elements[i]);
       }
     }
 
@@ -200,7 +211,7 @@ export default declare((api, opts) => {
       Function(path) {
         const params = path.get("params");
         for (let i = params.length - 1; i >= 0; i--) {
-          replaceRestElement(params[i].parentPath, params[i], i, params.length);
+          replaceRestElement(params[i].parentPath, params[i]);
         }
       },
       // adapted from transform-destructuring/src/index.js#pushObjectRest
@@ -380,8 +391,12 @@ export default declare((api, opts) => {
         const leftPath = path.get("left");
         const left = node.left;
 
-        // for ({a, ...b} of []) {}
-        if (t.isObjectPattern(left) && hasRestElement(leftPath)) {
+        if (!hasObjectPatternRestElement(leftPath)) {
+          return;
+        }
+
+        if (!t.isVariableDeclaration(left)) {
+          // for ({a, ...b} of []) {}
           const temp = scope.generateUidIdentifier("ref");
 
           node.left = t.variableDeclaration("var", [
@@ -401,27 +416,54 @@ export default declare((api, opts) => {
               t.assignmentExpression("=", left, t.cloneNode(temp)),
             ),
           );
+        } else {
+          // for (var {a, ...b} of []) {}
+          const pattern = left.declarations[0].id;
 
-          return;
+          const key = scope.generateUidIdentifier("ref");
+          node.left = t.variableDeclaration(left.kind, [
+            t.variableDeclarator(key, null),
+          ]);
+
+          path.ensureBlock();
+
+          node.body.body.unshift(
+            t.variableDeclaration(node.left.kind, [
+              t.variableDeclarator(pattern, t.cloneNode(key)),
+            ]),
+          );
         }
+      },
+      // [{a, ...b}] = c;
+      ArrayPattern(path) {
+        const objectPatterns = [];
 
-        if (!t.isVariableDeclaration(left)) return;
+        visitRestElements(path, path => {
+          if (!path.parentPath.isObjectPattern()) {
+            // Return early if the parent is not an ObjectPattern, but
+            // (for example) an ArrayPattern or Function, because that
+            // means this RestElement is an not an object property.
+            return;
+          }
 
-        const pattern = left.declarations[0].id;
-        if (!t.isObjectPattern(pattern)) return;
+          const objectPattern = path.parentPath;
 
-        const key = scope.generateUidIdentifier("ref");
-        node.left = t.variableDeclaration(left.kind, [
-          t.variableDeclarator(key, null),
-        ]);
+          const uid = path.scope.generateUidIdentifier("ref");
+          objectPatterns.push(t.variableDeclarator(objectPattern.node, uid));
 
-        path.ensureBlock();
+          objectPattern.replaceWith(t.cloneNode(uid));
+          path.skip();
+        });
 
-        node.body.body.unshift(
-          t.variableDeclaration(node.left.kind, [
-            t.variableDeclarator(pattern, t.cloneNode(key)),
-          ]),
-        );
+        if (objectPatterns.length > 0) {
+          const statementPath = path.getStatementParent();
+          statementPath.insertAfter(
+            t.variableDeclaration(
+              statementPath.node.kind || "var",
+              objectPatterns,
+            ),
+          );
+        }
       },
       // var a = { ...b, ...c }
       ObjectExpression(path, file) {

diff --git a/...plugin-proposal-object-rest-spread/test/fixtures/object-rest/for-x-array-pattern/input.js b/...plugin-proposal-object-rest-spread/test/fixtures/object-rest/for-x-array-pattern/input.js
@@ -0,0 +1,19 @@
+// ForXStatement
+for (const [{a, ...b}] of []) {}
+for ([{a, ...b}] of []) {}
+async function a() {
+  for await ([{a, ...b}] of []) {}
+}
+
+// skip
+for ([{a}] in {}) {}
+for ([{a}] of []) {}
+async function a() {
+  for await ([{a}] of []) {}
+}
+
+for ([a, ...b] in {}) {}
+for ([a, ...b] of []) {}
+async function a() {
+  for await ([a, ...b] of []) {}
+}
diff --git a/...lugin-proposal-object-rest-spread/test/fixtures/object-rest/for-x-array-pattern/output.js b/...lugin-proposal-object-rest-spread/test/fixtures/object-rest/for-x-array-pattern/output.js
@@ -0,0 +1,49 @@
+// ForXStatement
+for (const _ref of []) {
+  const [_ref2] = _ref;
+  const {
+    a
+  } = _ref2,
+        b = babelHelpers.objectWithoutProperties(_ref2, ["a"]);
+}
+
+for (var _ref3 of []) {
+  [_ref4] = _ref3;
+  var {
+    a
+  } = _ref4,
+      b = babelHelpers.objectWithoutProperties(_ref4, ["a"]);
+}
+
+async function a() {
+  for await (var _ref5 of []) {
+    [_ref6] = _ref5;
+    var {
+      a
+    } = _ref6,
+        b = babelHelpers.objectWithoutProperties(_ref6, ["a"]);
+  }
+} // skip
+
+
+for ([{
+  a
+}] in {}) {}
+
+for ([{
+  a
+}] of []) {}
+
+async function a() {
+  for await ([{
+    a
+  }] of []) {}
+}
+
+for ([a, ...b] in {}) {}
+
+for ([a, ...b] of []) {}
+
+async function a() {
+  for await ([a, ...b] of []) {}
+}
diff --git a/...abel-plugin-proposal-object-rest-spread/test/fixtures/object-rest/nested-array-2/input.js b/...abel-plugin-proposal-object-rest-spread/test/fixtures/object-rest/nested-array-2/input.js
@@ -0,0 +1 @@
+const [a, [{b, ...c}], {d, ...e}, [{ f, ...g}, {h: [i, {j, ...k}] }]] = x;
diff --git a/...bel-plugin-proposal-object-rest-spread/test/fixtures/object-rest/nested-array-2/output.js b/...bel-plugin-proposal-object-rest-spread/test/fixtures/object-rest/nested-array-2/output.js
@@ -0,0 +1,19 @@
+const [a, [_ref], _ref2, [_ref3, {
+  h: [i, _ref4]
+}]] = x;
+const {
+  b
+} = _ref,
+      c = babelHelpers.objectWithoutProperties(_ref, ["b"]),
+      {
+  d
+} = _ref2,
+      e = babelHelpers.objectWithoutProperties(_ref2, ["d"]),
+      {
+  f
+} = _ref3,
+      g = babelHelpers.objectWithoutProperties(_ref3, ["f"]),
+      {
+  j
+} = _ref4,
+      k = babelHelpers.objectWithoutProperties(_ref4, ["j"]);
diff --git a/.../babel-plugin-proposal-object-rest-spread/test/fixtures/object-rest/nested-array/input.js b/.../babel-plugin-proposal-object-rest-spread/test/fixtures/object-rest/nested-array/input.js
@@ -0,0 +1,6 @@
+const [a, {b, ...c}] = x;
+
+let [d, {e, ...f}] = x;
+
+[g, {h, ...i}] = x;
+
diff --git a/...babel-plugin-proposal-object-rest-spread/test/fixtures/object-rest/nested-array/output.js b/...babel-plugin-proposal-object-rest-spread/test/fixtures/object-rest/nested-array/output.js
@@ -0,0 +1,15 @@
+const [a, _ref] = x;
+const {
+  b
+} = _ref,
+      c = babelHelpers.objectWithoutProperties(_ref, ["b"]);
+let [d, _ref2] = x;
+let {
+  e
+} = _ref2,
+    f = babelHelpers.objectWithoutProperties(_ref2, ["e"]);
+[g, _ref3] = x;
+var {
+  h
+} = _ref3,
+    i = babelHelpers.objectWithoutProperties(_ref3, ["h"]);
diff --git a/packages/babel-preset-env/test/fixtures/corejs2/usage-for-of-destructure-with/output.mjs b/packages/babel-preset-env/test/fixtures/corejs2/usage-for-of-destructure-with/output.mjs
@@ -1,9 +1,8 @@
 import "core-js/modules/es7.string.pad-end";
 import "core-js/modules/es7.string.pad-start";
 
-for (const _ref of foo) {
-  const {
-    padStart
-  } = _ref;
+for (const {
+  padStart
+} of foo) {
   console.log('b'.padEnd(5));
 }
diff --git a/packages/babel-preset-env/test/fixtures/corejs3/usage-for-of-destructure-with/output.mjs b/packages/babel-preset-env/test/fixtures/corejs3/usage-for-of-destructure-with/output.mjs
@@ -2,9 +2,8 @@ import "core-js/modules/es.array.iterator";
 import "core-js/modules/es.string.pad-end";
 import "core-js/modules/es.string.pad-start";
 
-for (const _ref of foo) {
-  const {
-    padStart
-  } = _ref;
+for (const {
+  padStart
+} of foo) {
   console.log('b'.padEnd(5));
 }