From e0b2bfb78f5a02ecf2fe46eb808f5d8481f27a11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Tue, 29 Sep 2020 15:04:24 -0400 Subject: [PATCH] fix: mark Pattern in CatchClause as scope (#12119) --- packages/babel-traverse/src/scope/index.js | 13 ++++++------- .../fixtures/rename/catch-clause-2/input.js | 1 + .../rename/catch-clause-2/options.json | 3 +++ .../fixtures/rename/catch-clause-2/output.js | 3 +++ .../fixtures/rename/catch-clause-2/plugin.js | 9 +++++++++ .../test/fixtures/rename/catch-clause/input.js | 5 +++++ .../fixtures/rename/catch-clause/options.json | 3 +++ .../fixtures/rename/catch-clause/output.js | 7 +++++++ .../fixtures/rename/catch-clause/plugin.js | 9 +++++++++ packages/babel-types/src/validators/isScope.js | 18 ++++++++---------- 10 files changed, 54 insertions(+), 17 deletions(-) create mode 100644 packages/babel-traverse/test/fixtures/rename/catch-clause-2/input.js create mode 100644 packages/babel-traverse/test/fixtures/rename/catch-clause-2/options.json create mode 100644 packages/babel-traverse/test/fixtures/rename/catch-clause-2/output.js create mode 100644 packages/babel-traverse/test/fixtures/rename/catch-clause-2/plugin.js create mode 100644 packages/babel-traverse/test/fixtures/rename/catch-clause/input.js create mode 100644 packages/babel-traverse/test/fixtures/rename/catch-clause/options.json create mode 100644 packages/babel-traverse/test/fixtures/rename/catch-clause/output.js create mode 100644 packages/babel-traverse/test/fixtures/rename/catch-clause/plugin.js diff --git a/packages/babel-traverse/src/scope/index.js b/packages/babel-traverse/src/scope/index.js index 4a7a7f83ef..321db204cc 100644 --- a/packages/babel-traverse/src/scope/index.js +++ b/packages/babel-traverse/src/scope/index.js @@ -996,15 +996,14 @@ export default class Scope { const binding = scope.getOwnBinding(name); if (binding) { // Check if a pattern is a part of parameter expressions. - // 9.2.10.28: The closure created by this expression should not have visibility of + // Note: for performance reason we skip checking previousPath.parentPath.isFunction() + // because `scope.path` is validated as scope in packages/babel-types/src/validators/isScope.js + // That is, if a scope path is pattern, its parent must be Function/CatchClause + + // Spec 9.2.10.28: The closure created by this expression should not have visibility of // declarations in the function body. If the binding is not a `param`-kind, // then it must be defined inside the function body, thus it should be skipped - if ( - previousPath && - previousPath.isPattern() && - previousPath.parentPath.isFunction() && - binding.kind !== "param" - ) { + if (previousPath?.isPattern() && binding.kind !== "param") { // do nothing } else { return binding; diff --git a/packages/babel-traverse/test/fixtures/rename/catch-clause-2/input.js b/packages/babel-traverse/test/fixtures/rename/catch-clause-2/input.js new file mode 100644 index 0000000000..b1e9bae3ab --- /dev/null +++ b/packages/babel-traverse/test/fixtures/rename/catch-clause-2/input.js @@ -0,0 +1 @@ +try {} catch ({ a }) {} diff --git a/packages/babel-traverse/test/fixtures/rename/catch-clause-2/options.json b/packages/babel-traverse/test/fixtures/rename/catch-clause-2/options.json new file mode 100644 index 0000000000..14af0e5fea --- /dev/null +++ b/packages/babel-traverse/test/fixtures/rename/catch-clause-2/options.json @@ -0,0 +1,3 @@ +{ + "plugins": ["./plugin"] +} diff --git a/packages/babel-traverse/test/fixtures/rename/catch-clause-2/output.js b/packages/babel-traverse/test/fixtures/rename/catch-clause-2/output.js new file mode 100644 index 0000000000..9f960a0abf --- /dev/null +++ b/packages/babel-traverse/test/fixtures/rename/catch-clause-2/output.js @@ -0,0 +1,3 @@ +try {} catch ({ + a: z +}) {} diff --git a/packages/babel-traverse/test/fixtures/rename/catch-clause-2/plugin.js b/packages/babel-traverse/test/fixtures/rename/catch-clause-2/plugin.js new file mode 100644 index 0000000000..96ac9a3e31 --- /dev/null +++ b/packages/babel-traverse/test/fixtures/rename/catch-clause-2/plugin.js @@ -0,0 +1,9 @@ +module.exports = function () { + return { + visitor: { + CatchClause(path) { + path.scope.rename("a", "z"); + }, + }, + }; +}; diff --git a/packages/babel-traverse/test/fixtures/rename/catch-clause/input.js b/packages/babel-traverse/test/fixtures/rename/catch-clause/input.js new file mode 100644 index 0000000000..bd350e3823 --- /dev/null +++ b/packages/babel-traverse/test/fixtures/rename/catch-clause/input.js @@ -0,0 +1,5 @@ +let [a] = ["outside"]; + +try {} catch ({ g = () => a }) { + let [a] = ["inside"]; +} diff --git a/packages/babel-traverse/test/fixtures/rename/catch-clause/options.json b/packages/babel-traverse/test/fixtures/rename/catch-clause/options.json new file mode 100644 index 0000000000..14af0e5fea --- /dev/null +++ b/packages/babel-traverse/test/fixtures/rename/catch-clause/options.json @@ -0,0 +1,3 @@ +{ + "plugins": ["./plugin"] +} diff --git a/packages/babel-traverse/test/fixtures/rename/catch-clause/output.js b/packages/babel-traverse/test/fixtures/rename/catch-clause/output.js new file mode 100644 index 0000000000..a1763da536 --- /dev/null +++ b/packages/babel-traverse/test/fixtures/rename/catch-clause/output.js @@ -0,0 +1,7 @@ +let [a] = ["outside"]; + +try {} catch ({ + g = () => a +}) { + let [z] = ["inside"]; +} diff --git a/packages/babel-traverse/test/fixtures/rename/catch-clause/plugin.js b/packages/babel-traverse/test/fixtures/rename/catch-clause/plugin.js new file mode 100644 index 0000000000..96ac9a3e31 --- /dev/null +++ b/packages/babel-traverse/test/fixtures/rename/catch-clause/plugin.js @@ -0,0 +1,9 @@ +module.exports = function () { + return { + visitor: { + CatchClause(path) { + path.scope.rename("a", "z"); + }, + }, + }; +}; diff --git a/packages/babel-types/src/validators/isScope.js b/packages/babel-types/src/validators/isScope.js index dfd2f5edd9..b1cc9b78f0 100644 --- a/packages/babel-types/src/validators/isScope.js +++ b/packages/babel-types/src/validators/isScope.js @@ -11,17 +11,15 @@ import { * Check if the input `node` is a scope. */ export default function isScope(node: Object, parent: Object): boolean { - if (isBlockStatement(node) && isFunction(parent, { body: node })) { - return false; - } - - if (isBlockStatement(node) && isCatchClause(parent, { body: node })) { - return false; - } - - // If a Pattern is an immediate descendent of a Function, it must be in the params. + // If a BlockStatement is an immediate descendent of a Function/CatchClause, it must be in the body. // Hence we skipped the parentKey === "params" check - if (isPattern(node) && isFunction(parent)) { + if (isBlockStatement(node) && (isFunction(parent) || isCatchClause(parent))) { + return false; + } + + // If a Pattern is an immediate descendent of a Function/CatchClause, it must be in the params. + // Hence we skipped the parentKey === "params" check + if (isPattern(node) && (isFunction(parent) || isCatchClause(parent))) { return true; }