JsOptional restriction checks
Change-Id: Ic9ce57e392f9208ae316916bee4e32b4718be45f
diff --git a/dev/core/src/com/google/gwt/dev/javac/JsInteropUtil.java b/dev/core/src/com/google/gwt/dev/javac/JsInteropUtil.java
index 020ce6e..e15b624 100644
--- a/dev/core/src/com/google/gwt/dev/javac/JsInteropUtil.java
+++ b/dev/core/src/com/google/gwt/dev/javac/JsInteropUtil.java
@@ -21,6 +21,7 @@
import com.google.gwt.dev.jjs.ast.JField;
import com.google.gwt.dev.jjs.ast.JMember;
import com.google.gwt.dev.jjs.ast.JMethod;
+import com.google.gwt.dev.jjs.ast.JParameter;
import com.google.gwt.dev.jjs.ast.JPrimitiveType;
import org.eclipse.jdt.internal.compiler.ast.Annotation;
@@ -68,6 +69,12 @@
setJsInteropProperties(method, annotations, annotation, isPropertyAccessor, generateExport);
}
+ public static void maybeSetJsInteropProperties(JParameter parameter, Annotation... annotations) {
+ if (getInteropAnnotation(annotations, "JsOptional") != null) {
+ parameter.setOptional();
+ }
+ }
+
public static void maybeSetJsInteropProperties(
JField field, boolean generateExport, Annotation... annotations) {
AnnotationBinding annotation = getInteropAnnotation(annotations, "JsProperty");
@@ -86,7 +93,8 @@
boolean isPublicMemberForJsType = member.getEnclosingType().isJsType() && member.isPublic();
boolean memberForNativeType = member.getEnclosingType().isJsNative();
- if (!isPublicMemberForJsType && !memberForNativeType && memberAnnotation == null) {
+ if (memberAnnotation == null
+ && (!isPublicMemberForJsType && !memberForNativeType || member.isJsOverlay())) {
return;
}
diff --git a/dev/core/src/com/google/gwt/dev/javac/testing/impl/JavaResourceBase.java b/dev/core/src/com/google/gwt/dev/javac/testing/impl/JavaResourceBase.java
index 37d7e7a..022f114 100644
--- a/dev/core/src/com/google/gwt/dev/javac/testing/impl/JavaResourceBase.java
+++ b/dev/core/src/com/google/gwt/dev/javac/testing/impl/JavaResourceBase.java
@@ -419,6 +419,11 @@
"package jsinterop.annotations;",
"public @interface JsFunction {",
"}");
+ public static final MockJavaResource JSOPTIONAL =
+ createMockJavaResource("jsinterop.annotations.JsOptional",
+ "package jsinterop.annotations;",
+ "public @interface JsOptional {",
+ "}");
public static final MockJavaResource JSOVERLAY =
createMockJavaResource("jsinterop.annotations.JsOverlay",
"package jsinterop.annotations;",
@@ -433,7 +438,7 @@
JAVASCRIPTOBJECT, LIST, LONG, MAP, NO_CLASS_DEF_FOUND_ERROR, NUMBER, OBJECT,
RUNTIME_EXCEPTION, SERIALIZABLE, SHORT, STRING, STRING_BUILDER, SUPPRESS_WARNINGS, SYSTEM,
THROWABLE, SPECIALIZE_METHOD, JSTYPE, JSCONSTRUCTOR, JSPACKAGE, JSPROPERTY, JSMETHOD,
- JSIGNORE, JSFUNCTION, JSOVERLAY};
+ JSIGNORE, JSFUNCTION, JSOVERLAY, JSOPTIONAL};
}
/**
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
index fbfbd0a..8615cae 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
@@ -99,14 +99,14 @@
* Adds a new final parameter to this method.
*/
public JParameter createFinalParameter(SourceInfo info, String name, JType type) {
- return createParameter(info, name, type, true, false, false);
+ return createParameter(info, name, type, true, false, false, false);
}
/**
* Adds a new parameter to this method.
*/
public JParameter createParameter(SourceInfo info, String name, JType type) {
- return createParameter(info, name, type, false, false, false);
+ return createParameter(info, name, type, false, false, false, false);
}
/**
@@ -114,7 +114,7 @@
*/
public JParameter createParameter(SourceInfo info, String name, JType type, boolean isFinal,
boolean isVarargs) {
- return createParameter(info, name, type, isFinal, isVarargs, false);
+ return createParameter(info, name, type, isFinal, isVarargs, false, false);
}
/**
@@ -122,22 +122,22 @@
*/
public JParameter cloneParameter(JParameter from) {
return createParameter(from.getSourceInfo(), from.getName(), from.getType(), from.isFinal(),
- from.isVarargs(), from.isThis());
+ from.isVarargs(), from.isThis(), from.isOptional());
}
/**
* Creates a parameter to hold the value of this in devirtualized methods.
*/
public JParameter createThisParameter(SourceInfo info, JType type) {
- return createParameter(info, "this$static", type, true, false, true);
+ return createParameter(info, "this$static", type, true, false, true, false);
}
private JParameter createParameter(SourceInfo info, String name, JType type,
- boolean isFinal, boolean isVarargs, boolean isThis) {
+ boolean isFinal, boolean isVarargs, boolean isThis, boolean isOptional) {
assert (name != null);
assert (type != null);
- JParameter parameter = new JParameter(info, name, type, isFinal, isVarargs, isThis);
+ JParameter parameter = new JParameter(info, name, type, isFinal, isVarargs, isThis, isOptional);
addParameter(parameter);
return parameter;
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JParameter.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JParameter.java
index 4fa7e00..ca9e7c1 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JParameter.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JParameter.java
@@ -24,22 +24,32 @@
private final boolean isThis;
private final boolean isVarags;
+ private boolean isOptional;
public JParameter(SourceInfo info, String name, JType type, boolean isFinal) {
- this(info, name, type, isFinal, false, false);
+ this(info, name, type, isFinal, false, false, false);
}
JParameter(SourceInfo info, String name, JType type, boolean isFinal, boolean isVarargs,
- boolean isThis) {
+ boolean isThis, boolean isOptional) {
super(info, name, type, isFinal);
this.isThis = isThis;
this.isVarags = isVarargs;
+ this.isOptional = isOptional;
assert !isVarargs || type.isArrayType();
}
public JParameterRef createRef(SourceInfo info) {
return new JParameterRef(info, this);
}
+
+ /**
+ * Returns <code>true</code> if this parameter marked as optional.
+ */
+ public boolean isOptional() {
+ return isOptional;
+ }
+
/**
* Returns <code>true</code> if this parameter is the this parameter of a
* static impl method.
@@ -55,6 +65,10 @@
return isVarags;
}
+ public void setOptional() {
+ isOptional = true;
+ }
+
@Override
public JParameterRef makeRef(SourceInfo info) {
return new JParameterRef(info, this);
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java
index dd5e9ec..6debb78 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java
@@ -4081,6 +4081,11 @@
processSuppressedWarnings(method, x.annotations);
}
+ private void processAnnotations(JParameter parameter, Annotation... annotations) {
+ JsInteropUtil.maybeSetJsInteropProperties(parameter, annotations);
+ processSuppressedWarnings(parameter, annotations);
+ }
+
private void processSuppressedWarnings(CanHaveSuppressedWarnings x, Annotation... annotations) {
x.setSuppressedWarnings(JdtUtil.getSuppressedWarnings(annotations));
}
@@ -4137,9 +4142,9 @@
private void createParameter(SourceInfo info, LocalVariableBinding binding, String name,
JMethod method, boolean isVarargs, Annotation... annotations) {
- JParameter param =
+ JParameter parameter =
method.createParameter(info, name, typeMap.get(binding.type), binding.isFinal(), isVarargs);
- processSuppressedWarnings(param, annotations);
+ processAnnotations(parameter, annotations);
}
private void createParameters(JMethod method, AbstractMethodDeclaration x) {
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 583431e..0a0139f 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
@@ -243,6 +243,10 @@
checkIllegalOverrides(member);
}
+ if (member instanceof JMethod) {
+ checkMethodParameters((JMethod) member);
+ }
+
if (member.isJsOverlay()) {
checkJsOverlay(member);
return;
@@ -260,10 +264,6 @@
return;
}
- if (member.isJsMethodVarargs()) {
- checkJsVarargs(member);
- }
-
checkMemberQualifiedJsName(member);
if (isCheckedLocalName(member)) {
@@ -307,33 +307,38 @@
return;
}
- String methodDescription = JjsUtils.getReadableDescription(member);
+ String memberDescription = JjsUtils.getReadableDescription(member);
if (!member.getEnclosingType().isJsNative()) {
- logError(member, "JsOverlay '%s' can only be declared in a native type.", methodDescription);
+ logError(member, "JsOverlay '%s' can only be declared in a native type.", memberDescription);
+ }
+
+ if (member instanceof JConstructor) {
+ logError(member, "JsOverlay method '%s' cannot be a constructor.", memberDescription);
+ return;
+ }
+
+ if (member.getJsMemberType() != JsMemberType.NONE) {
+ logError(member, "JsOverlay method '%s' cannot be nor override a JsProperty or a JsMethod.",
+ memberDescription);
+ return;
}
if (member instanceof JField) {
JField field = (JField) member;
if (field.needsDynamicDispatch()) {
- logError(
- member, "JsOverlay field '%s' can only be static.",
- methodDescription);
+ logError(member, "JsOverlay field '%s' can only be static.", memberDescription);
}
return;
}
JMethod method = (JMethod) member;
- if (!method.getOverriddenMethods().isEmpty()) {
- logError(member,
- "JsOverlay method '%s' cannot override a supertype method.", methodDescription);
- return;
- }
+
+ assert method.getOverriddenMethods().isEmpty();
if (method.getBody() == null || (!method.isFinal() && !method.isStatic()
&& !method.isDefaultMethod())) {
- logError(member,
- "JsOverlay method '%s' cannot be non-final nor native.", methodDescription);
+ logError(member, "JsOverlay method '%s' cannot be non-final nor native.", memberDescription);
}
}
@@ -380,8 +385,34 @@
}
}
- private void checkJsVarargs(JMember member) {
- final JMethod method = (JMethod) member;
+ private void checkMethodParameters(JMethod method) {
+ boolean hasOptionalParameters = false;
+ for (JParameter parameter : method.getParams()) {
+ if (parameter.isOptional()) {
+ hasOptionalParameters = true;
+ continue;
+ }
+ if (hasOptionalParameters && !parameter.isVarargs()) {
+ logError(method, "JsOptional parameter '%s' in method %s cannot precede parameters "
+ + "that are not optional.", parameter.getName(), getMemberDescription(method));
+ break;
+ }
+ }
+
+ if (hasOptionalParameters
+ && method.getJsMemberType() != JsMemberType.CONSTRUCTOR
+ && method.getJsMemberType() != JsMemberType.METHOD
+ && !method.isOrOverridesJsFunctionMethod()) {
+ logError(method, "Method %s has JsOptional parameters and is not a JsMethod, "
+ + "a JsConstructor or a JsFunction method.", getMemberDescription(method));
+ }
+
+ if (method.isJsMethodVarargs()) {
+ checkJsVarargs(method);
+ }
+ }
+
+ private void checkJsVarargs(final JMethod method) {
if (!method.isJsniMethod()) {
return;
}
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 eb92871..3a35948 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
@@ -1496,7 +1496,84 @@
+ " method 'void EntryPoint.Interface.someOtherMethod()'.");
}
- public void testJsOverlayOnNativeJsTypeInterfaceSucceds() throws Exception {
+ public void testJsOptionalSucceeds() throws Exception {
+ addSnippetImport("jsinterop.annotations.JsConstructor");
+ addSnippetImport("jsinterop.annotations.JsMethod");
+ addSnippetImport("jsinterop.annotations.JsOptional");
+ addSnippetClassDecl(
+ "public static class Buggy {",
+ " @JsConstructor public Buggy(@JsOptional Object a) {}",
+ " @JsMethod public void fun(int a, Object b, @JsOptional String c) {}",
+ " @JsMethod public void bar(int a, @JsOptional Object b, @JsOptional String c) {}",
+ " @JsMethod public void baz(@JsOptional int a, @JsOptional Object b) {}",
+ "}");
+
+ assertBuggySucceeds();
+ }
+
+ public void testJsOptionalWithVarargsSucceeds() throws Exception {
+ addSnippetImport("jsinterop.annotations.JsMethod");
+ addSnippetImport("jsinterop.annotations.JsOptional");
+ addSnippetClassDecl(
+ "public class Buggy {",
+ " @JsMethod public void fun(@JsOptional String c, Object... os) {}",
+ " @JsMethod public void bar(int a, @JsOptional Object b, @JsOptional String... c) {}",
+ "}");
+
+ assertBuggySucceeds();
+ }
+
+ public void testJsOptionalNotAtEndFails() throws Exception {
+ addSnippetImport("jsinterop.annotations.JsConstructor");
+ addSnippetImport("jsinterop.annotations.JsMethod");
+ addSnippetImport("jsinterop.annotations.JsOptional");
+ addSnippetClassDecl(
+ "public static class Buggy {",
+ " @JsConstructor public Buggy(@JsOptional int a, Object b, @JsOptional String c) {}",
+ " @JsMethod public void bar(int a, @JsOptional Object b, String c) {}",
+ " @JsMethod public void baz(@JsOptional Object b, String c, Object... os) {}",
+ "}");
+
+ assertBuggyFails(
+ "Line 7: JsOptional parameter 'b' in method 'EntryPoint.Buggy.EntryPoint$Buggy(int, "
+ + "Object, String)' cannot precede parameters that are not optional.",
+ "Line 8: JsOptional parameter 'c' in method 'void EntryPoint.Buggy.bar(int, Object,"
+ + " String)' cannot precede parameters that are not optional.",
+ "Line 9: JsOptional parameter 'c' in method 'void EntryPoint.Buggy.baz(Object, String, "
+ + "Object[])' cannot precede parameters that are not optional.");
+ }
+
+ public void testJsOptionalOnNonJsExposedMethodsFails() throws Exception {
+ addSnippetImport("jsinterop.annotations.JsProperty");
+ addSnippetImport("jsinterop.annotations.JsOptional");
+ addSnippetClassDecl(
+ "public static class Buggy {",
+ " public void fun(int a, @JsOptional Object b, @JsOptional String c) {}",
+ " @JsProperty public void setBar(@JsOptional Object o) {}",
+ "}");
+
+ assertBuggyFails(
+ "Line 6: Method 'void EntryPoint.Buggy.fun(int, Object, String)' has JsOptional parameters "
+ + "and is not a JsMethod, a JsConstructor or a JsFunction method.",
+ "Line 7: Method 'void EntryPoint.Buggy.setBar(Object)' has JsOptional parameters and is "
+ + "not a JsMethod, a JsConstructor or a JsFunction method.");
+ }
+
+ public void testJsOptionalOnJsOverlayMethodsFails() throws Exception {
+ addSnippetImport("jsinterop.annotations.JsType");
+ addSnippetImport("jsinterop.annotations.JsOptional");
+ addSnippetImport("jsinterop.annotations.JsOverlay");
+ addSnippetClassDecl(
+ "@JsType(isNative = true) public static class Buggy {",
+ " @JsOverlay public final void fun(@JsOptional Object a) {}",
+ "}");
+
+ assertBuggyFails(
+ "Line 7: Method 'void EntryPoint.Buggy.fun(Object)' has JsOptional parameters and "
+ + "is not a JsMethod, a JsConstructor or a JsFunction method.");
+ }
+
+ public void testJsOverlayOnNativeJsTypeInterfaceSucceeds() throws Exception {
addSnippetImport("jsinterop.annotations.JsType");
addSnippetImport("jsinterop.annotations.JsOverlay");
addSnippetClassDecl(
@@ -1538,8 +1615,8 @@
" @JsOverlay public void m() { }",
"}");
- assertBuggyFails("Line 9: JsOverlay method 'void EntryPoint.Buggy.m()' cannot override a "
- + "supertype method.");
+ assertBuggyFails("Line 9: JsOverlay method 'void EntryPoint.Buggy.m()' cannot be nor override"
+ + " a JsProperty or a JsMethod.");
}
public void testJsOverlayOverridingSuperclassMethodFails() {
@@ -1553,8 +1630,8 @@
" @JsOverlay public void m() { }",
"}");
- assertBuggyFails(
- "Line 9: JsOverlay method 'void EntryPoint.Buggy.m()' cannot override a supertype method.");
+ assertBuggyFails("Line 9: JsOverlay method 'void EntryPoint.Buggy.m()' cannot be nor override"
+ + " a JsProperty or a JsMethod.");
}
public void testJsOverlayOnNonFinalMethodAndInstanceFieldFails() {
@@ -1614,6 +1691,33 @@
assertBuggySucceeds();
}
+ public void testJsOverlayOnJsMemberFails() {
+ addSnippetImport("jsinterop.annotations.JsType");
+ addSnippetImport("jsinterop.annotations.JsOverlay");
+ addSnippetImport("jsinterop.annotations.JsMethod");
+ addSnippetImport("jsinterop.annotations.JsProperty");
+ addSnippetImport("jsinterop.annotations.JsConstructor");
+ addSnippetClassDecl(
+ "@JsType(isNative=true) public static class Buggy {",
+ " @JsOverlay public Buggy() { }",
+ " @JsOverlay @JsConstructor protected Buggy(int i) { }",
+ " @JsMethod @JsOverlay public final void m() { }",
+ " @JsMethod @JsOverlay public static void n() { }",
+ " @JsProperty @JsOverlay public static void setA(String value) { }",
+ "}");
+
+ assertBuggyFails(
+ "Line 9: JsOverlay method 'EntryPoint.Buggy.EntryPoint$Buggy()' cannot be a constructor.",
+ "Line 10: JsOverlay method 'EntryPoint.Buggy.EntryPoint$Buggy(int)' cannot be a "
+ + "constructor.",
+ "Line 11: JsOverlay method 'void EntryPoint.Buggy.m()' cannot be nor override"
+ + " a JsProperty or a JsMethod.",
+ "Line 12: JsOverlay method 'void EntryPoint.Buggy.n()' cannot be nor override"
+ + " a JsProperty or a JsMethod.",
+ "Line 13: JsOverlay method 'void EntryPoint.Buggy.setA(String)' cannot be nor override"
+ + " a JsProperty or a JsMethod.");
+ }
+
public void testImplicitJsOverlayOnJsoMethodSucceeds() throws Exception {
addSnippetImport("com.google.gwt.core.client.JavaScriptObject");
addSnippetImport("jsinterop.annotations.JsOverlay");
