Require JsFunction implementations be marked final.

The requirement does not affect current expressivity as there is a restriction in place that JsFunction implementations can not be subclassed. Change-Id: I45ab86af287e746e4ad5d0c0a8fc8c508a52bc9c
gwtproject · Jun 17, 2016 · 4c8cdea · 4c8cdea
1 parent bbca228
commit 4c8cdea
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 26 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
@@ -688,6 +688,10 @@ private void checkJsFunction(JDeclaredType type) {
   }
 
   private void checkJsFunctionImplementation(JDeclaredType type) {
+    if (!type.isFinal()) {
+      logError("JsFunction implementation '%s' must be final.",
+          type);
+    }
     if (type.getImplements().size() != 1) {
       logError("JsFunction implementation '%s' cannot implement more than one interface.",
           type);
@@ -704,11 +708,6 @@ private void checkJsFunctionImplementation(JDeclaredType type) {
   }
 
   private void checkJsFunctionSubtype(JDeclaredType type) {
-    JClassType superClass = type.getSuperClass();
-    if (superClass != null && superClass.isJsFunctionImplementation()) {
-      logError(type, "'%s' cannot extend JsFunction implementation '%s'.",
-          JjsUtils.getReadableDescription(type), JjsUtils.getReadableDescription(superClass));
-    }
     for (JInterfaceType superInterface : type.getImplements()) {
       if (superInterface.isJsFunction()) {
         logError(type, "'%s' cannot extend JsFunction '%s'.",

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
@@ -1140,17 +1140,6 @@ public void testSingleJsTypeSucceeds() throws Exception {
     assertBuggySucceeds();
   }
 
-  public void testJsFunctionWithNoExtendsSucceeds() throws Exception {
-    addSnippetImport("jsinterop.annotations.JsFunction");
-    addSnippetClassDecl(
-        "@JsFunction",
-        "public interface Buggy {",
-        "  void foo();",
-        "}");
-
-    assertBuggySucceeds();
-  }
-
   public void testJsFunctionExtendsInterfaceFails() throws Exception {
     addSnippetImport("jsinterop.annotations.JsFunction");
     addSnippetClassDecl(
@@ -1185,11 +1174,19 @@ public void testJsFunctionMarkedAsJsTypeFails() throws Exception {
         "Line 6: 'EntryPoint.Buggy' cannot be both a JsFunction and a JsType at the same time.");
   }
 
-  public void testJsFunctionImplementationWithSingleInterfaceSucceeds() throws Exception {
+  public void testJsFunctionImplementationSucceeds() throws Exception {
-    addAll(jsFunctionInterface);
+    addSnippetImport("jsinterop.annotations.JsFunction");
     addSnippetClassDecl(
-        "public static class Buggy implements MyJsFunctionInterface {",
+        "@JsFunction",
-        "  public int foo(int x) { return 0; }",
+        "public interface Function {",
+        "  void foo();",
+        "}",
+        "public static final class Buggy implements Function {",
+        "  public void foo() {",
+        "    new Function() {",
+        "       public void foo() {}",
+        "    }.foo();",
+        "  }",
         "}");
 
     assertBuggySucceeds();
@@ -1199,7 +1196,7 @@ public void testJsFunctionImplementationWithMultipleSuperInterfacesFails() throw
     addAll(jsFunctionInterface);
     addSnippetClassDecl(
         "interface AnotherInterface {}",
-        "public static class Buggy implements MyJsFunctionInterface, AnotherInterface {",
+        "public static final class Buggy implements MyJsFunctionInterface, AnotherInterface {",
         "  public int foo(int x) { return 0; }",
         "  public int bar(int x) { return 0; }",
         "}");
@@ -1212,15 +1209,15 @@ public void testJsFunctionImplementationWithSuperClassFails() throws Exception {
     addAll(jsFunctionInterface);
     addSnippetClassDecl(
         "public static class BaseClass {}",
-        "public static class Buggy extends BaseClass implements MyJsFunctionInterface {",
+        "public static final class Buggy extends BaseClass implements MyJsFunctionInterface {",
         "  public int foo(int x) { return 0; }",
         "}");
 
     assertBuggyFails("Line 4: JsFunction implementation 'EntryPoint.Buggy' cannot "
         + "extend a class.");
   }
 
-  public void testJsFunctionImplementationWithSubclassesFails() throws Exception {
+  public void testJsFunctionImplementationNotFinalFails() throws Exception {
     addAll(jsFunctionInterface);
     addSnippetClassDecl(
         "public static class BaseClass implements MyJsFunctionInterface {",
@@ -1229,16 +1226,15 @@ public void testJsFunctionImplementationWithSubclassesFails() throws Exception {
         "public static class Buggy extends BaseClass  {",
         "}");
 
-    assertBuggyFails("Line 6: 'EntryPoint.Buggy' cannot extend "
+    assertBuggyFails("Line 3: JsFunction implementation 'EntryPoint.BaseClass' must be final.");
-        + "JsFunction implementation 'EntryPoint.BaseClass'.");
   }
 
   public void testJsFunctionImplementationMarkedAsJsTypeFails() throws Exception {
     addAll(jsFunctionInterface);
     addSnippetImport("jsinterop.annotations.JsType");
     addSnippetClassDecl(
         "@JsType",
-        "public static class Buggy implements MyJsFunctionInterface {",
+        "public static final class Buggy implements MyJsFunctionInterface {",
         "  public int foo(int x) { return 0; }",
         "}");