Skip to content

Commit

Permalink
Disallow concrete @JsProperty on static methods.
Browse files Browse the repository at this point in the history 
Only allow native @JsProperty on static methods for now.

Change-Id: I218fa5fe4aec99c86da5a5bba20ed16dee911331
  • Loading branch information
@rluble
rluble authored and Gerrit Code Review committed Jan 8, 2016
1 parent 0a80184 commit 69c37ea
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 33 deletions.
38 changes: 23 additions & 15 deletions dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java
View file
Expand Up @@ -400,33 +400,41 @@ public void endVisit(JsNameRef x, JsContext ctx) {
} }


private boolean checkJsPropertyAccessor(JMember member) { private boolean checkJsPropertyAccessor(JMember member) {
JsMemberType memberType = member.getJsMemberType();

if (member.getJsName().equals(JsInteropUtil.INVALID_JSNAME)) { if (member.getJsName().equals(JsInteropUtil.INVALID_JSNAME)) {
assert member.getJsMemberType().isPropertyAccessor(); assert memberType.isPropertyAccessor();
logError( logError(
member, member,
"JsProperty %s should either follow Java Bean naming conventions or provide a name.", "JsProperty %s should either follow Java Bean naming conventions or provide a name.",
getMemberDescription(member)); getMemberDescription(member));
return false; return false;
} }


if (member.getJsMemberType() == JsMemberType.UNDEFINED_ACCESSOR) { switch (memberType) {
logError(member, "JsProperty %s should have a correct setter or getter signature.", case UNDEFINED_ACCESSOR:
getMemberDescription(member)); logError(member, "JsProperty %s should have a correct setter or getter signature.",
}

if (member.getJsMemberType() == JsMemberType.GETTER) {
if (member.getType() != JPrimitiveType.BOOLEAN && member.getName().startsWith("is")) {
logError(member, "JsProperty %s cannot have a non-boolean return.",
getMemberDescription(member)); getMemberDescription(member));
} break;
case GETTER:
if (member.getType() != JPrimitiveType.BOOLEAN && member.getName().startsWith("is")) {
logError(member, "JsProperty %s cannot have a non-boolean return.",
getMemberDescription(member));
}
break;
case SETTER:
if (((JMethod) member).getParams().get(0).isVarargs()) {
logError(member, "JsProperty %s cannot have a vararg parameter.",
getMemberDescription(member));
}
break;
} }


if (member.getJsMemberType() == JsMemberType.SETTER) { if (memberType.isPropertyAccessor() && member.isStatic() && !member.isJsNative()) {
if (((JMethod) member).getParams().get(0).isVarargs()) { logError(member, "Static property accessor '%s' can only be native.",
logError(member, "JsProperty %s cannot have a vararg parameter.", JjsUtils.getReadableDescription(member));
getMemberDescription(member));
}
} }

return true; return true;
} }


Expand Down
49 changes: 31 additions & 18 deletions dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java
View file
Expand Up @@ -143,13 +143,13 @@ public void testJsPropertyGetterStyleSucceeds() throws Exception {
addSnippetImport("jsinterop.annotations.JsProperty"); addSnippetImport("jsinterop.annotations.JsProperty");
addSnippetClassDecl( addSnippetClassDecl(
"@JsType", "@JsType",
"public interface Buggy {", "public abstract static class Buggy {",
" @JsProperty static int getStaticX(){ return 0;}", " @JsProperty static native int getStaticX();",
" @JsProperty static void setStaticX(int x){}", " @JsProperty static native void setStaticX(int x);",
" @JsProperty int getX();", " @JsProperty abstract int getX();",
" @JsProperty void setX(int x);", " @JsProperty abstract void setX(int x);",
" @JsProperty boolean isY();", " @JsProperty abstract boolean isY();",
" @JsProperty void setY(boolean y);", " @JsProperty abstract void setY(boolean y);",
"}"); "}");


assertBuggySucceeds(); assertBuggySucceeds();
Expand All @@ -159,7 +159,6 @@ public void testJsPropertyIncorrectGetterStyleFails() throws Exception {
addSnippetImport("jsinterop.annotations.JsType"); addSnippetImport("jsinterop.annotations.JsType");
addSnippetImport("jsinterop.annotations.JsProperty"); addSnippetImport("jsinterop.annotations.JsProperty");
addSnippetClassDecl( addSnippetClassDecl(
"@JsType",
"public interface Buggy {", "public interface Buggy {",
" @JsProperty int isX();", " @JsProperty int isX();",
" @JsProperty int getY(int x);", " @JsProperty int getY(int x);",
Expand All @@ -172,20 +171,20 @@ public void testJsPropertyIncorrectGetterStyleFails() throws Exception {
"}"); "}");


assertBuggyFails( assertBuggyFails(
"Line 7: JsProperty 'int EntryPoint.Buggy.isX()' cannot have a non-boolean return.", "Line 6: JsProperty 'int EntryPoint.Buggy.isX()' cannot have a non-boolean return.",
"Line 8: JsProperty 'int EntryPoint.Buggy.getY(int)' should have a correct setter " "Line 7: JsProperty 'int EntryPoint.Buggy.getY(int)' should have a correct setter "
+ "or getter signature.", + "or getter signature.",
"Line 9: JsProperty 'void EntryPoint.Buggy.getZ()' should have a correct setter " "Line 8: JsProperty 'void EntryPoint.Buggy.getZ()' should have a correct setter "
+ "or getter signature.", + "or getter signature.",
"Line 10: JsProperty 'void EntryPoint.Buggy.setX(int, int)' should have a correct setter " "Line 9: JsProperty 'void EntryPoint.Buggy.setX(int, int)' should have a correct setter "
+ "or getter signature.", + "or getter signature.",
"Line 11: JsProperty 'void EntryPoint.Buggy.setY()' should have a correct setter " "Line 10: JsProperty 'void EntryPoint.Buggy.setY()' should have a correct setter "
+ "or getter signature.", + "or getter signature.",
"Line 12: JsProperty 'int EntryPoint.Buggy.setZ(int)' should have a correct setter " "Line 11: JsProperty 'int EntryPoint.Buggy.setZ(int)' should have a correct setter "
+ "or getter signature.", + "or getter signature.",
"Line 13: JsProperty 'void EntryPoint.Buggy.setStatic()' should have a correct setter " "Line 12: JsProperty 'void EntryPoint.Buggy.setStatic()' should have a correct setter "
+ "or getter signature.", + "or getter signature.",
"Line 14: JsProperty 'void EntryPoint.Buggy.setW(int[])' cannot have a vararg parameter."); "Line 13: JsProperty 'void EntryPoint.Buggy.setW(int[])' cannot have a vararg parameter.");
} }


public void testJsPropertyNonGetterStyleFails() throws Exception { public void testJsPropertyNonGetterStyleFails() throws Exception {
Expand Down Expand Up @@ -286,7 +285,8 @@ public void testCollidingJsMethodAndJsPropertySetterFails() throws Exception {
+ "cannot both use the same JavaScript name 'x'."); + "cannot both use the same JavaScript name 'x'.");
} }


public void testCollidingPropertyAccessorExportsFails() throws Exception { // TODO(rluble): enable when static property definitions are implemented.
public void __disabled__testCollidingPropertyAccessorExportsFails() throws Exception {
addSnippetImport("jsinterop.annotations.JsProperty"); addSnippetImport("jsinterop.annotations.JsProperty");
addSnippetClassDecl( addSnippetClassDecl(
"public static class Buggy {", "public static class Buggy {",
Expand Down Expand Up @@ -318,7 +318,8 @@ public void testCollidingMethodExportsFails() throws Exception {
+ "by 'void EntryPoint.Buggy.show()'."); + "by 'void EntryPoint.Buggy.show()'.");
} }


public void testCollidingMethodToPropertyAccessorExportsFails() throws Exception { // TODO(rluble): enable when static property definitions are implemented.
public void __disabled__testCollidingMethodToPropertyAccessorExportsFails() throws Exception {
addSnippetImport("jsinterop.annotations.JsMethod"); addSnippetImport("jsinterop.annotations.JsMethod");
addSnippetImport("jsinterop.annotations.JsProperty"); addSnippetImport("jsinterop.annotations.JsProperty");
addSnippetClassDecl( addSnippetClassDecl(
Expand Down Expand Up @@ -824,6 +825,18 @@ public void testJsPropertySuperCallFails() throws Exception {
"Line 9: Cannot call property accessor 'int EntryPoint.Super.getX()' via super."); "Line 9: Cannot call property accessor 'int EntryPoint.Super.getX()' via super.");
} }


public void testJsPropertyOnStaticMethodFails() {
addSnippetImport("jsinterop.annotations.JsType");
addSnippetImport("jsinterop.annotations.JsProperty");
addSnippetClassDecl(
"@JsType public static class Buggy {",
" @JsProperty public static int getX() { return 0; }",
"}");

assertBuggyFails(
"Line 6: Static property accessor 'int EntryPoint.Buggy.getX()' can only be native.");
}

public void testJsPropertyCallSucceeds() throws Exception { public void testJsPropertyCallSucceeds() throws Exception {
addSnippetImport("jsinterop.annotations.JsType"); addSnippetImport("jsinterop.annotations.JsType");
addSnippetImport("jsinterop.annotations.JsProperty"); addSnippetImport("jsinterop.annotations.JsProperty");
Expand Down

0 comments on commit 69c37ea

Please sign in to comment.