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() {
