Inject IIFE when variables shadow binding in rest param

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

@@ -1,5 +1,11 @@
 import { template, types as t } from "@babel/core";
-import type { NodePath, Scope, Visitor } from "@babel/traverse";
+import type { NodePath } from "@babel/traverse";
+
+import {
+  iifeVisitor,
+  collectShadowedParamsNames,
+  buildScopeIIFE,
+} from "./shadow-utils";
 
 const buildDefaultParam = template.statement(`
   let VARIABLE_NAME =
@@ -23,34 +29,6 @@ const buildSafeArgumentsAccess = template.statement(`
   let $0 = arguments.length > $1 ? arguments[$1] : undefined;
 `);
 
-const iifeVisitor: Visitor<State> = {
-  "ReferencedIdentifier|BindingIdentifier"(
-    path: NodePath<t.Identifier>,
-    state,
-  ) {
-    const { scope, node } = path;
-    const { name } = node;
-
-    if (
-      name === "eval" ||
-      (scope.getBinding(name) === state.scope.parent.getBinding(name) &&
-        state.scope.hasOwnBinding(name))
-    ) {
-      state.needsOuterBinding = true;
-      path.stop();
-    }
-  },
-  // type annotations don't use or introduce "real" bindings
-  "TypeAnnotation|TSTypeAnnotation|TypeParameterDeclaration|TSTypeParameterDeclaration":
-    (path: NodePath) => path.skip(),
-};
-
-type State = {
-  stop: boolean;
-  needsOuterBinding: boolean;
-  scope: Scope;
-};
-
 // last 2 parameters are optional -- they are used by proposal-object-rest-spread/src/index.js
 export default function convertFunctionParams(
   path: NodePath<t.Function>,
@@ -69,53 +47,17 @@ export default function convertFunctionParams(
 
   const { node, scope } = path;
 
-  const state = {
-    stop: false,
-    needsOuterBinding: false,
-    scope,
-  };
-
   const body = [];
   const shadowedParams = new Set<string>();
 
   for (const param of params) {
-    for (const name of Object.keys(param.getBindingIdentifiers())) {
-      const constantViolations = scope.bindings[name]?.constantViolations;
-      if (constantViolations) {
-        for (const redeclarator of constantViolations) {
-          const node = redeclarator.node;
-          // If a constant violation is a var or a function declaration,
-          // we first check to see if it's a var without an init.
-          // If so, we remove that declarator.
-          // Otherwise, we have to wrap it in an IIFE.
-          switch (node.type) {
-            case "VariableDeclarator": {
-              if (node.init === null) {
-                const declaration = redeclarator.parentPath;
-                // The following uninitialized var declarators should not be removed
-                // for (var x in {})
-                // for (var x;;)
-                if (
-                  !declaration.parentPath.isFor() ||
-                  declaration.parentPath.get("body") === declaration
-                ) {
-                  redeclarator.remove();
-                  break;
-                }
-              }
-
-              shadowedParams.add(name);
-              break;
-            }
-            case "FunctionDeclaration":
-              shadowedParams.add(name);
-              break;
-          }
-        }
-      }
-    }
+    collectShadowedParamsNames(param, scope, shadowedParams);
   }
 
+  const state = {
+    needsOuterBinding: false,
+    scope,
+  };
   if (shadowedParams.size === 0) {
     for (const param of params) {
       if (!param.isIdentifier()) param.traverse(iifeVisitor, state);
@@ -213,12 +155,7 @@ export default function convertFunctionParams(
   path.ensureBlock();
 
   if (state.needsOuterBinding || shadowedParams.size > 0) {
-    body.push(
-      buildScopeIIFE(
-        shadowedParams,
-        (path.get("body") as NodePath<t.BlockStatement>).node,
-      ),
-    );
+    body.push(buildScopeIIFE(shadowedParams, path.node.body));
 
     path.set("body", t.blockStatement(body as t.Statement[]));
 
@@ -244,18 +181,3 @@ export default function convertFunctionParams(
 
   return true;
 }
-
-function buildScopeIIFE(shadowedParams: Set<string>, body: t.BlockStatement) {
-  const args = [];
-  const params = [];
-
-  for (const name of shadowedParams) {
-    // We create them twice; the other option is to use t.cloneNode
-    args.push(t.identifier(name));
-    params.push(t.identifier(name));
-  }
-
-  return t.returnStatement(
-    t.callExpression(t.arrowFunctionExpression(params, body), args),
-  );
-}

packages/babel-plugin-transform-parameters/src/rest.ts

@@ -1,6 +1,12 @@
 import { template, types as t } from "@babel/core";
 import type { NodePath, Visitor } from "@babel/traverse";
 
+import {
+  iifeVisitor,
+  collectShadowedParamsNames,
+  buildScopeIIFE,
+} from "./shadow-utils";
+
 const buildRest = template.statement(`
   for (var LEN = ARGUMENTS.length,
            ARRAY = new Array(ARRAY_LEN),
@@ -292,9 +298,35 @@ export default function convertFunctionRest(path: NodePath<t.Function>) {
   const { node, scope } = path;
   if (!hasRest(node)) return false;
 
-  let rest = (node.params.pop() as t.RestElement).argument as
-    | t.Pattern
-    | t.Identifier;
+  const restPath = path.get(
+    `params.${node.params.length - 1}.argument`,
+  ) as NodePath<t.Pattern | t.Identifier>;
+
+  if (!restPath.isIdentifier()) {
+    const shadowedParams = new Set<string>();
+    collectShadowedParamsNames(restPath, path.scope, shadowedParams);
+
+    let needsIIFE = shadowedParams.size > 0;
+    if (!needsIIFE) {
+      const state = {
+        needsOuterBinding: false,
+        scope,
+      };
+      restPath.traverse(iifeVisitor, state);
+      needsIIFE = state.needsOuterBinding;
+    }
+
+    if (needsIIFE) {
+      path.ensureBlock();
+      path.set(
+        "body",
+        t.blockStatement([buildScopeIIFE(shadowedParams, path.node.body)]),
+      );
+    }
+  }
+
+  let rest = restPath.node;
+  node.params.pop(); // This returns 'rest'
 
   if (t.isPattern(rest)) {
     const pattern = rest;

packages/babel-plugin-transform-parameters/src/shadow-utils.ts

@@ -0,0 +1,89 @@
+import { types as t } from "@babel/core";
+import type { NodePath, Scope, Visitor } from "@babel/traverse";
+
+type State = {
+  needsOuterBinding: boolean;
+  scope: Scope;
+};
+
+export const iifeVisitor: Visitor<State> = {
+  "ReferencedIdentifier|BindingIdentifier"(
+    path: NodePath<t.Identifier>,
+    state,
+  ) {
+    const { scope, node } = path;
+    const { name } = node;
+
+    if (
+      name === "eval" ||
+      (scope.getBinding(name) === state.scope.parent.getBinding(name) &&
+        state.scope.hasOwnBinding(name))
+    ) {
+      state.needsOuterBinding = true;
+      path.stop();
+    }
+  },
+  // type annotations don't use or introduce "real" bindings
+  "TypeAnnotation|TSTypeAnnotation|TypeParameterDeclaration|TSTypeParameterDeclaration":
+    (path: NodePath) => path.skip(),
+};
+
+export function collectShadowedParamsNames(
+  param: NodePath<t.Function["params"][number]>,
+  functionScope: Scope,
+  shadowedParams: Set<string>,
+) {
+  for (const name of Object.keys(param.getBindingIdentifiers())) {
+    const constantViolations = functionScope.bindings[name]?.constantViolations;
+    if (constantViolations) {
+      for (const redeclarator of constantViolations) {
+        const node = redeclarator.node;
+        // If a constant violation is a var or a function declaration,
+        // we first check to see if it's a var without an init.
+        // If so, we remove that declarator.
+        // Otherwise, we have to wrap it in an IIFE.
+        switch (node.type) {
+          case "VariableDeclarator": {
+            if (node.init === null) {
+              const declaration = redeclarator.parentPath;
+              // The following uninitialized var declarators should not be removed
+              // for (var x in {})
+              // for (var x;;)
+              if (
+                !declaration.parentPath.isFor() ||
+                declaration.parentPath.get("body") === declaration
+              ) {
+                redeclarator.remove();
+                break;
+              }
+            }
+
+            shadowedParams.add(name);
+            break;
+          }
+          case "FunctionDeclaration":
+            shadowedParams.add(name);
+            break;
+        }
+      }
+    }
+  }
+}
+
+export function buildScopeIIFE(
+  shadowedParams: Set<string>,
+  body: t.BlockStatement,
+) {
+  const args = [];
+  const params = [];
+
+  for (const name of shadowedParams) {
+    // We create them twice; the other option is to use t.cloneNode
+    args.push(t.identifier(name));
+    params.push(t.identifier(name));
+  }
+
+  return t.returnStatement(
+    t.callExpression(t.arrowFunctionExpression(params, body), args),
+  );
+}

packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param-2/input.js

@@ -0,0 +1,9 @@
+function f(...{ length: x = 0, y = x }) {
+  var x;
+  return x;
+}
+
+function g(...{ length: x = 0, y = x }) {
+  var x = 1;
+  return x;
+}

packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param-2/output.js

@@ -0,0 +1,26 @@
+function f() {
+  for (var _len = arguments.length, _ref = new Array(_len), _key = 0; _key < _len; _key++) {
+    _ref[_key] = arguments[_key];
+  }
+
+  var _ref$length = _ref.length,
+      x = _ref$length === void 0 ? 0 : _ref$length,
+      _ref$y = _ref.y,
+      y = _ref$y === void 0 ? x : _ref$y;
+  return x;
+}
+
+function g() {
+  for (var _len2 = arguments.length, _ref2 = new Array(_len2), _key2 = 0; _key2 < _len2; _key2++) {
+    _ref2[_key2] = arguments[_key2];
+  }
+
+  var _ref2$length = _ref2.length,
+      x = _ref2$length === void 0 ? 0 : _ref2$length,
+      _ref2$y = _ref2.y,
+      y = _ref2$y === void 0 ? x : _ref2$y;
+  return function (x) {
+    var x = 1;
+    return x;
+  }(x);
+}

packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param/input.js

@@ -0,0 +1,9 @@
+function f(...[x, y = x]) {
+  var x;
+  return x;
+}
+
+function g(...[x, y = x]) {
+  var x = 1;
+  return x;
+}

packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param/output.js

@@ -0,0 +1,24 @@
+function f() {
+  for (var _len = arguments.length, _ref = new Array(_len), _key = 0; _key < _len; _key++) {
+    _ref[_key] = arguments[_key];
+  }
+
+  var x = _ref[0],
+      _ref$ = _ref[1],
+      y = _ref$ === void 0 ? x : _ref$;
+  return x;
+}
+
+function g() {
+  for (var _len2 = arguments.length, _ref2 = new Array(_len2), _key2 = 0; _key2 < _len2; _key2++) {
+    _ref2[_key2] = arguments[_key2];
+  }
+
+  var x = _ref2[0],
+      _ref2$ = _ref2[1],
+      y = _ref2$ === void 0 ? x : _ref2$;
+  return function (x) {
+    var x = 1;
+    return x;
+  }(x);
+}

packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-rest-param/input.js

@@ -0,0 +1,9 @@
+function f(...x) {
+  var x;
+  return x;
+}
+
+function g(...x) {
+  var x = 1;
+  return x;
+}

packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-rest-param/output.js

@@ -0,0 +1,17 @@
+function f() {
+  for (var _len = arguments.length, x = new Array(_len), _key = 0; _key < _len; _key++) {
+    x[_key] = arguments[_key];
+  }
+
+  var x;
+  return x;
+}
+
+function g() {
+  for (var _len2 = arguments.length, x = new Array(_len2), _key2 = 0; _key2 < _len2; _key2++) {
+    x[_key2] = arguments[_key2];
+  }
+
+  var x = 1;
+  return x;
+}