Fix for illegal method calls in JSNI.
Methods that are devirtualized or require trampoline dispatch are not
supported in JSNI.
Bug: #9356
Bug-Link: http://github.com/gwtproject/gwt/issues/9356
Change-Id: I3b4ccee4ef158be3b87ce16cddfbade2cfd65446
diff --git a/dev/core/src/com/google/gwt/dev/javac/testing/impl/JavaResourceBase.java b/dev/core/src/com/google/gwt/dev/javac/testing/impl/JavaResourceBase.java
index 022f114..46bc6f5 100644
--- a/dev/core/src/com/google/gwt/dev/javac/testing/impl/JavaResourceBase.java
+++ b/dev/core/src/com/google/gwt/dev/javac/testing/impl/JavaResourceBase.java
@@ -259,6 +259,7 @@
createMockJavaResource("java.lang.Number",
"package java.lang;",
"public class Number implements java.io.Serializable {",
+ " public double doubleValue() { return 0; }",
"}");
public static final MockJavaResource OBJECT =
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/JsniRestrictionChecker.java b/dev/core/src/com/google/gwt/dev/jjs/impl/JsniRestrictionChecker.java
index 92218c2..6ba6624 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/JsniRestrictionChecker.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/JsniRestrictionChecker.java
@@ -16,13 +16,18 @@
import com.google.gwt.core.ext.TreeLogger;
import com.google.gwt.core.ext.TreeLogger.Type;
import com.google.gwt.core.ext.UnableToCompleteException;
+import com.google.gwt.dev.jjs.HasSourceInfo;
import com.google.gwt.dev.jjs.ast.Context;
import com.google.gwt.dev.jjs.ast.JDeclaredType;
+import com.google.gwt.dev.jjs.ast.JInterfaceType;
import com.google.gwt.dev.jjs.ast.JMethod;
import com.google.gwt.dev.jjs.ast.JProgram;
import com.google.gwt.dev.jjs.ast.JVisitor;
import com.google.gwt.dev.jjs.ast.js.JsniMethodBody;
import com.google.gwt.dev.jjs.ast.js.JsniMethodRef;
+import com.google.gwt.thirdparty.guava.common.collect.Sets;
+
+import java.util.Set;
/**
* Checks and throws errors for invalid JSNI constructs.
@@ -39,18 +44,43 @@
private JMethod currentJsniMethod;
private final JProgram jprogram;
- private final TreeLogger logger;
+ private final Set<JDeclaredType> typesRequiringTrampolineDispatch;
+ private TreeLogger logger;
private boolean hasErrors;
public JsniRestrictionChecker(TreeLogger logger, JProgram jprogram) {
this.logger = logger;
this.jprogram = jprogram;
+ this.typesRequiringTrampolineDispatch = Sets.newHashSet();
+ for (JDeclaredType type : jprogram.getRepresentedAsNativeTypes()) {
+ collectAllSuperTypes(type , typesRequiringTrampolineDispatch);
+ }
+ }
+
+ private void collectAllSuperTypes(JDeclaredType type, Set<JDeclaredType> allSuperTypes) {
+ if (type.getSuperClass() != null) {
+ allSuperTypes.add(type.getSuperClass());
+ collectAllSuperTypes(type.getSuperClass(), allSuperTypes);
+ }
+ for (JInterfaceType interfaceType : type.getImplements()) {
+ allSuperTypes.add(interfaceType);
+ collectAllSuperTypes(interfaceType, allSuperTypes);
+ }
+ }
+
+ @Override
+ public boolean visit(JDeclaredType x, Context ctx) {
+ TreeLogger currentLogger = this.logger;
+ this.logger = this.logger.branch(Type.INFO, "Errors in " + x.getSourceInfo().getFileName());
+ accept(x.getMethods());
+ this.logger = currentLogger;
+ return false;
}
@Override
public boolean visit(JsniMethodBody x, Context ctx) {
currentJsniMethod = x.getMethod();
- return super.visit(x, ctx);
+ return true;
}
@Override
@@ -59,35 +89,48 @@
JDeclaredType enclosingTypeOfCalledMethod = calledMethod.getEnclosingType();
if (isNonStaticJsoClassDispatch(calledMethod, enclosingTypeOfCalledMethod)) {
- logError("JSNI method %s attempts to call non-static method %s on an instance which is a "
+ logError(x, "JSNI method %s calls non-static method %s on an instance which is a "
+ "subclass of JavaScriptObject. Only static method calls on JavaScriptObject subclasses "
- + "are allowed in JSNI.", currentJsniMethod.getQualifiedName(),
+ + "are allowed in JSNI.",
+ currentJsniMethod.getQualifiedName(),
calledMethod.getQualifiedName());
} else if (isJsoInterface(enclosingTypeOfCalledMethod)) {
- logError("JSNI method %s attempts to call method %s on an instance which might be a "
+ logError(x, "JSNI method %s calls method %s on an instance which might be a "
+ "JavaScriptObject. Such a method call is only allowed in pure Java (non-JSNI) "
- + "functions.", currentJsniMethod.getQualifiedName(), calledMethod.getQualifiedName());
- } else if (jprogram.isJavaLangString(enclosingTypeOfCalledMethod) && !calledMethod.isStatic()) {
- logError("JSNI method %s attempts to call method %s. Only static methods from " +
- "java.lang.String can be called from JSNI.",
+ + "functions.",
currentJsniMethod.getQualifiedName(),
calledMethod.getQualifiedName());
- } else if (jprogram.isJavaLangObject(enclosingTypeOfCalledMethod) && !calledMethod.isStatic()) {
- log(Type.WARN, "JSNI method %s calls method %s. Instance java.lang.Object methods should " +
- "not be called on String, Array or JSO instances.",
+ } else if (jprogram.isRepresentedAsNativeJsPrimitive(enclosingTypeOfCalledMethod)
+ && !calledMethod.isStatic()
+ && !calledMethod.isConstructor()) {
+ logError(x, "JSNI method %s calls method %s. Instance methods on %s "
+ + "cannot be called from JSNI.",
currentJsniMethod.getQualifiedName(),
- calledMethod.getQualifiedName());
+ calledMethod.getQualifiedName(),
+ enclosingTypeOfCalledMethod.getName());
+ } else if (typesRequiringTrampolineDispatch.contains(enclosingTypeOfCalledMethod)
+ && !calledMethod.isStatic()
+ && !calledMethod.isConstructor()) {
+ log(x, Type.WARN, "JSNI method %s calls method %s. Instance methods from %s should "
+ + "not be called on Boolean, Double, String, Array or JSO instances from JSNI.",
+ currentJsniMethod.getQualifiedName(),
+ calledMethod.getQualifiedName(),
+ enclosingTypeOfCalledMethod.getName());
}
- return super.visit(x, ctx);
+ return true;
}
- private void log(Type type, String format, Object... args) {
- logger.log(type, String.format(format, args));
+ private void log(HasSourceInfo hasSourceInfo, Type type, String format, Object... args) {
+ logger.log(type, String.format(
+ String.format("Line %d: %s",
+ hasSourceInfo.getSourceInfo().getStartLine(),
+ format),
+ args));
hasErrors |= type == Type.ERROR;
}
- private void logError(String format, Object... args) {
- log(Type.ERROR, format, args);
+ private void logError(HasSourceInfo hasSourceInfo, String format, Object... args) {
+ log(hasSourceInfo, Type.ERROR, format, args);
}
private boolean isJsoInterface(JDeclaredType type) {
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/JsniRestrictionCheckerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/JsniRestrictionCheckerTest.java
index cc27849..5ad8740 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/JsniRestrictionCheckerTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/JsniRestrictionCheckerTest.java
@@ -25,6 +25,60 @@
*/
public class JsniRestrictionCheckerTest extends OptimizerTestBase {
+ public void testConstructorsOnDevirtualizedTypesSucceeds() throws Exception {
+ addSnippetImport("com.google.gwt.core.client.JavaScriptObject");
+ addSnippetClassDecl(
+ "static class Buggy {",
+ " native void jsniMethod(Object o) /*-{",
+ " @java.lang.Double::new(D)();",
+ " @java.lang.Boolean::new(Z)();",
+ " }-*/;",
+ "}");
+
+ assertCompileSucceeds("new Buggy().jsniMethod(null);");
+ }
+
+ public void testInstanceCallToDevirtualizedFails() throws Exception {
+ addSnippetImport("com.google.gwt.core.client.JavaScriptObject");
+ addSnippetClassDecl(
+ "static class Buggy {",
+ " native void jsniMethod(Object o) /*-{",
+ " new Object().@java.lang.Double::doubleValue()();",
+ " }-*/;",
+ "}");
+
+ assertCompileFails("new Buggy().jsniMethod(null);",
+ "Line 6: JSNI method test.EntryPoint$Buggy.jsniMethod(Ljava/lang/Object;)V calls method "
+ + "java.lang.Double.doubleValue()D. Instance methods on java.lang.Double cannot be "
+ + "called from JSNI.");
+ }
+
+ public void testInstanceCallToTrampolineWarns() throws Exception {
+ addSnippetImport("com.google.gwt.core.client.JavaScriptObject");
+ addSnippetClassDecl(
+ "static class Buggy {",
+ " native void jsniMethod(Object o) /*-{",
+ " new Object().@java.lang.Number::doubleValue()();",
+ " new Object().@java.lang.CharSequence::charAt(I)(0);",
+ " \"Hello\".@java.lang.Object::toString()();",
+ " }-*/;",
+ "}");
+
+ assertCompileSucceeds("new Buggy().jsniMethod(null);",
+ "Line 6: JSNI method test.EntryPoint$Buggy.jsniMethod(Ljava/lang/Object;)V calls method "
+ + "java.lang.Number.doubleValue()D. "
+ + "Instance methods from java.lang.Number should not be called on "
+ + "Boolean, Double, String, Array or JSO instances from JSNI.",
+ "Line 7: JSNI method test.EntryPoint$Buggy.jsniMethod(Ljava/lang/Object;)V calls method "
+ + "java.lang.CharSequence.charAt(I)C. "
+ + "Instance methods from java.lang.CharSequence should not be called on "
+ + "Boolean, Double, String, Array or JSO instances from JSNI.",
+ "Line 8: JSNI method test.EntryPoint$Buggy.jsniMethod(Ljava/lang/Object;)V calls method "
+ + "java.lang.Object.toString()Ljava/lang/String;. "
+ + "Instance methods from java.lang.Object should not be called on "
+ + "Boolean, Double, String, Array or JSO instances from JSNI.");
+ }
+
public void testStaticJsoDispatchSucceeds() throws Exception {
addSnippetImport("com.google.gwt.core.client.JavaScriptObject");
addSnippetClassDecl(
@@ -52,13 +106,13 @@
" protected Foo() { };",
" public void foo() { };",
" }",
- " native void jsniMeth(Object o) /*-{",
+ " native void jsniMethod(Object o) /*-{",
" new Object().@Buggy.IFoo::foo()();",
" }-*/;",
"}");
- assertCompileFails("new Buggy().jsniMeth(null);",
- "JSNI method test.EntryPoint$Buggy.jsniMeth(Ljava/lang/Object;)V attempts to call " +
+ assertCompileFails("new Buggy().jsniMethod(null);",
+ "Line 13: JSNI method test.EntryPoint$Buggy.jsniMethod(Ljava/lang/Object;)V calls " +
"method test.EntryPoint$Buggy$IFoo.foo()V on an instance which might be a " +
"JavaScriptObject. Such a method call is only allowed in pure Java " +
"(non-JSNI) functions.");
@@ -68,13 +122,13 @@
addSnippetImport("com.google.gwt.core.client.JavaScriptObject");
addSnippetClassDecl(
"static class Buggy {",
- " native void jsniMeth(Object o) /*-{",
+ " native void jsniMethod(Object o) /*-{",
" new Object().@com.google.gwt.core.client.JavaScriptObject::toString()();",
" }-*/;",
"}");
- assertCompileFails("new Buggy().jsniMeth(null);",
- "JSNI method test.EntryPoint$Buggy.jsniMeth(Ljava/lang/Object;)V attempts to call " +
+ assertCompileFails("new Buggy().jsniMethod(null);",
+ "Line 6: JSNI method test.EntryPoint$Buggy.jsniMethod(Ljava/lang/Object;)V calls " +
"non-static method " +
"com.google.gwt.core.client.JavaScriptObject.toString()Ljava/lang/String; on an " +
"instance which is a subclass of JavaScriptObject. Only static method calls on " +
@@ -89,13 +143,13 @@
" protected Foo() { };",
" void foo() { };",
" }",
- " native void jsniMeth(Object o) /*-{",
+ " native void jsniMethod(Object o) /*-{",
" new Object().@Buggy.Foo::foo()();",
" }-*/;",
"}");
- assertCompileFails("new Buggy().jsniMeth(null);",
- "JSNI method test.EntryPoint$Buggy.jsniMeth(Ljava/lang/Object;)V attempts to call " +
+ assertCompileFails("new Buggy().jsniMethod(null);",
+ "Line 10: JSNI method test.EntryPoint$Buggy.jsniMethod(Ljava/lang/Object;)V calls " +
"non-static method test.EntryPoint$Buggy$Foo.foo()V on an instance which is a " +
"subclass of JavaScriptObject. Only static method calls on JavaScriptObject " +
"subclasses are allowed in JSNI.");
@@ -105,14 +159,14 @@
addSnippetClassDecl(
"static class Buggy {",
" static String foo;",
- " native void jsniMeth(Object o) /*-{",
+ " native void jsniMethod(Object o) /*-{",
" \"Hello\".@java.lang.String::length()();",
" }-*/;",
"}");
- assertCompileFails("new Buggy().jsniMeth(null);",
- "JSNI method test.EntryPoint$Buggy.jsniMeth(Ljava/lang/Object;)V attempts to call method " +
- "java.lang.String.length()I. Only static methods from java.lang.String can be called " +
+ assertCompileFails("new Buggy().jsniMethod(null);",
+ "Line 6: JSNI method test.EntryPoint$Buggy.jsniMethod(Ljava/lang/Object;)V calls method " +
+ "java.lang.String.length()I. Instance methods on java.lang.String cannot be called " +
"from JSNI.");
}
@@ -120,28 +174,12 @@
addSnippetClassDecl(
"static class Buggy {",
" static String foo;",
- " native void jsniMeth(Object o) /*-{",
+ " native void jsniMethod(Object o) /*-{",
" @java.lang.String::valueOf(Z)();",
" }-*/;",
"}");
- assertCompileSucceeds("new Buggy().jsniMeth(null);");
- }
-
- public void testObjectInstanceMethodCallWarn() throws Exception {
- addSnippetClassDecl(
- "static class Buggy {",
- " static String foo;",
- " native void jsniMeth(Object o) /*-{",
- " \"Hello\".@java.lang.Object::toString()();",
- " }-*/;",
- "}");
-
- assertCompileSucceeds("new Buggy().jsniMeth(null);",
- "JSNI method test.EntryPoint$Buggy.jsniMeth(Ljava/lang/Object;)V calls method " +
- "java.lang.Object.toString()Ljava/lang/String;. " +
- "Instance java.lang.Object methods should not be called on String, Array or " +
- "JSO instances.");
+ assertCompileSucceeds("new Buggy().jsniMethod(null);");
}
@Override
diff --git a/user/test/com/google/gwt/dev/jjs/test/JsniConstructorTest.java b/user/test/com/google/gwt/dev/jjs/test/JsniConstructorTest.java
index 612af22..926f4fb 100644
--- a/user/test/com/google/gwt/dev/jjs/test/JsniConstructorTest.java
+++ b/user/test/com/google/gwt/dev/jjs/test/JsniConstructorTest.java
@@ -188,6 +188,20 @@
assertEquals(o2.foo() + 1, i2.foo());
}
+ public native void testJsniDevirtualizedConstructors() /*-{
+ var aDoubleConstructor = @java.lang.Double::new(Ljava/lang/String;);
+ @junit.framework.Assert::assertEquals(DDD)(1.5, aDoubleConstructor("1.5"), 0.1);
+ @junit.framework.Assert::assertEquals(DDD)(1.5,
+ @java.lang.Double::new(Ljava/lang/String;)("1.5"), 0.1);
+
+ var aStringConstructor = @java.lang.String::new(Ljava/lang/String;);
+ @junit.framework.Assert::assertEquals(Ljava/lang/String;Ljava/lang/String;)(
+ "Hello", aStringConstructor("Hello"))
+ @junit.framework.Assert::assertEquals(Ljava/lang/String;Ljava/lang/String;)(
+ "Hello", @java.lang.String::new(Ljava/lang/String;)("Hello"))
+
+ }-*/;
+
private native InstanceObject instanceArg(StaticObject obj, int i) /*-{
return @com.google.gwt.dev.jjs.test.StaticObject.InstanceObject::new(Lcom/google/gwt/dev/jjs/test/StaticObject;I)(obj,i);
}-*/;
