Fixes 3 bugs in JsProperty and adds tests.
Getters of the form "getFoo()" were being improperly allowed to be
declared to return void. Fixed.
Is functions of the form "isFoo()" were being improperly translated to
JS as reads and writes of the "oo" property instead of the "foo"
property, which lead to failures to round trip values between
"setFoo()" and "isFoo()". Fixed.
Has function translation to JS was broken since "bar.hasFoo()" became
"foo in bar" rather than "'foo' in bar". This slight syntax error
lead to undefined property errors at runtime for any property that did
not yet have a value.
Change-Id: I24cedfd4ff1beb085cd8343a5a8966e34d3c6b02
Review-Link: https://gwt-review.googlesource.com/#/c/11740/
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
index 97ecb32..17d2dd5 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
@@ -1408,32 +1408,24 @@
} else {
JsName polyName = polymorphicNames.get(method);
// potentially replace method call with property access
- JMethod target = x.getTarget();
- for (JMethod overrideMethod : target.getOverriddenMethods()) {
+ JMethod targetMethod = x.getTarget();
+ for (JMethod overrideMethod : targetMethod.getOverriddenMethods()) {
if (overrideMethod.isJsProperty()) {
isJsProperty = true;
break;
}
}
- if (isJsProperty || target.isJsProperty()) {
- String getter = isGetter(target);
- String setter = isSetter(target);
- String has = isHas(target);
-
- // if fluent
- JType type = target.getType();
-
- boolean isFluent = type instanceof JReferenceType
- && type != program.getTypeJavaLangObject() && typeOracle.canTriviallyCast(
- x.getTarget().getEnclosingType(), type.getUnderlyingType());
+ if (isJsProperty || targetMethod.isJsProperty()) {
JsExpression qualExpr = pop();
-
- if (getter != null) {
- result = dispatchAsGetter(x, unnecessaryQualifier, getter, qualExpr);
- } else if (setter != null) {
- result = dispatchAsSetter(x, jsInvocation, setter, isFluent, qualExpr);
- } else if (has != null) {
- result = dispatchAsHas(x, has, qualExpr);
+ String methodName = targetMethod.getName();
+ if (targetMethod.getParams().size() == 0) {
+ if (startsWithCamelCase(methodName, "has")) {
+ result = createHasDispatch(x, methodName, qualExpr);
+ } else {
+ result = createGetterDispatch(x, unnecessaryQualifier, methodName, qualExpr);
+ }
+ } else if (targetMethod.getParams().size() == 1) {
+ result = createSetterDispatch(x, jsInvocation, targetMethod, qualExpr);
} else {
throw new InternalCompilerException("JsProperty not a setter, getter, or has.");
}
@@ -1441,8 +1433,8 @@
// insert trampoline (_ = instance, trampoline(_, _.jsBridgeMethRef,
// _.javaMethRef)).bind(_)(args)
if (typeOracle.needsJsInteropBridgeMethod(method)) {
- maybeDispatchViaTrampolineToBridgeMethod(x, method, jsInvocation,
- unnecessaryQualifier, result, polyName);
+ maybeDispatchViaTrampolineToBridgeMethod(x, method, jsInvocation, unnecessaryQualifier,
+ result, polyName);
return;
} else {
@@ -1517,35 +1509,47 @@
return tmpName;
}
- private JsExpression dispatchAsHas(JMethodCall x, String has, JsExpression qualExpr) {
- JsExpression result;JsNameRef property = new JsNameRef(x.getSourceInfo(), has);
- result = new JsBinaryOperation(x.getSourceInfo(), JsBinaryOperator.INOP,
- property, qualExpr);
- return result;
+ private JsExpression createHasDispatch(JMethodCall x, String methodName,
+ JsExpression qualExpr) {
+ String propertyName = toCamelCase(methodName.substring(3));
+ JsStringLiteral propertyNameLiteral = new JsStringLiteral(x.getSourceInfo(), propertyName);
+ return new JsBinaryOperation(x.getSourceInfo(), JsBinaryOperator.INOP, propertyNameLiteral,
+ qualExpr);
}
- private JsExpression dispatchAsSetter(JMethodCall x, JsInvocation jsInvocation, String setter, boolean isFluent, JsExpression qualExpr) {
- JsExpression result;JsNameRef property = new JsNameRef(x.getSourceInfo(), setter);
+ private JsExpression createSetterDispatch(JMethodCall x, JsInvocation jsInvocation,
+ JMethod targetMethod, JsExpression qualExpr) {
+ String methodName = targetMethod.getName();
+ JType returnType = targetMethod.getType();
+ boolean fluent = returnType instanceof JReferenceType
+ && returnType != program.getTypeJavaLangObject() && typeOracle.canTriviallyCast(
+ x.getTarget().getEnclosingType(), returnType.getUnderlyingType());
+ String propertyName = startsWithCamelCase(methodName, "set") ? toCamelCase(
+ methodName.substring(3)) : methodName;
+
+ JsExpression result;
+ JsNameRef propertyReference = new JsNameRef(x.getSourceInfo(), propertyName);
// either qualExpr.prop or _.prop depending on fluent or not
- property.setQualifier(isFluent ? globalTemp.makeRef(x.getSourceInfo()) : qualExpr);
+ propertyReference.setQualifier(fluent ? globalTemp.makeRef(x.getSourceInfo()) : qualExpr);
// propExpr = arg
- result = createAssignment(property, jsInvocation.getArguments().get(0));
- if (isFluent) {
+ result = createAssignment(propertyReference, jsInvocation.getArguments().get(0));
+ if (fluent) {
// (_ = qualExpr, _.prop = arg, _)
result = createCommaExpression(
createAssignment(globalTemp.makeRef(x.getSourceInfo()), qualExpr),
- createCommaExpression(result,
- globalTemp.makeRef(x.getSourceInfo())));
+ createCommaExpression(result, globalTemp.makeRef(x.getSourceInfo())));
}
return result;
}
- private JsExpression dispatchAsGetter(JMethodCall x, JsExpression unnecessaryQualifier, String getter, JsExpression qualExpr) {
- JsExpression result;// replace with qualExpr.property
- JsNameRef property = new JsNameRef(x.getSourceInfo(), getter);
- property.setQualifier(qualExpr);
- result = createCommaExpression(unnecessaryQualifier, property);
- return result;
+ private JsExpression createGetterDispatch(JMethodCall x, JsExpression unnecessaryQualifier,
+ String methodName, JsExpression qualExpr) {
+ String propertyName = startsWithCamelCase(methodName, "is") ? toCamelCase(
+ methodName.substring(2)) : startsWithCamelCase(methodName, "get") ? toCamelCase(
+ methodName.substring(3)) : methodName;
+ JsNameRef propertyReference = new JsNameRef(x.getSourceInfo(), propertyName);
+ propertyReference.setQualifier(qualExpr);
+ return createCommaExpression(unnecessaryQualifier, propertyReference);
}
/**
@@ -1582,46 +1586,6 @@
return JsNullLiteral.INSTANCE;
}
- private String isGetter(JMethod method) {
- String name = method.getName();
- // zero arg non-void getX()
- if (name.length() > 3 && name.startsWith("get") && Character.isUpperCase(name.charAt(3)) &&
- method.getType() != JPrimitiveType.VOID && method.getParams().size() == 0) {
- String propName = Character.toLowerCase(name.charAt(3)) + name.substring(4);
- return propName;
- } else if (name.length() > 3 && name.startsWith("is")
- && Character.isUpperCase(name.charAt(2)) && method.getType() == JPrimitiveType.BOOLEAN
- && method.getParams().size() == 0) {
- String propName = Character.toLowerCase(name.charAt(3)) + name.substring(4);
- return propName;
- } else if (method.getParams().size() == 0 && method.getType() != JPrimitiveType.VOID) {
- return name;
- }
- return null;
- }
-
- private String isSetter(JMethod method) {
- String name = method.getName();
- if (name.length() > 3 && name.startsWith("set") && Character.isUpperCase(name.charAt(3))
- && method.getParams().size() == 1) {
- String propName = Character.toLowerCase(name.charAt(3)) + name.substring(4);
- return propName;
- } else if (method.getParams().size() == 1) {
- return name;
- }
- return null;
- }
-
- private String isHas(JMethod method) {
- String name = method.getName();
- if (name.length() > 3 && name.startsWith("has") && Character.isUpperCase(name.charAt(3))
- && method.getParams().size() == 0 && method.getType() == JPrimitiveType.BOOLEAN) {
- String propName = Character.toLowerCase(name.charAt(3)) + name.substring(4);
- return propName;
- }
- return null;
- }
-
@Override
public void endVisit(JMultiExpression x, Context ctx) {
List<JsExpression> exprs = popList(x.getNumberOfExpressions());
@@ -3638,6 +3602,20 @@
}
}
+ private static boolean startsWithCamelCase(String string, String prefix) {
+ return string.length() > prefix.length() && string.startsWith(prefix)
+ && Character.isUpperCase(string.charAt(prefix.length()));
+ }
+
+ private static String toCamelCase(String string) {
+ if (string.length() == 0) {
+ return string;
+ } else if (string.length() == 1) {
+ return String.valueOf(Character.toLowerCase(string.charAt(0)));
+ }
+ return Character.toLowerCase(string.charAt(0)) + string.substring(1);
+ }
+
private Pair<JavaToJavaScriptMap, Set<JsNode>> execImpl() {
new FixNameClashesVisitor().accept(program);
uninitializedValuePotentiallyObservable = optimize ?
diff --git a/user/src/com/google/gwt/core/client/js/JsProperty.java b/user/src/com/google/gwt/core/client/js/JsProperty.java
index fb8b69a..05dfc50 100644
--- a/user/src/com/google/gwt/core/client/js/JsProperty.java
+++ b/user/src/com/google/gwt/core/client/js/JsProperty.java
@@ -22,17 +22,22 @@
import java.lang.annotation.Target;
/**
- * JsProperty marks a method in a {@link JsType} as a property accessor and recognizes
- * JavaBean style naming convention. Instead of translating method calls to JsProperty methods
- * as method calls in JS, they will be replaced with dotted property lookups.
- *<p> Examples:
+ * JsProperty marks a method in a {@link JsType} as a property accessor and recognizes JavaBean
+ * style naming convention. Instead of translating method calls to JsProperty methods as method
+ * calls in JS, they will be replaced with dotted property lookups.
+ * <p>
+ * In case of JsType with JsProperties implemented by Java classes, the property access still
+ * triggers the execution of the matching getter or setter methods as they will be translated into
+ * custom property setter and getter in JavaScript.
+ * <p>
+ * Examples:
* <ul>
* <li> {@code @JsProperty getX()} translates as <tt>this.x</tt>
* <li> {@code @JsProperty x()} translates as <tt>this.x</tt>
* <li> {@code @JsProperty setX(int y)} translates as <tt>this.x=y</tt>
* <li> {@code @JsProperty x(int x)} translates as <tt>this.x=y</tt>
* <li> {@code @JsProperty hasX(int x)} translates as <tt>x in this</tt>
- *</ul>
+ * </ul>
* <p>
* In addition, fluent style <tt>return this</tt> syntax is supported for setters, so
* {@code @JsProperty T setX(int x)} translates as <tt>this.x=x, return this</tt>.
diff --git a/user/test/com/google/gwt/core/client/interop/JsPoint.java b/user/test/com/google/gwt/core/client/interop/JsPoint.java
new file mode 100644
index 0000000..d83892f
--- /dev/null
+++ b/user/test/com/google/gwt/core/client/interop/JsPoint.java
@@ -0,0 +1,62 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.core.client.interop;
+
+import com.google.gwt.core.client.js.JsProperty;
+import com.google.gwt.core.client.js.JsType;
+
+/**
+ * An interface to model a raw JS Point type.
+ */
+@JsType
+public interface JsPoint {
+
+ @JsProperty
+ boolean hasX();
+
+ @JsProperty
+ boolean isX();
+
+ @JsProperty
+ int getX();
+
+ @JsProperty
+ int x();
+
+ @JsProperty
+ void setX(int x);
+
+ @JsProperty
+ JsPoint x(int x);
+
+ @JsProperty
+ boolean hasY();
+
+ @JsProperty
+ boolean isY();
+
+ @JsProperty
+ int getY();
+
+ @JsProperty
+ int y();
+
+ @JsProperty
+ void setY(int y);
+
+ @JsProperty
+ JsPoint y(int y);
+}
diff --git a/user/test/com/google/gwt/core/client/interop/JsTypeTest.java b/user/test/com/google/gwt/core/client/interop/JsTypeTest.java
index 1c34d63..7d3a111 100644
--- a/user/test/com/google/gwt/core/client/interop/JsTypeTest.java
+++ b/user/test/com/google/gwt/core/client/interop/JsTypeTest.java
@@ -17,6 +17,7 @@
import static com.google.gwt.core.client.ScriptInjector.TOP_WINDOW;
+import com.google.gwt.core.client.JavaScriptObject;
import com.google.gwt.core.client.ScriptInjector;
import com.google.gwt.junit.client.GWTTestCase;
@@ -129,6 +130,46 @@
assertEquals(58, mc.sum(0));
}
+ public void testJsPropertyIsX() {
+ JsPoint point = (JsPoint) JavaScriptObject.createObject();
+
+ assertFalse(point.isX());
+ point.setX(10);
+ assertTrue(point.isX());
+ point.y(999).x(0);
+ assertFalse(point.isX());
+ }
+
+ public void testJsPropertyHasX() {
+ JsPoint point = (JsPoint) JavaScriptObject.createObject();
+
+ assertFalse(point.hasX());
+ point.setX(10);
+ assertTrue(point.hasX());
+ point.y(999).x(0);
+ assertTrue(point.hasX());
+ }
+
+ public void testJsPropertyGetX() {
+ JsPoint point = (JsPoint) JavaScriptObject.createObject();
+
+ assertTrue(isUndefined(point.getX()));
+ point.setX(10);
+ assertEquals(10, point.getX());
+ point.y(999).x(0);
+ assertEquals(0, point.getX());
+ }
+
+ public void testJsPropertyX() {
+ JsPoint point = (JsPoint) JavaScriptObject.createObject();
+
+ assertTrue(isUndefined(point.x()));
+ point.setX(10);
+ assertEquals(10, point.x());
+ point.y(999).x(0);
+ assertEquals(0, point.x());
+ }
+
public void testCasts() {
MyJsInterface myClass;
assertNotNull(myClass = (MyJsInterface) createMyJsInterface());
@@ -245,6 +286,10 @@
return new $wnd['testfoo.bar.MyJsInterface']();
}-*/;
+ private static native boolean isUndefined(int value) /*-{
+ return value === undefined;
+ }-*/;
+
private static native boolean hasField(Object object, String fieldName) /*-{
return object[fieldName] != undefined;
}-*/;
