diff --git a/src/com/google/javascript/jscomp/Es6RewriteDestructuring.java b/src/com/google/javascript/jscomp/Es6RewriteDestructuring.java index 336c49aff9e..0127c4d518d 100644 --- a/src/com/google/javascript/jscomp/Es6RewriteDestructuring.java +++ b/src/com/google/javascript/jscomp/Es6RewriteDestructuring.java @@ -427,11 +427,7 @@ private void replaceObjectPattern( newRHS = defaultValueHook(getprop, defaultValue); } if (propsToDeleteForRest != null) { - Node propName = astFactory.createString(child.getString()); - if (child.isQuotedString()) { - propName.setQuotedString(); - } - propsToDeleteForRest.add(propName); + propsToDeleteForRest.add(child); } } else if (child.isComputedProp()) { // const {[propExpr]: newLHS = defaultValue} = newRHS; @@ -562,11 +558,25 @@ private Node objectPatternRestRHS( } private Node deletionNodeForRestProperty(Node restTempVarNameNode, Node property) { - boolean useSquareBrackets = !property.isString() || property.isQuotedString(); - Node get = - useSquareBrackets - ? astFactory.createGetElem(restTempVarNameNode, property) - : astFactory.createGetProp(restTempVarNameNode, property.getString()); + final Node get; + switch (property.getToken()) { + case STRING_KEY: + get = + property.isQuotedString() + ? astFactory.createGetElem( + restTempVarNameNode, astFactory.createString(property.getString())) + : astFactory.createGetProp(restTempVarNameNode, property.getString()); + break; + + case NAME: + get = astFactory.createGetElem(restTempVarNameNode, property); + break; + + default: + throw new IllegalStateException( + "Unexpected property to delete node: " + property.toStringTree()); + } + return astFactory.createDelProp(get); } diff --git a/src/com/google/javascript/jscomp/LateEs6ToEs3Converter.java b/src/com/google/javascript/jscomp/LateEs6ToEs3Converter.java index af330d029de..0eb8b820eee 100644 --- a/src/com/google/javascript/jscomp/LateEs6ToEs3Converter.java +++ b/src/com/google/javascript/jscomp/LateEs6ToEs3Converter.java @@ -209,9 +209,12 @@ private void visitObjectWithComputedProperty(Node obj) { } else { Node val = propdef.removeFirstChild(); JSType valueType = val.getJSType(); + Token token = propdef.isQuotedString() ? Token.GETELEM : Token.GETPROP; + propdef.setToken(Token.STRING); propdef.setJSType(stringType); - Token token = propdef.isQuotedString() ? Token.GETELEM : Token.GETPROP; + propdef.putBooleanProp(Node.QUOTED_PROP, false); + Node access = withType(new Node(token, withType(IR.name(objName), objectType), propdef), valueType); result = diff --git a/src/com/google/javascript/rhino/Node.java b/src/com/google/javascript/rhino/Node.java index f6531de4f6f..d2cd55fbcd3 100644 --- a/src/com/google/javascript/rhino/Node.java +++ b/src/com/google/javascript/rhino/Node.java @@ -48,6 +48,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Ascii; import com.google.common.base.Objects; +import com.google.common.collect.ImmutableList; import com.google.errorprone.annotations.DoNotCall; import com.google.javascript.rhino.StaticSourceFile.SourceKind; import com.google.javascript.rhino.jstype.JSType; @@ -62,6 +63,7 @@ import java.util.List; import java.util.NoSuchElementException; import java.util.Set; +import java.util.function.Function; import javax.annotation.CheckReturnValue; import javax.annotation.Nullable; @@ -1951,39 +1953,8 @@ public boolean isEquivalentTo( return false; } - if (token == Token.INC || token == Token.DEC) { - int post1 = this.getIntProp(Prop.INCRDECR); - int post2 = node.getIntProp(Prop.INCRDECR); - if (post1 != post2) { - return false; - } - } else if (token == Token.STRING || token == Token.STRING_KEY) { - if (token == Token.STRING_KEY) { - int quoted1 = this.getIntProp(Prop.QUOTED); - int quoted2 = node.getIntProp(Prop.QUOTED); - if (quoted1 != quoted2) { - return false; - } - } - - int slashV1 = this.getIntProp(Prop.SLASH_V); - int slashV2 = node.getIntProp(Prop.SLASH_V); - if (slashV1 != slashV2) { - return false; - } - } else if (token == Token.CALL) { - if (this.getBooleanProp(Prop.FREE_CALL) != node.getBooleanProp(Prop.FREE_CALL)) { - return false; - } - } else if (token == Token.FUNCTION) { - // Must be the same kind of function to be equivalent - if (this.isArrowFunction() != node.isArrowFunction()) { - return false; - } - if (this.isGeneratorFunction() != node.isGeneratorFunction()) { - return false; - } - if (this.isAsyncFunction() != node.isAsyncFunction()) { + for (Function getter : PROP_GETTERS_FOR_EQUALITY) { + if (!Objects.equal(getter.apply(this), getter.apply(node))) { return false; } } @@ -2011,6 +1982,33 @@ public boolean isEquivalentTo( return true; } + /** + * Accessors for {@link Node} properties that should also be compared when comparing nodes for + * equality. + * + *

We'd prefer to list the props that should be ignored rather than the ones that should be + * checked, but that was too difficult initially. In general these properties are ones that show + * up as keywords / symbols which don't have their own nodes. + * + *

Accessor functions are used rather than {@link Prop}s to encode the correct way of reading + * the prop. + */ + private static final ImmutableList> PROP_GETTERS_FOR_EQUALITY = + ImmutableList.of( + Node::isArrowFunction, + Node::isAsyncFunction, + Node::isAsyncGeneratorFunction, + Node::isGeneratorFunction, + Node::isStaticMember, + Node::isYieldAll, + (n) -> n.getIntProp(Prop.SLASH_V), + (n) -> n.getIntProp(Prop.INCRDECR), + (n) -> n.getIntProp(Prop.QUOTED), + (n) -> n.getBooleanProp(Prop.FREE_CALL), + (n) -> n.getBooleanProp(Prop.COMPUTED_PROP_METHOD), + (n) -> n.getBooleanProp(Prop.COMPUTED_PROP_GETTER), + (n) -> n.getBooleanProp(Prop.COMPUTED_PROP_SETTER)); + /** * This function takes a set of GETPROP nodes and produces a string that is each property * separated by dots. If the node ultimately under the left sub-tree is not a simple name, this is diff --git a/test/com/google/javascript/jscomp/AmbiguatePropertiesTest.java b/test/com/google/javascript/jscomp/AmbiguatePropertiesTest.java index 912cfa397db..979d1de3bf2 100644 --- a/test/com/google/javascript/jscomp/AmbiguatePropertiesTest.java +++ b/test/com/google/javascript/jscomp/AmbiguatePropertiesTest.java @@ -1313,14 +1313,14 @@ public void testTwoUnrelatedClasses_withMemberFns_ambiguated() { } @Test - public void testTwoUnrelatedClasses_withStaticMemberFns_ambiguatede() { + public void testTwoUnrelatedClasses_withStaticMemberFns_ambiguated() { test( lines( "class Foo { static methodFoo() {} }", // "class Bar { static methodBar() {} }"), lines( - "class Foo { a() {} }", // - "class Bar { a() {} }")); + "class Foo { static a() {} }", // + "class Bar { static a() {} }")); } @Test diff --git a/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java b/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java index 6ed6447b67f..d135a49fd04 100644 --- a/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java +++ b/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java @@ -3000,10 +3000,10 @@ public void testDisambiguateEs6ClassStaticMethods() { "Bar.method();"), lines( "class Foo {", - " _typeof_Foo_$method() {}", + " static _typeof_Foo_$method() {}", "}", "class Bar {", - " _typeof_Bar_$method() {}", + " static _typeof_Bar_$method() {}", "}", "Foo._typeof_Foo_$method();", "Bar._typeof_Bar_$method();")); @@ -3027,13 +3027,13 @@ public void testEs6ClassSideInheritedMethods_areDisambiguated() { "Bar.method();"), lines( "class Foo {", - " _typeof_Foo_$method() {}", + " static _typeof_Foo_$method() {}", "}", "class SubFoo extends Foo {", " static _typeof_Foo_$method() {}", "}", "class Bar {", - " _typeof_Bar_$method() {}", + " static _typeof_Bar_$method() {}", "}", "Foo._typeof_Foo_$method();", "SubFoo._typeof_Foo_$method();",