Removes duplicated JsType checking in JSORestrictionsChecker
This logic is broken at least wrt. visibilities and already covered
in JsInteropResctrictionChecker.
Also added some missing test scenarios.
Change-Id: I2e6d59ef492972cb2cae165c50a6d3abc600ef76
diff --git a/dev/core/src/com/google/gwt/dev/javac/JSORestrictionsChecker.java b/dev/core/src/com/google/gwt/dev/javac/JSORestrictionsChecker.java
index 1a4283b..82e2c7e 100644
--- a/dev/core/src/com/google/gwt/dev/javac/JSORestrictionsChecker.java
+++ b/dev/core/src/com/google/gwt/dev/javac/JSORestrictionsChecker.java
@@ -60,8 +60,6 @@
*/
public class JSORestrictionsChecker {
- public static final String ERR_JSTYPE_OVERLOADS_NOT_ALLOWED =
- "JsType methods cannot overload another method.";
public static final String ERR_JSEXPORT_ONLY_CTORS_STATIC_METHODS_AND_STATIC_FINAL_FIELDS =
"@JsExport may only be applied to public constructors and static methods and public "
+ "static final fields in public classes.";
@@ -101,7 +99,7 @@
static boolean LINT_MODE = false;
private enum ClassState {
- NORMAL, JSO, JSTYPE, JSTYPE_IMPL
+ NORMAL, JSO
}
/**
@@ -119,12 +117,6 @@
String alreadyImplementor = interfacesToJsoImpls.get(intfName);
String myName = CharOperation.toString(jsoType.binding.compoundName);
- if (!areInSameModule(jsoType, interf)) {
- String msg = errMustBeDefinedInTheSameModule(intfName,myName);
- errorOn(jsoType, cud, msg);
- return;
- }
-
if (alreadyImplementor != null) {
String msg = errAlreadyImplemented(intfName, alreadyImplementor, myName);
errorOn(jsoType, cud, msg);
@@ -133,12 +125,6 @@
interfacesToJsoImpls.put(intfName, myName);
}
-
- // TODO(rluble): (Separate compilation) Implement a real check that a JSO must is defined in
- // the same module as the interface(s) it implements. Depends on upcoming JProgram changes.
- private boolean areInSameModule(TypeDeclaration jsoType, ReferenceBinding interf) {
- return true;
- }
}
private class JSORestrictionsVisitor extends SafeASTVisitor implements
@@ -278,10 +264,7 @@
errorOn(type, ERR_JS_TYPE_WITH_PROTOTYPE_SET_NOT_ALLOWED_ON_CLASS_TYPES);
}
}
- Map<String, MethodBinding> methodSignatures = new HashMap<String, MethodBinding>();
- Map<String, MethodBinding> noExports = new HashMap<String, MethodBinding>();
- checkJsTypeMethodsForOverloads(methodSignatures, noExports, binding);
for (MethodBinding mb : binding.methods()) {
checkJsProperty(mb);
}
@@ -329,52 +312,16 @@
}
}
- private void checkJsTypeMethodsForOverloads(Map<String, MethodBinding> methodNamesAndSigs,
- Map<String, MethodBinding> noExports, ReferenceBinding binding) {
- if (isJsType(binding)) {
- for (MethodBinding mb : binding.methods()) {
- String methodName = String.valueOf(mb.selector);
- if (JdtUtil.getAnnotation(mb, JsInteropUtil.JSNOEXPORT_CLASS) != null) {
- noExports.put(methodName, mb);
- continue;
- }
- if (mb.isConstructor() || mb.isStatic()) {
- continue;
- }
- if (JdtUtil.getAnnotation(mb, JsInteropUtil.JSPROPERTY_CLASS) != null) {
- if (isGetter(methodName, mb) || isSetter(methodName, mb)) {
- // JS properties are allowed to be overloaded (setter/getter)
- continue;
- }
- }
- if (methodNamesAndSigs.containsKey(methodName)) {
- if (!methodNamesAndSigs.get(methodName).areParameterErasuresEqual(mb)) {
- if (noExports.containsKey(methodName)
- && noExports.get(methodName).areParameterErasuresEqual(mb)) {
- continue;
- }
- errorOn(mb, ERR_JSTYPE_OVERLOADS_NOT_ALLOWED);
- }
- } else {
- methodNamesAndSigs.put(methodName, mb);
- }
- }
- }
- for (ReferenceBinding rb : binding.superInterfaces()) {
- checkJsTypeMethodsForOverloads(methodNamesAndSigs, noExports, rb);
- }
- }
-
private ClassState checkType(TypeDeclaration type) {
SourceTypeBinding binding = type.binding;
checkJsFunction(type, binding);
if (isJsType(type.binding)) {
checkJsType(type, type.binding);
- return ClassState.JSTYPE;
+ return ClassState.NORMAL;
}
if (checkClassImplementingJsType(type)) {
- return ClassState.JSTYPE_IMPL;
+ return ClassState.NORMAL;
}
if (!isJsoSubclass(binding)) {
@@ -509,13 +456,13 @@
/**
* Walks up chain of interfaces and superinterfaces to find the first one marked with @JsType.
*/
- private ReferenceBinding findNearestJsType(ReferenceBinding binding, boolean mustHavePrototype) {
+ private ReferenceBinding findNearestJsType(ReferenceBinding binding) {
if (isJsType(binding)) {
return binding;
}
for (ReferenceBinding intb : binding.superInterfaces()) {
- ReferenceBinding checkSuperInt = findNearestJsType(intb, false);
+ ReferenceBinding checkSuperInt = findNearestJsType(intb);
if (checkSuperInt != null) {
return checkSuperInt;
}
@@ -524,7 +471,7 @@
}
private ReferenceBinding findNearestJsTypeRecursive(ReferenceBinding binding) {
- ReferenceBinding nearest = findNearestJsType(binding, false);
+ ReferenceBinding nearest = findNearestJsType(binding);
if (nearest != null) {
return nearest;
} else if (binding.superclass() != null) {
@@ -617,13 +564,6 @@
+ ") is implemented by both (" + impl1 + ") and (" + impl2 + ")";
}
- // TODO(rluble): (Separate compilation) It would be nice to have the actual module names here.
- static String errMustBeDefinedInTheSameModule(String intfName, String jsoImplementation) {
- return "A JavaScriptObject type may only implement an interface that is defined in the same"
- + " module. The interface (" + intfName + ") and the JavaScriptObject type (" +
- jsoImplementation + ") are defined different modules";
- }
-
private static void errorOn(ASTNode node, CompilationUnitDeclaration cud,
String error) {
GWTProblem.recordError(node, cud, error, new InstalledHelpInfo(
diff --git a/dev/core/test/com/google/gwt/dev/javac/JSORestrictionsTest.java b/dev/core/test/com/google/gwt/dev/javac/JSORestrictionsTest.java
index 0177ecb..27e86b6 100644
--- a/dev/core/test/com/google/gwt/dev/javac/JSORestrictionsTest.java
+++ b/dev/core/test/com/google/gwt/dev/javac/JSORestrictionsTest.java
@@ -356,31 +356,6 @@
shouldGenerateNoError(goodCode);
}
- public void testJsTypeNoOverloads() {
- StringBuilder buggyCode = new StringBuilder();
- buggyCode.append("import com.google.gwt.core.client.js.JsType;\n");
- buggyCode.append("@JsType\n");
- buggyCode.append("public interface Buggy {\n");
- buggyCode.append("void foo();\n");
- buggyCode.append("void foo(int x);\n");
- buggyCode.append("}\n");
-
- shouldGenerateError(buggyCode,
- "Line 5: " + JSORestrictionsChecker.ERR_JSTYPE_OVERLOADS_NOT_ALLOWED);
- }
-
- public void testJsTypeNoOverloadsHierarchy() {
- StringBuilder buggyCode = new StringBuilder();
- buggyCode.append("import com.google.gwt.core.client.js.JsType;\n");
- buggyCode.append("public interface Buggy {\n");
- buggyCode.append("@JsType interface Buggy2 { void foo(); }\n");
- buggyCode.append("@JsType interface Buggy3 extends Buggy2 { void foo(int x); }\n");
- buggyCode.append("}\n");
-
- shouldGenerateError(buggyCode,
- "Line 3: " + JSORestrictionsChecker.ERR_JSTYPE_OVERLOADS_NOT_ALLOWED);
- }
-
public void testJsExport() {
StringBuilder goodCode = new StringBuilder();
goodCode.append("import com.google.gwt.core.client.js.JsExport;\n");
@@ -903,10 +878,6 @@
logger.assertCorrectLogEntries();
}
- private void shouldGenerateNoError(SourceLevel sourceLevel, StringBuilder buggyCode) {
- shouldGenerateError(sourceLevel, buggyCode, (String[]) null);
- }
-
private void shouldGenerateError(CharSequence buggyCode, String... expectedErrors) {
shouldGenerateError(SourceLevel.DEFAULT_SOURCE_LEVEL, buggyCode, expectedErrors);
}
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 4346a09..eed058d 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
@@ -114,58 +114,58 @@
+ "global name 'show' is already taken.");
}
- public void testCollidingJsPropertiesHasAndGetterSucceeds() throws Exception {
+ public void testJsPropertyGetterStyleSucceeds() throws Exception {
addSnippetImport("com.google.gwt.core.client.js.JsType");
addSnippetImport("com.google.gwt.core.client.js.JsProperty");
addSnippetClassDecl(
"@JsType",
"public static interface IBuggy {",
- " @JsProperty",
- " boolean hasX();",
- " @JsProperty",
- " int x();",
+ " @JsProperty boolean hasX();",
+ " @JsProperty int getX();",
+ " @JsProperty void setX(int x);",
"}",
"public static class Buggy implements IBuggy {",
" public boolean hasX() {return false;}",
- " public int x() {return 0;}",
+ " public int getX() {return 0;}",
+ " public void setX(int x) {};",
"}");
assertCompileSucceeds();
}
- public void testCollidingJsPropertiesHasAndSetterSucceeds() throws Exception {
+ public void testJsPropertyNonGetterStyleSucceeds() throws Exception {
addSnippetImport("com.google.gwt.core.client.js.JsType");
addSnippetImport("com.google.gwt.core.client.js.JsProperty");
addSnippetClassDecl(
"@JsType",
"public static interface IBuggy {",
- " @JsProperty",
- " boolean hasX();",
- " @JsProperty",
- " void x(int x);",
+ " @JsProperty boolean hasX();",
+ " @JsProperty int x();",
+ " @JsProperty void x(int x);",
"}",
"public static class Buggy implements IBuggy {",
" public boolean hasX() {return false;}",
- " public void x(int x) {}",
+ " public int x() {return 0;}",
+ " public void x(int x) {};",
"}");
assertCompileSucceeds();
}
- public void testCollidingJsPropertiesSetterAndGetterSucceeds() throws Exception {
+ public void testJsPropertyFluentStyleSucceeds() throws Exception {
addSnippetImport("com.google.gwt.core.client.js.JsType");
addSnippetImport("com.google.gwt.core.client.js.JsProperty");
addSnippetClassDecl(
"@JsType",
"public static interface IBuggy {",
- " @JsProperty",
- " int x();",
- " @JsProperty",
- " void x(int x);",
+ " @JsProperty boolean hasX();",
+ " @JsProperty int x();",
+ " @JsProperty IBuggy x(int x);",
"}",
"public static class Buggy implements IBuggy {",
+ " public boolean hasX() {return false;}",
" public int x() {return 0;}",
- " public void x(int x) {}",
+ " public IBuggy x(int x) {return null;};",
"}");
assertCompileSucceeds();
@@ -303,6 +303,19 @@
+ "'test.EntryPoint$Buggy' because the member name 'show' is already taken.");
}
+ public void testCollidingMethodToMethodJsTypeFails() throws Exception {
+ addSnippetImport("com.google.gwt.core.client.js.JsType");
+ addSnippetClassDecl(
+ "@JsType",
+ "public static class Buggy {",
+ " public void show(int x) {}",
+ " public void show() {}",
+ "}");
+
+ assertCompileFails("Method 'test.EntryPoint$Buggy.show()V' can't be exported in type "
+ + "'test.EntryPoint$Buggy' because the member name 'show' is already taken.");
+ }
+
public void testCollidingSubclassExportedFieldToFieldJsTypeSucceeds() throws Exception {
addSnippetImport("com.google.gwt.core.client.js.JsType");
addSnippetClassDecl(
