Change restrictions for JsFunction implementations.
This patch *adds* the following restrictions:
- JsFunction implementations cannot define any method that has
dynamic dispatch. In particular it can not override j.l.Object
methods.
- JsFunction interfaces fields and methods other than the SAM
need explicit @JsOverlay markers
This patch *relaxes* the following restrictions:
- Default methods can be declared in JsFunction interfaces as long
as they are JsOverlay.
The intention is to make sure JsFunction implementations do not
need to be backed up by class at runtime and can eventually be
implemented as just lightweight JS functions.
Bug: #9348
Bug-Link: http://github.com/gwtproject/gwt/issues/9348
Change-Id: I7746100e9122c47c0100e396d085884582b0f916
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 f251fa8..e3eb085 100644
--- a/dev/core/src/com/google/gwt/dev/javac/JSORestrictionsChecker.java
+++ b/dev/core/src/com/google/gwt/dev/javac/JSORestrictionsChecker.java
@@ -75,8 +75,6 @@
"Methods cannot be overridden in JavaScriptObject subclasses";
public static final String ERR_JS_FUNCTION_ONLY_ALLOWED_ON_FUNCTIONAL_INTERFACE =
"@JsFunction is only allowed on functional interface";
- public static final String ERR_JS_FUNCTION_CANNOT_HAVE_DEFAULT_METHODS =
- "JsFunction cannot have default methods";
private enum ClassState {
NORMAL, JSO
@@ -221,10 +219,6 @@
errorOn(type, ERR_JS_FUNCTION_ONLY_ALLOWED_ON_FUNCTIONAL_INTERFACE);
return;
}
- // If a functional interface has more than one method, it means it has default methods.
- if (binding.methods().length > 1) {
- errorOn(type, ERR_JS_FUNCTION_CANNOT_HAVE_DEFAULT_METHODS);
- }
}
private ClassState checkType(TypeDeclaration type) {
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 f168d42..f31c96a 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
@@ -227,7 +227,7 @@
}
private boolean isJsFunctionMethod() {
- return enclosingType.isJsFunction();
+ return enclosingType.isJsFunction() && isAbstract();
}
public boolean isOrOverridesJsFunctionMethod() {
@@ -248,7 +248,9 @@
public boolean isJsOverlay() {
return isJsOverlay
|| getEnclosingType().isJsoType()
- || getEnclosingType().isJsNative() && JProgram.isClinit(this);
+ // Clinits are implicit overlays on native types and JsFunction interfaces.
+ || JProgram.isClinit(this)
+ && (getEnclosingType().isJsNative() || getEnclosingType().isJsFunction());
}
public void setSyntheticAccidentalOverride() {
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 3f8dc71..2e070c2 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
@@ -298,8 +298,10 @@
String memberDescription = JjsUtils.getReadableDescription(member);
- if (!member.getEnclosingType().isJsNative()) {
- logError(member, "JsOverlay '%s' can only be declared in a native type.", memberDescription);
+ if (!member.getEnclosingType().isJsNative() && !member.getEnclosingType().isJsFunction()) {
+ logError(member,
+ "JsOverlay '%s' can only be declared in a native type or a JsFunction interface.",
+ memberDescription);
}
if (member instanceof JConstructor) {
@@ -676,6 +678,26 @@
return true;
}
+ private void checkMemberOfJsFunction(JMember member) {
+ if (member.getJsMemberType() != JsMemberType.NONE) {
+ logError(member,
+ "JsFunction interface member '%s' cannot be JsMethod nor JsProperty.",
+ JjsUtils.getReadableDescription(member));
+ }
+
+ if (member.isJsOverlay() || member.isSynthetic()) {
+ return;
+ }
+
+ if (member instanceof JMethod && ((JMethod) member).isOrOverridesJsFunctionMethod()) {
+ return;
+ }
+
+ logError(member, "JsFunction interface '%s' cannot declare non-JsOverlay member '%s'.",
+ JjsUtils.getReadableDescription(member.getEnclosingType()),
+ JjsUtils.getReadableDescription(member));
+ }
+
private void checkJsFunction(JDeclaredType type) {
if (!isClinitEmpty(type, false)) {
logError("JsFunction '%s' cannot have static initializer.", type);
@@ -687,7 +709,39 @@
if (type.isJsType()) {
logError("'%s' cannot be both a JsFunction and a JsType at the same time.", type);
+ return;
}
+
+ // Functional interface restriction already enforced by JSORestrictionChecker. It is safe
+ // to assume here that there is a single abstract method.
+ for (JMember member : type.getMembers()) {
+ checkMemberOfJsFunction(member);
+ }
+ }
+
+ private void checkMemberOfJsFunctionImplementation(JMember member) {
+ if (member.getJsMemberType() != JsMemberType.NONE) {
+ logError(member,
+ "JsFunction implementation member '%s' cannot be JsMethod nor JsProperty.",
+ JjsUtils.getReadableDescription(member));
+ }
+
+ if (!(member instanceof JMethod)) {
+ return;
+ }
+
+ JMethod method = (JMethod) member;
+ if (method.isOrOverridesJsFunctionMethod()
+ || method.isSynthetic()
+ || method.getOverriddenMethods().isEmpty()) {
+ return;
+ }
+
+ // Methods that are not effectively static dispatch are disallowed. In this case these
+ // could only be overridable methods of java.lang.Object, i.e. toString, hashCode and equals.
+ logError(method, "JsFunction implementation '%s' cannot implement method '%s'.",
+ JjsUtils.getReadableDescription(member.getEnclosingType()),
+ JjsUtils.getReadableDescription(method));
}
private void checkJsFunctionImplementation(JDeclaredType type) {
@@ -695,18 +749,23 @@
logError("JsFunction implementation '%s' must be final.",
type);
}
+
if (type.getImplements().size() != 1) {
logError("JsFunction implementation '%s' cannot implement more than one interface.",
type);
}
+ if (type.getSuperClass() != jprogram.getTypeJavaLangObject()) {
+ logError("JsFunction implementation '%s' cannot extend a class.", type);
+ }
+
if (type.isJsType()) {
logError("'%s' cannot be both a JsFunction implementation and a JsType at the same time.",
type);
+ return;
}
-
- if (type.getSuperClass() != jprogram.getTypeJavaLangObject()) {
- logError("JsFunction implementation '%s' cannot extend a class.", type);
+ for (JMember member : type.getMembers()) {
+ checkMemberOfJsFunctionImplementation(member);
}
}
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 4c6e8ff..d5e1d6d 100644
--- a/dev/core/test/com/google/gwt/dev/javac/JSORestrictionsTest.java
+++ b/dev/core/test/com/google/gwt/dev/javac/JSORestrictionsTest.java
@@ -411,18 +411,6 @@
"Line 2: " + JSORestrictionsChecker.ERR_JS_FUNCTION_ONLY_ALLOWED_ON_FUNCTIONAL_INTERFACE);
}
- public void testJsFunctionNotOnInterfaceWithDefaultMethod() {
- StringBuilder buggyCode = new StringBuilder();
- buggyCode.append("import jsinterop.annotations.JsFunction;\n");
- buggyCode.append("@JsFunction public interface Buggy {\n");
- buggyCode.append("int foo(int x);\n");
- buggyCode.append("default void bar() { }\n");
- buggyCode.append("}\n");
-
- shouldGenerateError(SourceLevel.JAVA8, buggyCode,
- "Line 2: " + JSORestrictionsChecker.ERR_JS_FUNCTION_CANNOT_HAVE_DEFAULT_METHODS);
- }
-
/**
* Test that when compiling buggyCode, the TypeOracleUpdater emits
* expectedError somewhere in its output. The code should define a class named
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 44007b6..a110ab3 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
@@ -1147,6 +1147,10 @@
"@JsFunction",
"public interface Function {",
" int getFoo();",
+ " @JsOverlay",
+ " default void m() {}",
+ " @JsOverlay",
+ " static void n() {}",
"}",
"public static final class Buggy implements Function {",
" public int getFoo() { return 0; }",
@@ -1168,19 +1172,34 @@
public void testJsFunctionFails() throws Exception {
addSnippetImport("jsinterop.annotations.JsFunction");
+ addSnippetImport("jsinterop.annotations.JsMethod");
+ addSnippetImport("jsinterop.annotations.JsOverlay");
addSnippetImport("jsinterop.annotations.JsProperty");
addSnippetImport("jsinterop.annotations.JsType");
addSnippetClassDecl(
"@JsFunction",
- "interface Function {",
+ "public interface Function {",
" int getFoo();",
"}",
- "static final class Buggy implements Function {",
+ "public static final class Buggy implements Function {",
+ " @JsProperty",
" public int getFoo() { return 0; }",
+ " @JsMethod",
+ " private void bleh() {}",
+ " @JsProperty",
+ " public int prop = 0;",
+ " public String toString() { return \"\"; }",
+ " public boolean equals(Object o) { return false; }",
+ " public int hashCode() { return 0; }",
"}",
"@JsFunction",
- "interface InvalidFunction {",
+ "public interface InvalidFunction {",
+ " @JsProperty",
" int getFoo();",
+ " default void m() {}",
+ " int f = 0;",
+ " static void n() {}",
+ " @JsOverlay",
" String s = null;",
"}",
"static class NonFinalJsFunction implements Function {",
@@ -1204,19 +1223,38 @@
"}");
assertBuggyFails(
- "Line 14: JsFunction 'EntryPoint.InvalidFunction' cannot have static initializer.",
- "Line 18: JsFunction implementation 'EntryPoint.NonFinalJsFunction' must be final.",
- "Line 22: 'EntryPoint.JsFunctionMarkedAsJsType' cannot be both a JsFunction implementation "
+ "Line 14: JsFunction implementation member 'int EntryPoint.Buggy.getFoo()' "
+ + "cannot be JsMethod nor JsProperty.",
+ "Line 16: JsFunction implementation member 'void EntryPoint.Buggy.bleh()' cannot "
+ + "be JsMethod nor JsProperty.",
+ "Line 18: JsFunction implementation member 'int EntryPoint.Buggy.prop' cannot "
+ + "be JsMethod nor JsProperty.",
+ "Line 19: JsFunction implementation 'EntryPoint.Buggy' cannot implement method "
+ + "'String EntryPoint.Buggy.toString()'.",
+ "Line 20: JsFunction implementation 'EntryPoint.Buggy' cannot implement method "
+ + "'boolean EntryPoint.Buggy.equals(Object)'.",
+ "Line 21: JsFunction implementation 'EntryPoint.Buggy' cannot implement method "
+ + "'int EntryPoint.Buggy.hashCode()'.",
+ "Line 26: JsFunction interface member 'int EntryPoint.InvalidFunction.getFoo()' cannot "
+ + "be JsMethod nor JsProperty.",
+ "Line 27: JsFunction interface 'EntryPoint.InvalidFunction' cannot declare non-JsOverlay "
+ + "member 'void EntryPoint.InvalidFunction.m()'.",
+ "Line 28: JsFunction interface 'EntryPoint.InvalidFunction' cannot declare non-JsOverlay "
+ + "member 'int EntryPoint.InvalidFunction.f'.",
+ "Line 29: JsFunction interface 'EntryPoint.InvalidFunction' cannot declare non-JsOverlay "
+ + "member 'void EntryPoint.InvalidFunction.n()'.",
+ "Line 31: JsFunction implementation 'EntryPoint.NonFinalJsFunction' must be final.",
+ "Line 35: 'EntryPoint.JsFunctionMarkedAsJsType' cannot be both a JsFunction implementation "
+ "and a JsType at the same time.",
- "Line 26: JsFunction 'EntryPoint.JsFunctionExtendsInterface' cannot extend other"
+ "Line 39: JsFunction 'EntryPoint.JsFunctionExtendsInterface' cannot extend other"
+ " interfaces.",
- "Line 29: 'EntryPoint.InterfaceExtendsJsFunction' cannot extend "
+ "Line 42: 'EntryPoint.InterfaceExtendsJsFunction' cannot extend "
+ "JsFunction 'EntryPoint.Function'.",
- "Line 30: Cannot do instanceof against JsFunction implementation "
+ "Line 43: Cannot do instanceof against JsFunction implementation "
+ "'EntryPoint.Buggy'.",
- "Line 31: JsFunction implementation 'EntryPoint.JsFunctionExtendingBaseClass' cannot "
+ "Line 44: JsFunction implementation 'EntryPoint.JsFunctionExtendingBaseClass' cannot "
+ "extend a class.",
- "Line 34: JsFunction implementation 'EntryPoint.JsFunctionMultipleInterfaces' cannot "
+ "Line 47: JsFunction implementation 'EntryPoint.JsFunctionMultipleInterfaces' cannot "
+ "implement more than one interface.");
}
@@ -1753,8 +1791,10 @@
"}");
assertBuggyFails(
- "Line 6: JsOverlay 'int EntryPoint.Buggy.f' can only be declared in a native type.",
- "Line 7: JsOverlay 'void EntryPoint.Buggy.m()' can only be declared in a native type.");
+ "Line 6: JsOverlay 'int EntryPoint.Buggy.f' can only be declared in a native type "
+ + "or a JsFunction interface.",
+ "Line 7: JsOverlay 'void EntryPoint.Buggy.m()' can only be declared in a native type "
+ + "or a JsFunction interface.");
}
public void testJsTypeExtendsNativeJsTypeSucceeds() throws Exception {
diff --git a/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java8Test.java b/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java8Test.java
index d38bf76..a040c85 100644
--- a/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java8Test.java
+++ b/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java8Test.java
@@ -1600,4 +1600,27 @@
}
}.fAsPredicate().apply());
}
+
+ @JsFunction
+ interface JsFunctionInterface {
+ Double m();
+ @JsOverlay
+ default Double callM() {
+ return this.m();
+ }
+ }
+
+ private static native JsFunctionInterface createNative() /*-{
+ return function () { return 5; };
+ }-*/;
+ public void testJsFunction_withOverlay() {
+ JsFunctionInterface f = new JsFunctionInterface() {
+ @Override
+ public Double m() {
+ return new Double(2.0);
+ }
+ };
+ assertEquals(2, f.callM().intValue());
+ assertEquals(5, createNative().callM().intValue());
+ }
}
diff --git a/user/test/com/google/gwt/dev/jjs/test/Java8Test.java b/user/test/com/google/gwt/dev/jjs/test/Java8Test.java
index e435da0..e77792f 100644
--- a/user/test/com/google/gwt/dev/jjs/test/Java8Test.java
+++ b/user/test/com/google/gwt/dev/jjs/test/Java8Test.java
@@ -296,6 +296,10 @@
assertFalse(isGwtSourceLevel8());
}
+ public void testJsFunction_withOverlay() {
+ assertFalse(isGwtSourceLevel8());
+ }
+
private boolean isGwtSourceLevel8() {
return JUnitShell.getCompilerOptions().getSourceLevel().compareTo(SourceLevel.JAVA8) >= 0;
}
