Fix the checking for JsProperty override name clash Change-Id: Iec0e0d524157fc639cd9e7e9af0c7a3ba6c532b7 Review-Link: https://gwt-review.googlesource.com/#/c/13994/

diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
index cff496e..fa3def1 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java

@@ -16,7 +16,6 @@
 package com.google.gwt.dev.jjs.ast;
 
 import com.google.gwt.dev.common.InliningMode;
-import com.google.gwt.dev.javac.JsInteropUtil;
 import com.google.gwt.dev.jjs.InternalCompilerException;
 import com.google.gwt.dev.jjs.SourceInfo;
 import com.google.gwt.dev.jjs.SourceOrigin;
@@ -24,6 +23,7 @@
 import com.google.gwt.dev.jjs.impl.JjsUtils;
 import com.google.gwt.dev.util.StringInterner;
 import com.google.gwt.dev.util.collect.Lists;
+import com.google.gwt.thirdparty.guava.common.collect.Iterables;
 import com.google.gwt.thirdparty.guava.common.collect.Sets;
 
 import java.io.IOException;
@@ -76,11 +76,8 @@
 
   @Override
   public boolean canBeReferencedExternally() {
-    if (exported || isJsFunctionMethod()) {
-      return true;
-    }
-    for (JMethod overriddenMethod : getOverriddenMethods()) {
-      if (overriddenMethod.exported || overriddenMethod.isJsFunctionMethod()) {
+    for (JMethod method : getOverriddenMethodsIncludingSelf()) {
+      if (method.exported || method.isJsFunctionMethod()) {
         return true;
       }
     }
@@ -124,18 +121,12 @@
 
   @Override
   public String getJsName() {
-    String jsMemberName = jsName;
-    for (JMethod override : getOverriddenMethods()) {
-      String jsMemberOverrideName = override.jsName;
-      if (jsMemberOverrideName == null) {
-        continue;
+    for (JMethod method : getOverriddenMethodsIncludingSelf()) {
+      if (method.jsName != null) {
+        return method.jsName;
       }
-      if (jsMemberName != null && !jsMemberName.equals(jsMemberOverrideName)) {
-        return JsInteropUtil.INVALID_JSNAME;
-      }
-      jsMemberName = jsMemberOverrideName;
     }
-    return jsMemberName;
+    return null;
   }
 
   public boolean isJsConstructor() {
@@ -166,12 +157,9 @@
 
   @Override
   public JsMemberType getJsMemberType() {
-    if (jsMemberType != JsMemberType.NONE) {
-      return jsMemberType;
-    }
-    for (JMethod overriddenMethod : getOverriddenMethods()) {
-      if (overriddenMethod.jsMemberType != JsMemberType.NONE) {
-        return overriddenMethod.jsMemberType;
+    for (JMethod method : getOverriddenMethodsIncludingSelf()) {
+      if (method.jsMemberType != JsMemberType.NONE) {
+        return method.jsMemberType;
       }
     }
     return JsMemberType.NONE;
@@ -182,11 +170,8 @@
   }
 
   public boolean isOrOverridesJsFunctionMethod() {
-    if (isJsFunctionMethod()) {
-      return true;
-    }
-    for (JMethod overriddenMethod : getOverriddenMethods()) {
-      if (overriddenMethod.isJsFunctionMethod()) {
+    for (JMethod method : getOverriddenMethodsIncludingSelf()) {
+      if (method.isJsFunctionMethod()) {
         return true;
       }
     }
@@ -522,6 +507,13 @@
   }
 
   /**
+   * Return all overridden methods including the method itself (where it is the first item).
+   */
+  private Iterable<JMethod> getOverriddenMethodsIncludingSelf() {
+    return Iterables.concat(Collections.singleton(this), overriddenMethods);
+  }
+
+  /**
    * Returns the transitive closure of all the methods that override this method; caveat this
    * list is only complete in monolithic compiles and should not be used in incremental compiles.
    * The returned set is ordered in such a way that most specific overriding methods appear after

diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java b/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java
index d54b64c..bc9cc5b 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java

@@ -246,13 +246,10 @@
       return;
     }
 
-    if (member.getJsName().equals(JsInteropUtil.INVALID_JSNAME)) {
-      logInvalidName(member);
+    if (!checkJsPropertyAccessor(member)) {
       return;
     }
 
-    checkJsPropertyAccessor(member);
-
     checkMemberQualifiedJsName(member);
 
     if (isCheckedLocalName(member)) {
@@ -340,21 +337,16 @@
     }
   }
 
-  private void logInvalidName(JMember member) {
-    if (member.getJsMemberType().isPropertyAccessor()) {
+  private boolean checkJsPropertyAccessor(JMember member) {
+    if (member.getJsName().equals(JsInteropUtil.INVALID_JSNAME)) {
+      assert member.getJsMemberType().isPropertyAccessor();
       logError(
           member,
           "JsProperty %s should either follow Java Bean naming conventions or provide a name.",
           getMemberDescription(member));
-    } else {
-      logError(
-          member,
-          "%s cannot be assigned a different JavaScript name than the method it overrides.",
-          getMemberDescription(member));
+      return false;
     }
-  }
 
-  private void checkJsPropertyAccessor(JMember member) {
     if (member.getJsMemberType() == JsMemberType.UNDEFINED_ACCESSOR) {
       logError(member, "JsProperty %s should have a correct setter or getter signature.",
           getMemberDescription(member));
@@ -366,6 +358,7 @@
             getMemberDescription(member));
       }
     }
+    return true;
   }
 
   private void checkMemberQualifiedJsName(JMember member) {
@@ -414,6 +407,7 @@
     JsMember oldJsMember = oldAndNewJsMember.left;
     JsMember newJsMember = oldAndNewJsMember.right;
 
+    checkNameConsistency(member);
     checkJsPropertyConsistency(member, newJsMember);
 
     if (oldJsMember == null || oldJsMember == newJsMember) {
@@ -460,6 +454,22 @@
     }
   }
 
+  private void checkNameConsistency(JMember member) {
+    if (member instanceof JMethod) {
+      String jsName = member.getJsName();
+      for (JMethod jMethod : ((JMethod) member).getOverriddenMethods()) {
+        String parentName = jMethod.getJsName();
+        if (parentName != null && !parentName.equals(jsName)) {
+          logError(
+              member,
+              "%s cannot be assigned a different JavaScript name than the method it overrides.",
+              getMemberDescription(member));
+          break;
+        }
+      }
+    }
+  }
+
   private void checkStaticJsPropertyCalls() {
     new JVisitor() {
       @Override

diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java
index 63b6205..9e479b4 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java

@@ -711,21 +711,18 @@
         + "cannot be assigned a different JavaScript name than the method it overrides.");
   }
 
-  // TODO(goktug): enable once the property names are handled.
-  public void __disabled__testRenamedSuperclassJsPropertyFails() {
-    addSnippetImport("jsinterop.annotations.JsType");
+  public void testRenamedSuperclassJsPropertyFails() {
     addSnippetImport("jsinterop.annotations.JsProperty");
     addSnippetClassDecl(
-        "@JsType",
         "public static class ParentBuggy {",
         "  @JsProperty public int getFoo() { return 0; }",
         "}",
         "public static class Buggy extends ParentBuggy {",
-        "  @JsProperty(name = \"bar\") public int getFoo() { return 0;}",
+        "  @JsProperty(name = \"bar\") public int getFoo() { return 0; }",
         "}");
 
-    assertBuggyFails("'EntryPoint.Buggy.getFoo()I' cannot be exported because the method "
-        + "overrides a method with different name.");
+    assertBuggyFails("Line 8: 'int EntryPoint.Buggy.getFoo()' "
+        + "cannot be assigned a different JavaScript name than the method it overrides.");
   }
 
   public void testJsPropertyDifferentFlavourInSubclassFails() {