diff --git a/src/com/google/javascript/jscomp/AggressiveInlineAliases.java b/src/com/google/javascript/jscomp/AggressiveInlineAliases.java index 2678f03481f..6dd7fa57422 100644 --- a/src/com/google/javascript/jscomp/AggressiveInlineAliases.java +++ b/src/com/google/javascript/jscomp/AggressiveInlineAliases.java @@ -247,7 +247,7 @@ private boolean rewriteAllSubclassInheritedAccesses( if (subclassPropNameObj != null) { Set newNodes = new LinkedHashSet<>(); - // Use this node as a template for rewriteAliasProp. + // Use this node as a template for rewriteNestedAliasReference. Node superclassNameNode = superclassNameObj.getDeclaration().getNode(); if (superclassNameNode.isName()) { superclassNameNode = superclassNameNode.cloneNode(); @@ -257,7 +257,7 @@ private boolean rewriteAllSubclassInheritedAccesses( return false; } - rewriteAliasProp(superclassNameNode, 0, newNodes, subclassPropNameObj); + rewriteNestedAliasReference(superclassNameNode, 0, newNodes, subclassPropNameObj); namespace.scanNewNodes(newNodes); } return true; @@ -662,21 +662,63 @@ private void rewriteAliasProps(Name name, Node value, int depth, Set value, name.getFullName()); for (Name prop : name.props) { - rewriteAliasProp(value, depth, newNodes, prop); + rewriteNestedAliasReference(value, depth, newNodes, prop); } } /** + * Replaces references to an alias that are nested inside a longer getprop chain or an object + * literal + * + *

For example: if we have an inlined alias 'const A = B;', and reference a property 'A.x', + * then this method is responsible for replacing 'A.x' with 'B.x'. + * + *

This is necessary because in the above example, given 'A.x', there is only one {@link Ref} + * that points to the whole name 'A.x', not a direct {@link Ref} to 'A'. So the only way to + * replace 'A.x' with 'B.x' is by looking at the property 'x' reference. + * * @param value The value to use when rewriting. - * @param depth The chain depth. + * @param depth The property chain depth. * @param newNodes Expression nodes that have been updated. * @param prop The property to rewrite with value. */ - private void rewriteAliasProp(Node value, int depth, Set newNodes, Name prop) { + private void rewriteNestedAliasReference( + Node value, int depth, Set newNodes, Name prop) { rewriteAliasProps(prop, value, depth + 1, newNodes); List refs = new ArrayList<>(prop.getRefs()); for (Ref ref : refs) { Node target = ref.getNode(); + if (target.isStringKey() && target.getParent().isDestructuringPattern()) { + // Do nothing for alias properties accessed through object destructuring. This would be + // redundant. This method is intended for names nested inside getprop chains, because + // GlobalNamespace only creates a single Ref for the outermost getprop. However, for + // destructuring property accesses, GlobalNamespace creates multiple Refs, one for the + // destructured object, and one for each string key in the pattern. + // + // For example, consider: + // const originalObj = {key: 0}; + // const rhs = originalObj; + // const {key: lhs} = rhs; + // const otherLhs = rhs.key; + // AggressiveInlineAliases is inlining rhs -> originalObj. + // + // GlobalNamespace creates two Refs for the name 'rhs': one for its declaration, + // and one for 'const {key: lhs} = rhs;'. There is no Ref pointing directly to the 'rhs' + // in 'const otherLhs = rhs.key', though. + // There are also two Refs to the name 'rhs.key': one for the destructuring access and one + // for the getprop access. This loop will visit both Refs. + // This method is responsible for inlining "const otherLhs = originalObj.key" but not + // "const {key: lhs} = originalObj;". We bail out at the Ref in the latter case. + checkState( + target.getGrandparent().isAssign() || target.getGrandparent().isDestructuringLhs(), + // Currently GlobalNamespace doesn't create Refs for 'b' in const {a: {b}} = obj; + // If it does start creating those Refs, we may have to update this method to handle + // them explicitly. + "Did not expect GlobalNamespace to create Ref for key in nested object pattern %s", + target); + continue; + } + for (int i = 0; i <= depth; i++) { if (target.isGetProp()) { target = target.getFirstChild(); @@ -690,22 +732,10 @@ private void rewriteAliasProp(Node value, int depth, Set newNodes, Na checkState(NodeUtil.isObjectLitKey(gparent)); target = gparent; } - } else if (target.getParent().isObjectPattern()) { - // The rhs of the pattern has its own 'Ref' and will already have been rewritten. - Node grandparent = target.getGrandparent(); - // Right now GlobalNamespace only handles object patterns in assignments or declarations, - // not nested object patterns, so expect the grandparent to be an assign/decl. - checkState(grandparent.isAssign() || grandparent.isDestructuringLhs()); - target = null; - } else { - throw new IllegalStateException("unexpected: " + target); + throw new IllegalStateException("unexpected node: " + target); } } - if (target == null) { - // Ignore the rhs of destructuring patterns - continue; - } checkState(target.isGetProp() || target.isName()); Node newValue = value.cloneTree(); target.replaceWith(newValue); diff --git a/src/com/google/javascript/jscomp/GlobalNamespace.java b/src/com/google/javascript/jscomp/GlobalNamespace.java index 4ca92ad4678..08ecc3a35c2 100644 --- a/src/com/google/javascript/jscomp/GlobalNamespace.java +++ b/src/com/google/javascript/jscomp/GlobalNamespace.java @@ -396,6 +396,8 @@ public void collect(JSModule module, Scope scope, Node n) { } break; case GETPROP: + // This name is nested in a getprop. Return and only create a Ref for the outermost + // getprop in the chain. return; case FUNCTION: Node grandparent = parent.getParent(); @@ -453,6 +455,8 @@ public void collect(JSModule module, Scope scope, Node n) { type = Name.Type.OTHER; break; case GETPROP: + // This is nested in another getprop. Return and only create a Ref for the outermost + // getprop in the chain. return; default: if (NodeUtil.isAssignmentOp(parent) && parent.getFirstChild() == n) { diff --git a/test/com/google/javascript/jscomp/AggressiveInlineAliasesTest.java b/test/com/google/javascript/jscomp/AggressiveInlineAliasesTest.java index 99dc3914667..1307f2cdc71 100644 --- a/test/com/google/javascript/jscomp/AggressiveInlineAliasesTest.java +++ b/test/com/google/javascript/jscomp/AggressiveInlineAliasesTest.java @@ -875,6 +875,25 @@ public void testGlobalAliasWithProperties3() { + "use(x)"); } + @Test + public void testGlobalAliasWithPropertiesAsNestedObjectLits() { + test( + lines( + "var ns = {};" + + "ns.Foo = function() {};" + + "ns.Bar = ns.Foo;" + + "/** @enum { number } */ ns.Bar.Other = { X: {Y: 1}};" + + "var x = function() { use(ns.Bar.Other.X.Y) };" + + "use(x)"), + lines( + "var ns = {};" + + "ns.Foo = function() {};" + + "ns.Bar = null;" + + "/** @enum { number } */ ns.Foo.Other = { X: {Y: 1}};" + + "var x = function() { use(ns.Foo.Other.X.Y) };" + + "use(x)")); + } + @Test public void testGlobalWriteToNonAncestor() { test( @@ -2054,6 +2073,27 @@ public void testDestructuredPropAccessInDeclarationWithDefault() { "use(y);")); } + @Test + public void testDestructuringPropertyOnAliasedNamespace() { + // We can inline a part of a getprop chain on the rhs of a destructuring pattern: + // replace 'alias -> a.b' in 'const {A} = alias.Enum;' + test( + lines( + "const a = {};", + "/** @const */ a.b = {};", + "/** @enum {string} */ a.b.Enum = {A: 'a'};", + "", + "const alias = a.b;", + "function f() { const {A} = alias.Enum; }"), + lines( + "const a = {}; ", + "/** @const */ a.b = {};", + "/** @enum {string} */ a.b.Enum = {A: 'a'};", + "", + "const alias = null;", + "function f() { const {A} = a.b.Enum; }")); + } + @Test public void testReplaceSuperGetPropInStaticMethod() { test(