From f2347e42fd72180ed04fdc155b3e47d49411a472 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Fri, 8 Jul 2022 09:46:05 +0200 Subject: [PATCH] Inject IIFE when variables shadow binding in rest param (#14736) --- .../src/params.ts | 104 +++--------------- .../src/rest.ts | 38 ++++++- .../src/shadow-utils.ts | 89 +++++++++++++++ .../input.js | 9 ++ .../output.js | 26 +++++ .../input.js | 9 ++ .../output.js | 24 ++++ .../var-same-name-as-rest-param/input.js | 9 ++ .../var-same-name-as-rest-param/output.js | 17 +++ 9 files changed, 231 insertions(+), 94 deletions(-) create mode 100644 packages/babel-plugin-transform-parameters/src/shadow-utils.ts create mode 100644 packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param-2/input.js create mode 100644 packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param-2/output.js create mode 100644 packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param/input.js create mode 100644 packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param/output.js create mode 100644 packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-rest-param/input.js create mode 100644 packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-rest-param/output.js diff --git a/packages/babel-plugin-transform-parameters/src/params.ts b/packages/babel-plugin-transform-parameters/src/params.ts index 2ed848779811..b93351baf100 100644 --- a/packages/babel-plugin-transform-parameters/src/params.ts +++ b/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 = { - "ReferencedIdentifier|BindingIdentifier"( - path: NodePath, - 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, @@ -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(); 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).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, 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), - ); -} diff --git a/packages/babel-plugin-transform-parameters/src/rest.ts b/packages/babel-plugin-transform-parameters/src/rest.ts index 7087a936bac2..e3d04d747844 100644 --- a/packages/babel-plugin-transform-parameters/src/rest.ts +++ b/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) { 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; + + if (!restPath.isIdentifier()) { + const shadowedParams = new Set(); + 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; diff --git a/packages/babel-plugin-transform-parameters/src/shadow-utils.ts b/packages/babel-plugin-transform-parameters/src/shadow-utils.ts new file mode 100644 index 000000000000..fe9648ed765c --- /dev/null +++ b/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 = { + "ReferencedIdentifier|BindingIdentifier"( + path: NodePath, + 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, + functionScope: Scope, + shadowedParams: Set, +) { + 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, + 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), + ); +} diff --git a/packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param-2/input.js b/packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param-2/input.js new file mode 100644 index 000000000000..bc58981bda4d --- /dev/null +++ b/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; +} diff --git a/packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param-2/output.js b/packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param-2/output.js new file mode 100644 index 000000000000..eae5090a3040 --- /dev/null +++ b/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); +} diff --git a/packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param/input.js b/packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param/input.js new file mode 100644 index 000000000000..fbecdb6dc1c4 --- /dev/null +++ b/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; +} diff --git a/packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param/output.js b/packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-nested-in-rest-param/output.js new file mode 100644 index 000000000000..92ade92006ac --- /dev/null +++ b/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); +} diff --git a/packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-rest-param/input.js b/packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-rest-param/input.js new file mode 100644 index 000000000000..d247c7098ef7 --- /dev/null +++ b/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; +} diff --git a/packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-rest-param/output.js b/packages/babel-plugin-transform-parameters/test/fixtures/parameters/var-same-name-as-rest-param/output.js new file mode 100644 index 000000000000..27a15efadef6 --- /dev/null +++ b/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; +}