Skip to content

Commit

Permalink
Enforces some "public" requirements for @JsExport.
Browse files Browse the repository at this point in the history 
Starts enforcing that @JsExport is only allowed on static methods when 
they are public, on static fields when they are public and only within 
types that are transitively public (having themselves and their entire 
enclosing type chain as public).

Change-Id: I21195524c5b8a0ebe3ec035e42bdd8b446bf6e6a
Review-Link: https://gwt-review.googlesource.com/#/c/11490/
  • Loading branch information
@stalcup
stalcup committed Feb 4, 2015
1 parent 704bd50 commit 08c6a03
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 16 deletions.
31 changes: 22 additions & 9 deletions dev/core/src/com/google/gwt/dev/javac/JSORestrictionsChecker.java
View file
Expand Up @@ -63,7 +63,8 @@ public class JSORestrictionsChecker {
public static final String ERR_JSTYPE_OVERLOADS_NOT_ALLOWED =
"JsType methods cannot overload another method.";
public static final String ERR_JSEXPORT_ONLY_CTORS_STATIC_METHODS_AND_STATIC_FINAL_FIELDS =
"@JsExport may only be applied to constructors and static methods and static final fields.";
"@JsExport may only be applied to public constructors and static methods and public "
+ "static final fields in public classes.";
public static final String ERR_EITHER_JSEXPORT_JSNOEXPORT =
"@JsExport and @JsNoExport is not allowed at the same time.";
public static final String ERR_JSPROPERTY_ONLY_BEAN_OR_FLUENT_STYLE_NAMING =
Expand Down Expand Up @@ -138,6 +139,7 @@ private class JSORestrictionsVisitor extends SafeASTVisitor implements
ClassFileConstants {

private final Stack<ClassState> classStateStack = new Stack<ClassState>();
private final Stack<SourceTypeBinding> typeBindingStack = new Stack<SourceTypeBinding>();

@Override
public void endVisit(AllocationExpression exp, BlockScope scope) {
Expand Down Expand Up @@ -226,19 +228,19 @@ public void endVisitValid(TypeDeclaration type, BlockScope scope) {

@Override
public boolean visit(TypeDeclaration type, ClassScope scope) {
pushState(checkType(type));
pushState(type);
return true;
}

@Override
public boolean visit(TypeDeclaration type, CompilationUnitScope scope) {
pushState(checkType(type));
pushState(type);
return true;
}

@Override
public boolean visitValid(TypeDeclaration type, BlockScope scope) {
pushState(checkType(type));
pushState(type);
return true;
}

Expand All @@ -263,7 +265,8 @@ private void checkJsType(TypeDeclaration type, TypeBinding typeBinding) {

private void checkJsExport(MethodBinding mb) {
if (JdtUtil.getAnnotation(mb, JsInteropUtil.JSEXPORT_CLASS) != null) {
if (!mb.isConstructor() && !mb.isStatic()) {
boolean isStatic = mb.isConstructor() || mb.isStatic();
if (!areAllEnclosingClassesPublic() || !isStatic || !mb.isPublic()) {
errorOn(mb, ERR_JSEXPORT_ONLY_CTORS_STATIC_METHODS_AND_STATIC_FINAL_FIELDS);
}
if (JdtUtil.getAnnotation(mb, JsInteropUtil.JSNOEXPORT_CLASS) != null) {
Expand All @@ -274,7 +277,7 @@ private void checkJsExport(MethodBinding mb) {

private void checkJsExport(FieldBinding fb) {
if (JdtUtil.getAnnotation(fb, JsInteropUtil.JSEXPORT_CLASS) != null) {
if (!fb.isStatic() || !fb.isFinal()) {
if (!areAllEnclosingClassesPublic() || !fb.isStatic() || !fb.isFinal() || !fb.isPublic()) {
errorOn(fb, ERR_JSEXPORT_ONLY_CTORS_STATIC_METHODS_AND_STATIC_FINAL_FIELDS);
}
if (JdtUtil.getAnnotation(fb, JsInteropUtil.JSNOEXPORT_CLASS) != null) {
Expand Down Expand Up @@ -498,20 +501,30 @@ private ReferenceBinding findNearestJsTypeRecursive(ReferenceBinding binding) {
return null;
}

private boolean areAllEnclosingClassesPublic() {
for (SourceTypeBinding typeBinding : typeBindingStack) {
if (!typeBinding.isPublic()) {
return false;
}
}
return true;
}

private boolean isJso() {
return classStateStack.peek() == ClassState.JSO;
}

private void popState() {
classStateStack.pop();
typeBindingStack.pop();
}

private void pushState(ClassState cstate) {
classStateStack.push(cstate);
private void pushState(TypeDeclaration type) {
classStateStack.push(checkType(type));
typeBindingStack.push(type.binding);
}
}


/**
* Checks an entire
* {@link org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration}.
Expand Down
50 changes: 45 additions & 5 deletions dev/core/test/com/google/gwt/dev/javac/JSORestrictionsTest.java
View file
Expand Up @@ -384,13 +384,16 @@ public void testJsExport() {
StringBuilder goodCode = new StringBuilder();
goodCode.append("import com.google.gwt.core.client.js.JsExport;\n");
goodCode.append("public class Buggy {\n");
goodCode.append("@JsExport public static final String field = null;\n");
goodCode.append("@JsExport public static void method() {}\n");
goodCode.append("public interface Foo {\n");
goodCode.append("@JsExport String field1 = null;\n");
goodCode.append(" @JsExport public static final String field = null;\n");
goodCode.append(" @JsExport public static void method() {}\n");
goodCode.append(" public interface Foo {\n");
goodCode.append(" @JsExport String field1 = null;\n");
goodCode.append(" interface ImplicitlyPublicInner {\n");
goodCode.append(" @JsExport String field2 = null;\n");
goodCode.append(" }\n");
// TODO: enable after java 8 becomes default
// goodCode.append("@JsExport static void method1() {}\n");
goodCode.append("}\n");
goodCode.append(" }\n");
goodCode.append("}\n");

shouldGenerateNoError(goodCode);
Expand All @@ -412,6 +415,43 @@ public void testJsExportOnInterface() {
shouldGenerateNoError(goodCode);
}

public void testJsExportNotOnNonPublicClass() {
StringBuilder buggyCode = new StringBuilder();
buggyCode.append("import com.google.gwt.core.client.js.JsExport;\n");
buggyCode.append("public class Buggy {\n");
buggyCode.append(" private static class PrivateNested {\n");
buggyCode.append(" public static class PublicNested {\n");
buggyCode.append(" @JsExport public static Object foo() {return null;}\n");
buggyCode.append(" }\n");
buggyCode.append(" }\n");
buggyCode.append("}\n");

shouldGenerateError(buggyCode, "Line 5: "
+ JSORestrictionsChecker.ERR_JSEXPORT_ONLY_CTORS_STATIC_METHODS_AND_STATIC_FINAL_FIELDS);
}

public void testJsExportNotOnNonPublicField() {
StringBuilder buggyCode = new StringBuilder();
buggyCode.append("import com.google.gwt.core.client.js.JsExport;\n");
buggyCode.append("public class Buggy {\n");
buggyCode.append("@JsExport final static String foo = null;\n");
buggyCode.append("}\n");

shouldGenerateError(buggyCode, "Line 3: "
+ JSORestrictionsChecker.ERR_JSEXPORT_ONLY_CTORS_STATIC_METHODS_AND_STATIC_FINAL_FIELDS);
}

public void testJsExportNotOnNonPublicMethod() {
StringBuilder buggyCode = new StringBuilder();
buggyCode.append("import com.google.gwt.core.client.js.JsExport;\n");
buggyCode.append("public class Buggy {\n");
buggyCode.append("@JsExport Object foo() {return null;}\n");
buggyCode.append("}\n");

shouldGenerateError(buggyCode, "Line 3: "
+ JSORestrictionsChecker.ERR_JSEXPORT_ONLY_CTORS_STATIC_METHODS_AND_STATIC_FINAL_FIELDS);
}

public void testJsExportNotOnObjectMethod() {
StringBuilder buggyCode = new StringBuilder();
buggyCode.append("import com.google.gwt.core.client.js.JsExport;\n");
Expand Down
13 changes: 11 additions & 2 deletions user/test/com/google/gwt/core/client/interop/MyClassExportsMethod.java
View file
Expand Up @@ -17,7 +17,10 @@

import com.google.gwt.core.client.js.JsExport;

class MyClassExportsMethod {
/**
* A test class that exhibits a variety of @JsExports.
*/
public class MyClassExportsMethod {
public static boolean calledFromJs = false;

@JsExport("exportedFromJava")
Expand All @@ -28,6 +31,9 @@ public static void callMe() {
static boolean calledFromFoo = false;
static boolean calledFromBar = false;

/**
* Static inner class A.
*/
public static class A {
public void bar() {
calledFromBar = true;
Expand All @@ -38,6 +44,9 @@ public void foo() {
}
}

/**
* Static inner subclass of A.
*/
public static class SubclassOfA extends A {
@Override
public void foo() { }
Expand All @@ -56,7 +65,7 @@ public static void callFoo(A a) {
}

@JsExport("newA")
private static A newA() {
public static A newA() {
return new A();
}
}

0 comments on commit 08c6a03

Please sign in to comment.