Define default value for vars shadowing params (#11307)

* Define default value for vars shadowing params

* Remove from package.json

* Only convert traverse the params if needed

This prevents this plugin from running after the destructuring
transform, which causes #11256

* Review

* Review

* Update packages/babel-plugin-transform-parameters/src/params.js [skip ci]

Co-Authored-By: Brian Ng <bng412@gmail.com>

Co-authored-by: Brian Ng <bng412@gmail.com>

packages/babel-plugin-transform-parameters/package.json

@@ -9,7 +9,6 @@
   },
   "main": "lib/index.js",
   "dependencies": {
-    "@babel/helper-call-delegate": "^7.8.7",
     "@babel/helper-get-function-arity": "^7.8.3",
     "@babel/helper-plugin-utils": "^7.8.3"
   },

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

@@ -1,4 +1,3 @@
-import callDelegate from "@babel/helper-call-delegate";
 import { template, types as t } from "@babel/core";
 
 const buildDefaultParam = template(`
@@ -23,47 +22,40 @@ const buildSafeArgumentsAccess = template(`
   let $0 = arguments.length > $1 ? arguments[$1] : undefined;
 `);
 
-function isSafeBinding(scope, node) {
-  if (!scope.hasOwnBinding(node.name)) return true;
-  const { kind } = scope.getOwnBinding(node.name);
-  return kind === "param" || kind === "local";
-}
-
 const iifeVisitor = {
-  ReferencedIdentifier(path, state) {
+  "ReferencedIdentifier|BindingIdentifier"(path, state) {
     const { scope, node } = path;
+    const { name } = node;
+
     if (
-      node.name === "eval" ||
-      !isSafeBinding(scope, node) ||
-      !isSafeBinding(state.scope, node)
+      name === "eval" ||
+      (scope.getBinding(name) === state.scope.parent.getBinding(name) &&
+        state.scope.hasOwnBinding(name))
     ) {
-      state.iife = true;
+      state.needsOuterBinding = true;
       path.stop();
     }
   },
-
-  Scope(path) {
-    // different bindings
-    path.skip();
-  },
 };
 
 export default function convertFunctionParams(path, loose) {
+  const params = path.get("params");
+
+  const isSimpleParameterList = params.every(param => param.isIdentifier());
+  if (isSimpleParameterList) return false;
+
   const { node, scope } = path;
 
   const state = {
-    iife: false,
-    scope: scope,
+    stop: false,
+    needsOuterBinding: false,
+    scope,
   };
 
   const body = [];
-  const params = path.get("params");
-
-  let firstOptionalIndex = null;
-
-  for (let i = 0; i < params.length; i++) {
-    const param = params[i];
+  const shadowedParams = new Set();
 
+  for (const param of params) {
     for (const name of Object.keys(param.getBindingIdentifiers())) {
       const constantViolations = scope.bindings[name]?.constantViolations;
       if (constantViolations) {
@@ -74,7 +66,7 @@ export default function convertFunctionParams(path, loose) {
           // If so, we remove that declarator.
           // Otherwise, we have to wrap it in an IIFE.
           switch (node.type) {
-            case "VariableDeclarator":
+            case "VariableDeclarator": {
               if (node.init === null) {
                 const declaration = redeclarator.parentPath;
                 // The following uninitialized var declarators should not be removed
@@ -88,14 +80,30 @@ export default function convertFunctionParams(path, loose) {
                   break;
                 }
               }
-            // fall through
+
+              shadowedParams.add(name);
+              break;
+            }
             case "FunctionDeclaration":
-              state.iife = true;
+              shadowedParams.add(name);
               break;
           }
         }
       }
     }
+  }
+
+  if (shadowedParams.size === 0) {
+    for (const param of params) {
+      if (!param.isIdentifier()) param.traverse(iifeVisitor, state);
+      if (state.needsOuterBinding) break;
+    }
+  }
+
+  let firstOptionalIndex = null;
+
+  for (let i = 0; i < params.length; i++) {
+    const param = params[i];
 
     const paramIsAssignmentPattern = param.isAssignmentPattern();
     if (paramIsAssignmentPattern && (loose || node.kind === "set")) {
@@ -131,15 +139,6 @@ export default function convertFunctionParams(path, loose) {
       const left = param.get("left");
       const right = param.get("right");
 
-      if (!state.iife) {
-        if (right.isIdentifier() && !isSafeBinding(scope, right.node)) {
-          // the right hand side references a parameter
-          state.iife = true;
-        } else {
-          right.traverse(iifeVisitor, state);
-        }
-      }
-
       const defNode = buildDefaultParam({
         VARIABLE_NAME: left.node,
         DEFAULT_VALUE: right.node,
@@ -162,13 +161,7 @@ export default function convertFunctionParams(path, loose) {
 
       param.replaceWith(t.cloneNode(uid));
     }
-
-    if (!state.iife && !param.isIdentifier()) {
-      param.traverse(iifeVisitor, state);
   }
-  }
-
-  if (body.length === 0) return false;
 
   // we need to cut off all trailing parameters
   if (firstOptionalIndex !== null) {
@@ -178,13 +171,34 @@ export default function convertFunctionParams(path, loose) {
   // ensure it's a block, useful for arrow functions
   path.ensureBlock();
 
-  if (state.iife) {
-    // we don't want to hoist the inner declarations up
-    body.push(callDelegate(path, scope, false));
+  if (state.needsOuterBinding || shadowedParams.size > 0) {
+    body.push(buildScopeIIFE(shadowedParams, path.get("body").node));
+
     path.set("body", t.blockStatement(body));
+
+    // We inject an arrow and then transform it to a normal function, to be
+    // sure that we correctly handle this and arguments.
+    const bodyPath = path.get("body.body");
+    const arrowPath = bodyPath[bodyPath.length - 1].get("argument.callee");
+    arrowPath.arrowFunctionToExpression();
   } else {
     path.get("body").unshiftContainer("body", body);
   }
 
   return true;
 }
+
+function buildScopeIIFE(shadowedParams, body) {
+  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), params),
+  );
+}

packages/babel-plugin-transform-parameters/test/fixtures/parameters/fn-decl-same-as-param/output.js

@@ -1,6 +1,6 @@
 function foo() {
   var a = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : 2;
-  return function () {
+  return function (a) {
     function a() {}
-  }();
+  }(a);
 }

packages/babel-plugin-transform-parameters/test/fixtures/parameters/iife-this/output.js

@@ -10,11 +10,13 @@ var Test = /*#__PURE__*/function () {
   babelHelpers.createClass(Test, [{
     key: "invite",
     value: function invite() {
+      var _this = this;
+
       var p = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : a;
       return function () {
         var a;
-        this;
-      }.apply(this);
+        _this;
+      }();
     }
   }]);
   return Test;

packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-no-init-for-init-same-as-param/exec.js

@@ -1,5 +1,5 @@
 function foo(a = 2) {
   for (var a, i = 0; i < 1; i++);
-  expect(a).toBe(undefined);
+  expect(a).toBe(2);
 }
 foo();

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

@@ -1,8 +1,8 @@
 function f() {
   var a = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : 2;
   var b = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : 3;
-  return function () {
+  return function (b) {
     var b = 4;
     return a + b;
-  }();
+  }(b);
 }

packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-no-init-then-var-with-init-same-as-param/output.js

@@ -1,11 +1,11 @@
 function f() {
   var a = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : 2;
   var b = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : 3;
-  return function () {
+  return function (a) {
     var _a = {
       a: 4
     },
         a = _a.a;
     return a + b;
-  }();
+  }(a);
 }

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

@@ -1,9 +1,9 @@
 function foo(_ref) {
   var a = _ref.a,
       b = _ref.b;
-  return function () {
+  return function (a) {
     var a = 3;
     var c = 2;
     var d = a + b + c;
-  }();
+  }(a);
 }

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

@@ -3,9 +3,9 @@ function foo(_ref) {
       a = _ref2[0],
       b = _ref2[1];
 
-  return function () {
+  return function (a) {
     var a = 3;
     var c = 2;
     var d = a + b + c;
-  }();
+  }(a);
 }

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

@@ -1,6 +1,6 @@
 function foo() {
   var a = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : 2;
-  return function () {
+  return function (a) {
     var a = 1;
-  }();
+  }(a);
 }

packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-with-init-then-var-no-init-same-as-param/output.js

@@ -1,11 +1,11 @@
 function f() {
   var a = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : 2;
   var b = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : 3;
-  return function () {
+  return function (a) {
     var _a = {
       a: 4
     },
         a = _a.a;
     return a + b;
-  }();
+  }(a);
 }

packages/babel-plugin-transform-parameters/test/fixtures/regression/11231/output.mjs

@@ -1,6 +1,6 @@
 (function () {
   let i = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : "__proto__";
-  return function () {
+  return function (i) {
     for (var i in {});
-  }();
+  }(i);
 });

packages/babel-plugin-transform-parameters/test/fixtures/regression/11256/input.js

@@ -0,0 +1,7 @@
+class A {
+  a = b => {
+    {
+      ({ b } = {});
+    }
+  };
+}

packages/babel-plugin-transform-parameters/test/fixtures/regression/11256/options.json

@@ -0,0 +1,8 @@
+{
+  "plugins": [
+    "proposal-class-properties",
+    "transform-parameters",
+    "transform-destructuring",
+    ["external-helpers", { "helperVersion": "7.100.0" }]
+  ]
+}

packages/babel-plugin-transform-parameters/test/fixtures/regression/11256/output.js

@@ -0,0 +1,11 @@
+class A {
+  constructor() {
+    babelHelpers.defineProperty(this, "a", b => {
+      {
+        var _ref = {};
+        b = _ref.b;
+      }
+    });
+  }
+
+}

packages/babel-plugin-transform-parameters/test/fixtures/regression/11303/exec.js

@@ -0,0 +1,6 @@
+function test(a, b = 1) {
+  var a = a + b;
+  return a;
+}
+
+expect(test(2)).toBe(3);

packages/babel-plugin-transform-parameters/test/fixtures/regression/11303/input.js

@@ -0,0 +1,4 @@
+function test(a, b = 1) {
+  var a = a + b;
+  return a;
+}

packages/babel-plugin-transform-parameters/test/fixtures/regression/11303/options.json

@@ -0,0 +1,5 @@
+{
+  "plugins": [
+    "transform-parameters"
+  ]
+}

packages/babel-plugin-transform-parameters/test/fixtures/regression/11303/output.js

@@ -0,0 +1,7 @@
+function test(a) {
+  let b = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : 1;
+  return function (a) {
+    var a = a + b;
+    return a;
+  }(a);
+}