From cdc5654eb67fc6951c47da4876bec96c24eec1ad Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Tue, 28 Apr 2020 04:41:16 -0400 Subject: [PATCH] Fix nested class traversal --- .../src/fields.js | 125 +++++++++--------- 1 file changed, 65 insertions(+), 60 deletions(-) diff --git a/packages/babel-helper-create-class-features-plugin/src/fields.js b/packages/babel-helper-create-class-features-plugin/src/fields.js index 338ea54da927..566458b1c585 100644 --- a/packages/babel-helper-create-class-features-plugin/src/fields.js +++ b/packages/babel-helper-create-class-features-plugin/src/fields.js @@ -70,63 +70,86 @@ export function buildPrivateNamesNodes(privateNamesMap, loose, state) { // Traverses the class scope, handling private name references. If an inner // class redeclares the same private name, it will hand off traversal to the // restricted visitor (which doesn't traverse the inner class's inner scope). -const privateNameVisitor = { - PrivateName(path) { - const { privateNamesMap } = this; - const { node, parentPath } = path; +function privateNameVisitorFactory(visitor) { + const privateNameVisitor = { + ...visitor, + + Class(path) { + const { privateNamesMap } = this; + const body = path.get("body.body"); + + const visiblePrivateNames = new Map(privateNamesMap); + const redeclared = []; + for (const prop of body) { + if (!prop.isPrivate()) continue; + const { name } = prop.node.key.id; + visiblePrivateNames.delete(name); + redeclared.push(name); + } - if (!parentPath.isMemberExpression({ property: node })) return; - if (!privateNamesMap.has(node.id.name)) return; + // If the class doesn't redeclare any private fields, we can continue with + // our overall traversal. + if (!redeclared.length) { + return; + } - this.handle(parentPath); - }, + // This class redeclares some private field. We need to process the outer + // environment with access to all the outer privates, then we can process + // the inner environment with only the still-visible outer privates. + path.get("body").traverse(nestedVisitor, { + ...this, + redeclared, + }); + path.traverse(privateNameVisitor, { + ...this, + privateNamesMap: visiblePrivateNames, + }); - Class(path) { - const { privateNamesMap } = this; - const body = path.get("body.body"); + // We'll eventually hit this class node again with the overall Class + // Features visitor, which'll process the redeclared privates. + path.skipKey("body"); + }, + }; - const visiblePrivateNames = new Map(privateNamesMap); - const redeclared = []; - for (const prop of body) { - if (!prop.isPrivate()) continue; - const { name } = prop.node.key.id; - visiblePrivateNames.delete(name); - redeclared.push(name); - } + // Traverses the outer portion of a class, without touching the class's inner + // scope, for private names. + const nestedVisitor = traverse.visitors.merge([ + { + ...visitor, + }, + environmentVisitor, + ]); - // If the class doesn't redeclare any private fields, we can continue with - // our overall traversal. - if (!redeclared.length) { - return; - } + return privateNameVisitor; +} - // This class redeclares some private field. We need to process the outer - // environment with access to all the outer privates, then we can process - // the inner environment with only the still-visible outer privates. - path.get("body").traverse(privateNameNestedVisitor, { - ...this, - redeclared, - }); - path.traverse(privateNameVisitor, { - ...this, - privateNamesMap: visiblePrivateNames, - }); +const privateNameVisitor = privateNameVisitorFactory({ + PrivateName(path) { + const { privateNamesMap, redeclared } = this; + const { node, parentPath } = path; + + if (!parentPath.isMemberExpression({ property: node })) return; - // We'll eventually hit this class node again with the overall Class - // Features visitor, which'll process the redeclared privates. - path.skipKey("body"); + const { name } = node.id; + if (!privateNamesMap.has(name)) return; + if (redeclared && redeclared.includes(name)) return; + + this.handle(parentPath); }, -}; +}); -const privateInVisitor = { +const privateInVisitor = privateNameVisitorFactory({ BinaryExpression(path) { const { operator, left, right } = path.node; if (operator !== "in") return; if (!path.get("left").isPrivateName()) return; - const { loose, privateNamesMap } = this; + const { loose, privateNamesMap, redeclared } = this; const { name } = left.id; + if (!privateNamesMap.has(name)) return; + if (redeclared && redeclared.includes(name)) return; + if (loose) { const { id } = privateNamesMap.get(name); path.replaceWith(template.expression.ast` @@ -144,25 +167,7 @@ const privateInVisitor = { path.replaceWith(template.expression.ast`${id}.has(${right})`); }, - - Class: privateNameVisitor.Class, -}; - -// Traverses the outer portion of a class, without touching the class's inner -// scope, for private names. -const privateNameNestedVisitor = traverse.visitors.merge([ - { - PrivateName(path) { - const { redeclared } = this; - const { name } = path.node.id; - if (redeclared.includes(name)) path.skip(); - }, - }, - { - PrivateName: privateNameVisitor.PrivateName, - }, - environmentVisitor, -]); +}); const privateNameHandlerSpec = { memoise(member, count) {