Add additional checks to Node::isEquivalentTo covering some node-pr…

…ops that were ignored before.

Without this, some pretty semantically different snippets of code were considered equal in tests. Example, static and non-static class members with the same name. The new method of checking the props is also easier to expand.

Some tests are updated to conform to the tightened comparison.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=248049740

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);
   }
 

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 =

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<Node, Object> 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.
+   *
+   * <p>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.
+   *
+   * <p>Accessor functions are used rather than {@link Prop}s to encode the correct way of reading
+   * the prop.
+   */
+  private static final ImmutableList<Function<Node, Object>> 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

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

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();",