Adds support for static overlay methods.

This patch also makes sure that no JSNI is allowed in native classes. Although JsOverlay JSNI results in correct code, mixing too concepts together results in classes that is difficult to reason about. Change-Id: I397e60d8dd05efdb0a19099fbe8ba49c47d44890
gwtproject · Nov 5, 2015 · 667d21f · 667d21f
1 parent 91fdb93
commit 667d21f
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 51 deletions.
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
@@ -253,6 +253,7 @@ private void checkMethod(Map<String, JsMember> localNames, JMethod method) {
 
     if (method.isJsOverlay()) {
       checkJsOverlay(method);
+      return;
     }
 
     checkUnusableByJs(method);
@@ -362,32 +363,30 @@ private void checkJsOverlay(JMethod method) {
       return;
     }
 
-    if (method.isJsNative() || method.isJsniMethod() || method.isStatic() || !method.isFinal()) {
+    if (method.isJsNative() || (!method.isFinal() && !method.isStatic())) {
       logError(method,
-          "JsOverlay method '%s' cannot be non-final, static, nor native.", methodDescription);
+          "JsOverlay method '%s' cannot be non-final nor native.", methodDescription);
     }
   }
 
   private void checkMemberOfNativeJsType(JMember member) {
+    if (member instanceof JMethod && ((JMethod) member).isJsniMethod()) {
+      logError(member, "JSNI method %s is not allowed in a native JsType.",
+          getMemberDescription(member));
+      return;
+    }
+
     if (member.isSynthetic() || member.isJsNative() || member.isJsOverlay()) {
       return;
     }
 
     if (member.getJsName() == null) {
       logError(member, "Native JsType member %s is not public or has @JsIgnore.",
           getMemberDescription(member));
-      return;
+    } else {
-    }
+      logError(member, "Native JsType method %s should be native or abstract.",
-
+          getMemberDescription(member));
-    JMethod method = (JMethod) member;
-    if (method.isJsniMethod()) {
-      logError(method, "JSNI method %s is not allowed in a native JsType.",
-          getMemberDescription(method));
-      return;
     }
-
-    logError(method, "Native JsType method %s should be native or abstract.",
-        getMemberDescription(method));
   }
 
   private void checkMemberQualifiedJsName(JMember member) {

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
@@ -1356,6 +1356,7 @@ public void testJsOverlayOnNativeJsTypeMemberSucceeds() throws Exception {
     addSnippetClassDecl(
         "@JsType(isNative=true) public static class Buggy {",
         "  @JsOverlay public final void m() { }",
+        "  @JsOverlay public final void m(int x) { }",
         "  @JsOverlay private final void n() { }",
         "  @JsOverlay final void o() { }",
         "  @JsOverlay protected final void p() { }",
@@ -1364,16 +1365,17 @@ public void testJsOverlayOnNativeJsTypeMemberSucceeds() throws Exception {
     assertBuggySucceeds();
   }
 
-  public void testJsOverlayOnStaticFails() {
+  public void testJsOverlayOnStaticSucceds() throws Exception {
     addSnippetImport("jsinterop.annotations.JsType");
     addSnippetImport("jsinterop.annotations.JsOverlay");
     addSnippetClassDecl(
         "@JsType(isNative=true) public static class Buggy {",
-        "  @JsOverlay public static final void m() { }",
+        "  @JsOverlay public static void m() { }",
+        "  @JsOverlay public static void m(int x) { }",
+        "  @JsOverlay private static void m(boolean x) { }",
         "}");
 
-    assertBuggyFails("Line 6: JsOverlay method 'void EntryPoint.Buggy.m()' cannot be "
+    assertBuggySucceeds();
-        + "non-final, static, nor native.");
   }
 
   public void testJsOverlayImplementingInterfaceMethodFails() {
@@ -1415,21 +1417,25 @@ public void testJsOverlayOnNonFinalFails() {
         "}");
 
     assertBuggyFails(
-        "Line 6: JsOverlay method 'void EntryPoint.Buggy.m()' cannot be non-final, "
+        "Line 6: JsOverlay method 'void EntryPoint.Buggy.m()' cannot be non-final nor native.");
-            + "static, nor native.");
   }
 
   public void testJsOverlayOnNativeMethodFails() {
     addSnippetImport("jsinterop.annotations.JsType");
     addSnippetImport("jsinterop.annotations.JsOverlay");
     addSnippetClassDecl(
         "@JsType(isNative=true) public static class Buggy {",
-        "  @JsOverlay public final native void m();",
+        "  @JsOverlay public static final native void m1();",
+        "  @JsOverlay public static final native void m2()/*-{}-*/;",
+        "  @JsOverlay public final native void m3();",
+        "  @JsOverlay public final native void m4()/*-{}-*/;",
         "}");
 
     assertBuggyFails(
-        "Line 6: JsOverlay method 'void EntryPoint.Buggy.m()' cannot be non-final, "
+        "Line 6: JsOverlay method 'void EntryPoint.Buggy.m1()' cannot be non-final nor native.",
-            + "static, nor native.");
+        "Line 7: JSNI method 'void EntryPoint.Buggy.m2()' is not allowed in a native JsType.",
+        "Line 8: JsOverlay method 'void EntryPoint.Buggy.m3()' cannot be non-final nor native.",
+        "Line 9: JSNI method 'void EntryPoint.Buggy.m4()' is not allowed in a native JsType.");
   }
 
   public void testJsOverlayOnJsoMethodSucceeds() throws Exception {
@@ -1593,55 +1599,48 @@ public void testNativeMethodOnJsTypeSucceeds() throws Exception {
     assertBuggySucceeds();
   }
 
-  public void testNativeJsTypeMutlipleConstructorSucceeds() throws Exception {
+  public void testNativeJsTypeOverloadsSucceeds() throws Exception {
     addSnippetImport("jsinterop.annotations.JsType");
     addSnippetClassDecl(
         "@JsType(isNative=true) static class Buggy {",
-        "  public Buggy(int i) { }",
+        "  public static native void m();",
+        "  public static native void m(Object o);",
+        "  public static native void m(String o);",
         "  public Buggy() { }",
+        "  public Buggy(Object o) { }",
+        "  public Buggy(String o) { }",
+        "  public native void n();",
+        "  public native void n(Object o);",
+        "  public native void n(String o);",
         "}");
 
     assertBuggySucceeds();
   }
 
-  public void testNativeJsTypeNonPublicConstructorSucceeds() throws Exception {
+  public void testNativeJsTypeAbstractMethodSucceeds() throws Exception {
     addSnippetImport("jsinterop.annotations.JsType");
     addSnippetClassDecl(
-        "@JsType(isNative=true) static class Buggy {",
+        "@JsType(isNative=true) static abstract class Buggy {",
-        "  Buggy() { }",
+        "  public abstract void m(Object o);",
         "}");
 
     assertBuggySucceeds();
   }
 
-  public void testNativeJsTypeDefaultConstructorSucceeds() throws Exception {
+  public void testNativeJsTypeNonPublicConstructorSucceeds() throws Exception {
     addSnippetImport("jsinterop.annotations.JsType");
     addSnippetClassDecl(
         "@JsType(isNative=true) static class Buggy {",
+        "  Buggy() { }",
         "}");
 
     assertBuggySucceeds();
   }
 
-  public void testNativeJsTypeInstanceMethodOverloadSucceeds() throws Exception {
+  public void testNativeJsTypeDefaultConstructorSucceeds() throws Exception {
-    addSnippetImport("jsinterop.annotations.JsType");
-    addSnippetClassDecl(
-        "@SuppressWarnings(\"unusable-by-js\")",
-        "@JsType(isNative=true) public static class Buggy {",
-        "  public native void m(Object o);",
-        "  public native void m(Object[] o);",
-        "}");
-
-    assertBuggySucceeds();
-  }
-
-  public void testNativeJsTypeStaticMethodOverloadSucceeds() throws Exception {
     addSnippetImport("jsinterop.annotations.JsType");
     addSnippetClassDecl(
-        "@SuppressWarnings(\"unusable-by-js\")",
+        "@JsType(isNative=true) static class Buggy {",
-        "@JsType(isNative=true) public static class Buggy {",
-        "  public static native void m(Object o);",
-        "  public static native void m(Object[] o);",
         "}");
 
     assertBuggySucceeds();

diff --git a/user/test/com/google/gwt/core/client/interop/JsTypeTest.java b/user/test/com/google/gwt/core/client/interop/JsTypeTest.java
@@ -23,6 +23,7 @@
 
 import java.util.Iterator;
 
+import javaemul.internal.annotations.DoNotInline;
 import jsinterop.annotations.JsFunction;
 import jsinterop.annotations.JsOverlay;
 import jsinterop.annotations.JsPackage;
@@ -504,13 +505,21 @@ private static native void setProperty(Object object, String name, int value) /*
     object[name] = value;
   }-*/;
 
-  @JsType(isNative = true)
+  @JsType(isNative = true, namespace = GLOBAL, name = "Object")
   static class NativeJsTypeWithOverlay {
-    public native int m();
 
-    @JsOverlay
+    public static native String[] keys(Object o);
-    public final int callM() {
+
-      return m();
+    @JsOverlay @DoNotInline
+    public static final boolean hasM(Object obj) {
+      return keys(obj)[0].equals("m");
+    }
+
+    public native boolean hasOwnProperty(String name);
+
+    @JsOverlay @DoNotInline
+    public final boolean hasM() {
+      return hasOwnProperty("m");
     }
   }
 
@@ -520,6 +529,7 @@ private native NativeJsTypeWithOverlay createNativeJsTypeWithOverlay() /*-{
 
   public void testNativeJsTypeWithOverlay() {
     NativeJsTypeWithOverlay object = createNativeJsTypeWithOverlay();
-    assertEquals(6, object.callM());
+    assertTrue(object.hasM());
+    assertTrue(NativeJsTypeWithOverlay.hasM(object));
   }
 }