Disallow @jstype in local classes and native @jstype in proper inner …

…classes. Change-Id: I1478898d0f2ddddefdcbb895e0d336685ee9cbc8
gwtproject · Nov 3, 2015 · 2517ed5 · 2517ed5
1 parent ef18385
commit 2517ed5
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 19 deletions.
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
@@ -1458,8 +1458,7 @@ private void generateExports() {
           continue;
         }
 
-        if (type.isJsType() && !type.getClassDisposition().isLocalType()) {
+        if (type.isJsType()) {
-          // only types with explicit source names in Java may have an exported prototype
           exportedMembersByExportName.put(type.getQualifiedJsName(), type);
         }
 

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
@@ -25,6 +25,7 @@
 import com.google.gwt.dev.jjs.ast.JConstructor;
 import com.google.gwt.dev.jjs.ast.JDeclarationStatement;
 import com.google.gwt.dev.jjs.ast.JDeclaredType;
+import com.google.gwt.dev.jjs.ast.JDeclaredType.NestedClassDisposition;
 import com.google.gwt.dev.jjs.ast.JExpression;
 import com.google.gwt.dev.jjs.ast.JExpressionStatement;
 import com.google.gwt.dev.jjs.ast.JField;
@@ -413,12 +414,33 @@ public boolean visit(JInstanceOf x, Context ctx) {
     }.accept(jprogram);
   }
 
-  private void checkNativeJsType(JDeclaredType type) {
+  private boolean checkJsType(JDeclaredType type) {
+    // Java (at least up to Java 8) does not allow to annotate anonymous classes or lambdas; if
+    // it ever becomes possible we should emit an error.
+    assert type.getClassDisposition() != NestedClassDisposition.ANONYMOUS
+        && type.getClassDisposition() != NestedClassDisposition.LAMBDA;
+
+    if  (type.getClassDisposition() == NestedClassDisposition.LOCAL) {
+      logError("Local class '%s' cannot be a JsType.", type);
+      return false;
+    }
+
+    return true;
+  }
+
+  private boolean checkNativeJsType(JDeclaredType type) {
     // TODO(rluble): add inheritance restrictions.
+
     if (type.isEnumOrSubclass() != null) {
       logError("Enum '%s' cannot be a native JsType.", type);
-      return;
+      return false;
     }
+
+    if (type.getClassDisposition() == NestedClassDisposition.INNER) {
+      logError("Non static inner class '%s' cannot be a native JsType.", type);
+      return false;
+    }
+
     if (!isClinitEmpty(type)) {
       logError("Native JsType '%s' cannot have static initializer.", type);
     }
@@ -429,6 +451,7 @@ private void checkNativeJsType(JDeclaredType type) {
             getMemberDescription(constructor));
       }
     }
+    return true;
   }
 
   private void checkJsFunction(JDeclaredType type) {
@@ -489,8 +512,15 @@ private boolean checkProgram(TreeLogger logger) {
   private void checkType(JDeclaredType type) {
     minimalRebuildCache.removeExportedNames(type.getName());
 
+    if (type.isJsType()) {
+      if (!checkJsType(type)) {
+        return;
+      }
+    }
     if (type.isJsNative()) {
-      checkNativeJsType(type);
+      if (!checkNativeJsType(type)) {
+        return;
+      }
     }
 
     if (type.isJsFunction()) {

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
@@ -1156,6 +1156,32 @@ public void testNativeJsTypeEnumFails() {
         "Line 4: Enum 'EntryPoint.Buggy' cannot be a native JsType.");
   }
 
+  public void testInnerNativeJsTypeFails() {
+    addSnippetImport("jsinterop.annotations.JsType");
+    addSnippetClassDecl(
+        "@JsType(isNative=true) public class Buggy { }");
+
+    assertBuggyFails(
+        "Line 4: Non static inner class 'EntryPoint.Buggy' cannot be a native JsType.");
+  }
+
+  public void testInnerJsTypeSucceeds() throws Exception {
+    addSnippetImport("jsinterop.annotations.JsType");
+    addSnippetClassDecl(
+        "@SuppressWarnings(\"unusable-by-js\") @JsType public class Buggy { }");
+
+    assertBuggySucceeds();
+  }
+
+  public void testLocalJsTypeFails() {
+    addSnippetImport("jsinterop.annotations.JsType");
+    addSnippetClassDecl(
+        "public class Buggy { void m() { @JsType class Local {} } }");
+
+    assertBuggyFails(
+        "Line 4: Local class 'EntryPoint.Buggy.1Local' cannot be a JsType.");
+  }
+
   public void testNativeJsTypeInterfaceCompileTimeConstantSucceeds() throws Exception {
     addSnippetImport("jsinterop.annotations.JsType");
     addSnippetClassDecl(

diff --git a/user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java b/user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java
@@ -314,25 +314,26 @@ public native void testMultipleClassLiteralReferences() /*-{
    * <p>
    * Typetightener used to incorrectly tighten method calls marked with STATIC_DISPATCH_ONLY.
    */
+
   public void testIncorrectDispatch() {
-    final int[] state = new int[1];
+    state = new int[1];
+    new B().m();
+    assertEquals(1, state[0]);
+  }
 
-    @JsType
+  static int[] state;
-    abstract class A {
+  @JsType
-      public void m() {
+  abstract static class A {
-        state[0] = 1;
+    public void m() {
-      }
+      state[0] = 1;
     }
+  }
 
-    @JsType
+  @JsType
-    class B extends A {
+  static class B extends A {
-      public void m() {
+    public void m() {
-        super.m();
+      super.m();
-      }
     }
-
-    new B().m();
-    assertEquals(1, state[0]);
   }
 
   private static void assertEqualContents(float[] expected, float[] actual) {