From 69c37ea302a762e10ef1bb62b386dcb4d56ad8db Mon Sep 17 00:00:00 2001 From: Roberto Lublinerman Date: Tue, 13 Oct 2015 11:52:34 -0700 Subject: [PATCH] Disallow concrete @JsProperty on static methods. Only allow native @JsProperty on static methods for now. Change-Id: I218fa5fe4aec99c86da5a5bba20ed16dee911331 --- .../jjs/impl/JsInteropRestrictionChecker.java | 38 ++++++++------ .../impl/JsInteropRestrictionCheckerTest.java | 49 ++++++++++++------- 2 files changed, 54 insertions(+), 33 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 index 899aaf64561..583431ece42 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java @@ -400,8 +400,10 @@ public void endVisit(JsNameRef x, JsContext ctx) { } private boolean checkJsPropertyAccessor(JMember member) { + JsMemberType memberType = member.getJsMemberType(); + if (member.getJsName().equals(JsInteropUtil.INVALID_JSNAME)) { - assert member.getJsMemberType().isPropertyAccessor(); + assert memberType.isPropertyAccessor(); logError( member, "JsProperty %s should either follow Java Bean naming conventions or provide a name.", @@ -409,24 +411,30 @@ private boolean checkJsPropertyAccessor(JMember member) { return false; } - if (member.getJsMemberType() == JsMemberType.UNDEFINED_ACCESSOR) { - logError(member, "JsProperty %s should have a correct setter or getter signature.", - getMemberDescription(member)); - } - - if (member.getJsMemberType() == JsMemberType.GETTER) { - if (member.getType() != JPrimitiveType.BOOLEAN && member.getName().startsWith("is")) { - logError(member, "JsProperty %s cannot have a non-boolean return.", + switch (memberType) { + case UNDEFINED_ACCESSOR: + logError(member, "JsProperty %s should have a correct setter or getter signature.", 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 (((JMethod) member).getParams().get(0).isVarargs()) { - logError(member, "JsProperty %s cannot have a vararg parameter.", - getMemberDescription(member)); - } + if (memberType.isPropertyAccessor() && member.isStatic() && !member.isJsNative()) { + logError(member, "Static property accessor '%s' can only be native.", + JjsUtils.getReadableDescription(member)); } + return true; } 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 index abbff0b5f19..7bd4b60f564 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java @@ -143,13 +143,13 @@ public void testJsPropertyGetterStyleSucceeds() throws Exception { addSnippetImport("jsinterop.annotations.JsProperty"); addSnippetClassDecl( "@JsType", - "public interface Buggy {", - " @JsProperty static int getStaticX(){ return 0;}", - " @JsProperty static void setStaticX(int x){}", - " @JsProperty int getX();", - " @JsProperty void setX(int x);", - " @JsProperty boolean isY();", - " @JsProperty void setY(boolean y);", + "public abstract static class Buggy {", + " @JsProperty static native int getStaticX();", + " @JsProperty static native void setStaticX(int x);", + " @JsProperty abstract int getX();", + " @JsProperty abstract void setX(int x);", + " @JsProperty abstract boolean isY();", + " @JsProperty abstract void setY(boolean y);", "}"); assertBuggySucceeds(); @@ -159,7 +159,6 @@ public void testJsPropertyIncorrectGetterStyleFails() throws Exception { addSnippetImport("jsinterop.annotations.JsType"); addSnippetImport("jsinterop.annotations.JsProperty"); addSnippetClassDecl( - "@JsType", "public interface Buggy {", " @JsProperty int isX();", " @JsProperty int getY(int x);", @@ -172,20 +171,20 @@ public void testJsPropertyIncorrectGetterStyleFails() throws Exception { "}"); assertBuggyFails( - "Line 7: 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 6: JsProperty 'int EntryPoint.Buggy.isX()' cannot have a non-boolean return.", + "Line 7: JsProperty 'int EntryPoint.Buggy.getY(int)' should have a correct setter " + "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.", - "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.", - "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.", - "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.", - "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.", - "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 { @@ -286,7 +285,8 @@ public void testCollidingJsMethodAndJsPropertySetterFails() throws Exception { + "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"); addSnippetClassDecl( "public static class Buggy {", @@ -318,7 +318,8 @@ public void testCollidingMethodExportsFails() throws Exception { + "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.JsProperty"); addSnippetClassDecl( @@ -824,6 +825,18 @@ public void testJsPropertySuperCallFails() throws Exception { "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 { addSnippetImport("jsinterop.annotations.JsType"); addSnippetImport("jsinterop.annotations.JsProperty");