Correctly transpile when cross-param refs with obj rest (#11326)

* Transform initializers with ids in rest elements

Fix issue 11281. Transform parameters with default initializers that
have ids that are also in a parameter with a rest element.
Before, these parameters were not transformed.

* Add plugin-transform-parameters as dependency

* Remove outdated comment

* Use set instead of array for paramsWithRestElement

* Skip when encounter "Scope"

Previously, f({...R}, f = R => R) would be incorrectly transformed.

* Pass in loose mode option instead of false

* Address review and re-organize tests

Checking the RHS of an assignment pattern/checking the parent of
an identifier node fails on cases like "({...R}, a = f(R))" or
"({...R}, {[R.key]: a = 42})".

Also refactor tests by removing unecessary tests and
separating "should transform" from "should not transform" tests.

packages/babel-plugin-proposal-object-rest-spread/package.json

@@ -13,7 +13,8 @@
   ],
   "dependencies": {
     "@babel/helper-plugin-utils": "^7.8.3",
-    "@babel/plugin-syntax-object-rest-spread": "^7.8.0"
+    "@babel/plugin-syntax-object-rest-spread": "^7.8.0",
+    "@babel/plugin-transform-parameters": "^7.9.3"
   },
   "peerDependencies": {
     "@babel/core": "^7.0.0-0"

packages/babel-plugin-proposal-object-rest-spread/src/index.js

@@ -1,6 +1,7 @@
 import { declare } from "@babel/helper-plugin-utils";
 import syntaxObjectRestSpread from "@babel/plugin-syntax-object-rest-spread";
 import { types as t } from "@babel/core";
+import { convertFunctionParams } from "@babel/plugin-transform-parameters";
 
 // TODO: Remove in Babel 8
 // @babel/types <=7.3.3 counts FOO as referenced in var { x: FOO }.
@@ -177,9 +178,9 @@ export default declare((api, opts) => {
     ];
   }
 
-  function replaceRestElement(parentPath, paramPath) {
+  function replaceRestElement(parentPath, paramPath, container) {
     if (paramPath.isAssignmentPattern()) {
-      replaceRestElement(parentPath, paramPath.get("left"));
+      replaceRestElement(parentPath, paramPath.get("left"), container);
       return;
     }
 
@@ -187,7 +188,7 @@ export default declare((api, opts) => {
       const elements = paramPath.get("elements");
 
       for (let i = 0; i < elements.length; i++) {
-        replaceRestElement(parentPath, elements[i]);
+        replaceRestElement(parentPath, elements[i], container);
       }
     }
 
@@ -198,8 +199,12 @@ export default declare((api, opts) => {
         t.variableDeclarator(paramPath.node, uid),
       ]);
 
-      parentPath.ensureBlock();
+      if (container) {
-      parentPath.get("body").unshiftContainer("body", declar);
+        container.push(declar);
+      } else {
+        parentPath.ensureBlock();
+        parentPath.get("body").unshiftContainer("body", declar);
+      }
       paramPath.replaceWith(t.cloneNode(uid));
     }
   }
@@ -209,12 +214,71 @@ export default declare((api, opts) => {
     inherits: syntaxObjectRestSpread,
 
     visitor: {
-      // taken from transform-parameters/src/destructuring.js
       // function a({ b, ...c }) {}
       Function(path) {
         const params = path.get("params");
-        for (let i = params.length - 1; i >= 0; i--) {
+        const paramsWithRestElement = new Set();
-          replaceRestElement(params[i].parentPath, params[i]);
+        const idsInRestParams = new Set();
+        for (let i = 0; i < params.length; ++i) {
+          const param = params[i];
+          if (hasRestElement(param)) {
+            paramsWithRestElement.add(i);
+            for (const name of Object.keys(param.getBindingIdentifiers())) {
+              idsInRestParams.add(name);
+            }
+          }
+        }
+
+        // if true, a parameter exists that has an id in its initializer
+        // that is also an id bound in a rest parameter
+        // example: f({...R}, a = R)
+        let idInRest = false;
+
+        const IdentifierHandler = function(path, functionScope) {
+          const name = path.node.name;
+          if (
+            path.scope.getBinding(name) === functionScope.getBinding(name) &&
+            idsInRestParams.has(name)
+          ) {
+            idInRest = true;
+            path.stop();
+          }
+        };
+
+        let i;
+        for (i = 0; i < params.length && !idInRest; ++i) {
+          const param = params[i];
+          if (!paramsWithRestElement.has(i)) {
+            if (param.isReferencedIdentifier() || param.isBindingIdentifier()) {
+              IdentifierHandler(path, path.scope);
+            } else {
+              param.traverse(
+                {
+                  "Scope|TypeAnnotation|TSTypeAnnotation": path => path.skip(),
+                  "ReferencedIdentifier|BindingIdentifier": IdentifierHandler,
+                },
+                path.scope,
+              );
+            }
+          }
+        }
+
+        if (!idInRest) {
+          for (let i = 0; i < params.length; ++i) {
+            const param = params[i];
+            if (paramsWithRestElement.has(i)) {
+              replaceRestElement(param.parentPath, param);
+            }
+          }
+        } else {
+          const shouldTransformParam = idx =>
+            idx >= i - 1 || paramsWithRestElement.has(idx);
+          convertFunctionParams(
+            path,
+            loose,
+            shouldTransformParam,
+            replaceRestElement,
+          );
         }
       },
       // adapted from transform-destructuring/src/index.js#pushObjectRest

packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-rest/parameters-extra/input.js

@@ -0,0 +1,9 @@
+({...R}, a = R) => {}
+({X: Y,...R}, {a = {Y}}) => {}
+(a = R, {...R}) => {}
+({...R}, e, c = 2, a = R, f = q) => { let q; }
+({...R}, a = f(R)) => {}
+({...R}, { [R.key]: a = 42 }) => {}
+
+({...R}, {a = {R: b}}) => {}
+({...R}, {a = R => R} = {b: R => R}) => {}

packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-rest/parameters-extra/output.js

@@ -0,0 +1,61 @@
+(_ref) => {
+  let R = babelHelpers.extends({}, _ref);
+  let a = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : R;
+};
+
+(_ref2, _ref3) => {
+  let {
+    X: Y
+  } = _ref2,
+      R = babelHelpers.objectWithoutProperties(_ref2, ["X"]);
+  let {
+    a = {
+      Y
+    }
+  } = _ref3;
+};
+
+() => {
+  let a = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : R;
+
+  let _ref4 = arguments.length > 1 ? arguments[1] : undefined;
+
+  let R = babelHelpers.extends({}, _ref4);
+};
+
+(_ref5, e, c = 2) => {
+  let R = babelHelpers.extends({}, _ref5);
+  let a = arguments.length > 3 && arguments[3] !== undefined ? arguments[3] : R;
+  let f = arguments.length > 4 && arguments[4] !== undefined ? arguments[4] : q;
+  return function () {
+    let q;
+  }();
+};
+
+(_ref6) => {
+  let R = babelHelpers.extends({}, _ref6);
+  let a = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : f(R);
+};
+
+(_ref7, _ref8) => {
+  let R = babelHelpers.extends({}, _ref7);
+  let {
+    [R.key]: a = 42
+  } = _ref8;
+};
+
+(_ref9, {
+  a = {
+    R: b
+  }
+}) => {
+  let R = babelHelpers.extends({}, _ref9);
+};
+
+(_ref10, {
+  a = R => R
+} = {
+  b: R => R
+}) => {
+  let R = babelHelpers.extends({}, _ref10);
+};

packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-rest/parameters/output.js

@@ -17,15 +17,15 @@ function a3(_ref3) {
       c2 = babelHelpers.objectWithoutProperties(_ref3, ["a2", "b2"]);
 }
 
-function a4(_ref5, _ref4) {
+function a4(_ref4, _ref5) {
-  let {
-    a3
-  } = _ref5,
-      c3 = babelHelpers.objectWithoutProperties(_ref5, ["a3"]);
   let {
     a5
+  } = _ref5,
+      c5 = babelHelpers.objectWithoutProperties(_ref5, ["a5"]);
+  let {
+    a3
   } = _ref4,
-      c5 = babelHelpers.objectWithoutProperties(_ref4, ["a5"]);
+      c3 = babelHelpers.objectWithoutProperties(_ref4, ["a3"]);
 }
 
 function a5(_ref6) {

packages/babel-plugin-transform-parameters/src/index.js

@@ -1,6 +1,7 @@
 import { declare } from "@babel/helper-plugin-utils";
 import convertFunctionParams from "./params";
 import convertFunctionRest from "./rest";
+export { convertFunctionParams };
 
 export default declare((api, options) => {
   api.assertVersion(7);

packages/babel-plugin-transform-parameters/src/params.js

@@ -41,7 +41,13 @@ const iifeVisitor = {
     path.skip(),
 };
 
-export default function convertFunctionParams(path, loose) {
+// last 2 parameters are optional -- they are used by proposal-object-rest-spread/src/index.js
+export default function convertFunctionParams(
+  path,
+  loose,
+  shouldTransformParam,
+  replaceRestElement,
+) {
   const params = path.get("params");
 
   const isSimpleParameterList = params.every(param => param.isIdentifier());
@@ -108,6 +114,14 @@ export default function convertFunctionParams(path, loose) {
   for (let i = 0; i < params.length; i++) {
     const param = params[i];
 
+    if (shouldTransformParam && !shouldTransformParam(i)) {
+      continue;
+    }
+    const transformedRestNodes = [];
+    if (replaceRestElement) {
+      replaceRestElement(param.parentPath, param, transformedRestNodes);
+    }
+
     const paramIsAssignmentPattern = param.isAssignmentPattern();
     if (paramIsAssignmentPattern && (loose || node.kind === "set")) {
       const left = param.get("left");
@@ -164,6 +178,12 @@ export default function convertFunctionParams(path, loose) {
 
       param.replaceWith(t.cloneNode(uid));
     }
+
+    if (transformedRestNodes) {
+      for (const transformedNode of transformedRestNodes) {
+        body.push(transformedNode);
+      }
+    }
   }
 
   // we need to cut off all trailing parameters