Improve supoprt for IsWidget in ui.xml.
This will fixes two bugs for IsWidget implementations:
- If the IsWidget interface implemented an interface, all the methods
of the parent interface had to be redeclared. Otherwise, UiBinder would
fail on them.
- For actual Widget classes, the UiObjectParser does smart things for
addStyleNames. For IsWidget implmentations, a setAddStyleNames method
had to be added.
Change-Id: Ie72eb3604230078b594076a6df4c84111195a3ce
Review-Link: https://gwt-review.googlesource.com/#/c/11610/
diff --git a/user/src/com/google/gwt/uibinder/elementparsers/BeanParser.java b/user/src/com/google/gwt/uibinder/elementparsers/BeanParser.java
index f54e96d..c763491 100644
--- a/user/src/com/google/gwt/uibinder/elementparsers/BeanParser.java
+++ b/user/src/com/google/gwt/uibinder/elementparsers/BeanParser.java
@@ -41,6 +41,17 @@
*/
public class BeanParser implements ElementParser {
+ /**
+ * Mapping between parameters and special UIObject methods. The {@link UIObjectParser} has a few
+ * methods that extend the normal bean naming pattern. So, that implementations of
+ * {@link IsWidget} behave like UIObjects, they have to be translated.
+ */
+ private static final Map<String, String> ADD_PROPERTY_TO_SETTER_MAP =
+ new HashMap<String, String>() { {
+ put("addStyleNames", "addStyleName");
+ put("addStyleDependentNames", "addStyleDepndentName");
+ }};
+
private final UiBinderContext context;
public BeanParser(UiBinderContext context) {
@@ -51,7 +62,7 @@
* Generates code to initialize all bean attributes on the given element.
* Includes support for <ui:attribute /> children that will apply to
* setters
- *
+ *
* @throws UnableToCompleteException
*/
public void parse(XMLElement elem, String fieldName, JClassType type,
@@ -62,6 +73,7 @@
final Map<String, String> setterValues = new HashMap<String, String>();
final Map<String, String> localizedValues = fetchLocalizedAttributeValues(
elem, writer);
+ final Map<String, String[]> adderValues = new HashMap<>();
final Map<String, String> requiredValues = new HashMap<String, String>();
final Map<String, JType> unfilledRequiredParams = new HashMap<String, JType>();
@@ -71,9 +83,8 @@
/*
* Handle @UiFactory and @UiConstructor, but only if the user
- * hasn't provided an instance via @UiField(provided = true)
+ * hasn't provided an instance via @UiField(provided = true)
*/
-
JAbstractMethod creator = null;
OwnerField uiField = writer.getOwnerClass().getUiField(fieldName);
if ((uiField == null) || (!uiField.isProvided())) {
@@ -153,18 +164,33 @@
unfilledRequiredParams.remove(propertyName);
} else {
JMethod setter = ownerFieldClass.getSetter(propertyName);
- if (setter == null) {
+ if (setter != null) {
+ String n = attribute.getName();
+ String value = elem.consumeAttributeWithDefault(n, null, getParamTypes(setter));
+
+ if (value == null) {
+ writer.die(elem, "Unable to parse %s.", attribute);
+ }
+ setterValues.put(propertyName, value);
+ } else if (ADD_PROPERTY_TO_SETTER_MAP.containsKey(propertyName)) {
+ String addMethod = ADD_PROPERTY_TO_SETTER_MAP.get(propertyName);
+ JType stringType = writer.getOracle().findType(String.class.getName());
+ if (ownerFieldClass.getRawType().findMethod(addMethod, new JType[]{stringType}) != null) {
+ String n = attribute.getName();
+ String[] value = elem.consumeStringArrayAttribute(n);
+
+ if (value == null) {
+ writer.die(elem, "Unable to parse %s.", attribute);
+ }
+ adderValues.put(addMethod, value);
+ } else {
+ writer.die(elem, "Class %s has no appropriate %s() method",
+ elem.getLocalName(), addMethod);
+ }
+ } else {
writer.die(elem, "Class %s has no appropriate set%s() method",
elem.getLocalName(), initialCap(propertyName));
}
- String n = attribute.getName();
- String value = elem.consumeAttributeWithDefault(n, null,
- getParamTypes(setter));
-
- if (value == null) {
- writer.die(elem, "Unable to parse %s.", attribute);
- }
- setterValues.put(propertyName, value);
}
}
@@ -203,6 +229,13 @@
writer.addStatement("%s.set%s(%s);", fieldName, initialCap(propertyName),
value);
}
+
+ for (Map.Entry<String, String[]> entry : adderValues.entrySet()) {
+ String addMethodName = entry.getKey();
+ for (String s : entry.getValue()) {
+ writer.addStatement("%s.%s(%s);", fieldName, addMethodName, s);
+ }
+ }
}
/**
diff --git a/user/src/com/google/gwt/uibinder/rebind/model/OwnerFieldClass.java b/user/src/com/google/gwt/uibinder/rebind/model/OwnerFieldClass.java
index fe5d310..0020224 100644
--- a/user/src/com/google/gwt/uibinder/rebind/model/OwnerFieldClass.java
+++ b/user/src/com/google/gwt/uibinder/rebind/model/OwnerFieldClass.java
@@ -23,13 +23,14 @@
import com.google.gwt.core.ext.typeinfo.JPrimitiveType;
import com.google.gwt.core.ext.typeinfo.JType;
import com.google.gwt.dev.util.Pair;
+import com.google.gwt.thirdparty.guava.common.collect.LinkedHashMultimap;
+import com.google.gwt.thirdparty.guava.common.collect.Multimap;
import com.google.gwt.uibinder.client.UiChild;
import com.google.gwt.uibinder.client.UiConstructor;
import com.google.gwt.uibinder.rebind.MortalLogger;
import com.google.gwt.uibinder.rebind.UiBinderContext;
import java.beans.Introspector;
-import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
@@ -69,7 +70,7 @@
tmpTypeRank.put("java.lang.Short", 3);
TYPE_RANK = Collections.unmodifiableMap(tmpTypeRank);
}
-
+
/**
* Gets or creates the descriptor for the given field class.
*
@@ -160,24 +161,6 @@
}
/**
- * Adds a setter for a given property to the given map of setters.
- *
- * @param allSetters the map of setters (keyed by property name)
- * @param propertyName the property name to use
- * @param method the setter to use
- */
- private void addSetter(Map<String, Collection<JMethod>> allSetters,
- String propertyName, JMethod method) {
- Collection<JMethod> propertyMethods = allSetters.get(propertyName);
- if (propertyMethods == null) {
- propertyMethods = new ArrayList<JMethod>();
- allSetters.put(propertyName, propertyMethods);
- }
-
- propertyMethods.add(method);
- }
-
- /**
* Given a collection of setters for the same property, picks which one to
* use. Not having a proper setter is not an error unless of course the user
* tries to use it.
@@ -203,13 +186,8 @@
minRank = rank;
preferredMethod = method;
ambiguousSetters.remove(propertyName);
- } else if (rank == minRank &&
- !sameParameterTypes(preferredMethod, method)) {
- // sameParameterTypes test is necessary because a setter can be
- // overridden by a subclass and that is not considered ambiguous.
- if (!ambiguousSetters.contains(propertyName)) {
- ambiguousSetters.add(propertyName);
- }
+ } else if (rank == minRank && !ambiguousSetters.contains(propertyName)) {
+ ambiguousSetters.add(propertyName);
}
}
@@ -228,20 +206,9 @@
* @param fieldType the leaf type to look at
* @return a multimap of property name to the setter methods
*/
- private Map<String, Collection<JMethod>> findAllSetters(JClassType fieldType) {
- Map<String, Collection<JMethod>> allSetters;
-
- // First, get all setters from the parent class, recursively.
- JClassType superClass = fieldType.getSuperclass();
- if (superClass != null) {
- allSetters = findAllSetters(superClass);
- } else {
- // Stop recursion - deepest level creates return value
- allSetters = new HashMap<String, Collection<JMethod>>();
- }
-
- JMethod[] methods = fieldType.getMethods();
- for (JMethod method : methods) {
+ private Multimap<String, JMethod> findAllSetters(JClassType fieldType) {
+ Multimap<String, JMethod> allSetters = LinkedHashMultimap.create();
+ for (JMethod method : fieldType.getInheritableMethods()) {
if (!isSetterMethod(method)) {
continue;
}
@@ -251,13 +218,13 @@
// turn "PropertyName" into "propertyName"
String beanPropertyName = Introspector.decapitalize(propertyName);
- addSetter(allSetters, beanPropertyName, method);
+ allSetters.put(beanPropertyName, method);
// keep backwards compatibility (i.e. hTML instead of HTML for setHTML)
String legacyPropertyName = propertyName.substring(0, 1).toLowerCase(Locale.ROOT)
+ propertyName.substring(1);
if (!legacyPropertyName.equals(beanPropertyName)) {
- addSetter(allSetters, legacyPropertyName, method);
+ allSetters.put(legacyPropertyName, method);
}
}
@@ -272,7 +239,7 @@
*/
private void findSetters(JClassType fieldType) {
// Pass one - get all setter methods
- Map<String, Collection<JMethod>> allSetters = findAllSetters(fieldType);
+ Multimap<String, JMethod> allSetters = findAllSetters(fieldType);
// Pass two - disambiguate
ambiguousSetters = new HashSet<String>();
@@ -396,31 +363,4 @@
assert (rank >= 0);
return rank;
}
-
- /**
- * Checks whether two methods have the same parameter types.
- *
- * @param m1 the first method to compare
- * @param m2 the second method to compare
- * @return whether the methods have the same parameter types
- */
- private boolean sameParameterTypes(JMethod m1, JMethod m2) {
- JParameter[] p1 = m1.getParameters();
- JParameter[] p2 = m2.getParameters();
-
- if (p1.length != p2.length) {
- return false;
- }
-
- for (int i = 0; i < p1.length; i++) {
- JType type1 = p1[i].getType();
- JType type2 = p2[i].getType();
-
- if (!type1.equals(type2)) {
- return false;
- }
- }
-
- return true;
- }
}
diff --git a/user/test/com/google/gwt/uibinder/rebind/JClassTypeAdapter.java b/user/test/com/google/gwt/uibinder/rebind/JClassTypeAdapter.java
index 94489a7..c1f4126 100644
--- a/user/test/com/google/gwt/uibinder/rebind/JClassTypeAdapter.java
+++ b/user/test/com/google/gwt/uibinder/rebind/JClassTypeAdapter.java
@@ -15,6 +15,9 @@
*/
package com.google.gwt.uibinder.rebind;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.when;
+
import com.google.gwt.core.ext.typeinfo.HasAnnotations;
import com.google.gwt.core.ext.typeinfo.HasTypeParameters;
import com.google.gwt.core.ext.typeinfo.JAbstractMethod;
@@ -26,9 +29,6 @@
import com.google.gwt.core.ext.typeinfo.JPrimitiveType;
import com.google.gwt.core.ext.typeinfo.JType;
-import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.when;
-
import org.mockito.Mockito;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
@@ -43,8 +43,12 @@
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.Iterator;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
/**
* Creates stub adapters for GWT reflection using EasyMock.
@@ -195,6 +199,134 @@
return adaptJavaClass(superclass);
}
});
+ when(type.getImplementedInterfaces()).thenAnswer(new Answer<JClassType[]>() {
+ @Override
+ public JClassType[] answer(InvocationOnMock invocation) throws Throwable {
+ Class<?>[] interfaces = clazz.getInterfaces();
+ if ((interfaces == null) || (interfaces.length == 0)) {
+ return null;
+ }
+
+ JClassType[] adaptedInterfaces = new JClassType[interfaces.length];
+ for (int i = 0; i < interfaces.length; i++) {
+ adaptedInterfaces[i] = adaptJavaClass(interfaces[i]);
+ }
+ return adaptedInterfaces;
+ }
+ });
+ when(type.getFlattenedSupertypeHierarchy()).thenAnswer(new Answer<Set<JClassType>>() {
+ @Override
+ public Set<JClassType> answer(InvocationOnMock invocation) throws Throwable {
+ return flatten(clazz);
+ }
+
+ private Set<JClassType> flatten(Class<?> clazz) {
+ Set<JClassType> flattened = new LinkedHashSet<JClassType>();
+ flattened.add(adaptJavaClass(clazz));
+
+ for (Class<?> intf : clazz.getInterfaces()) {
+ flattened.addAll(flatten(intf));
+ }
+
+ Class<?> superClass = clazz.getSuperclass();
+ if (superClass != null) {
+ flattened.addAll(flatten(superClass));
+ }
+
+ return flattened;
+ }
+ });
+ when(type.getInheritableMethods()).thenAnswer(new Answer<JMethod[]>() {
+ @Override
+ public JMethod[] answer(InvocationOnMock invocation) throws Throwable {
+ Map<String, Method> methodsBySignature = new TreeMap<String, Method>();
+ getInheritableMethodsOnSuperinterfacesAndMaybeThisInterface(clazz, methodsBySignature);
+ if (!clazz.isInterface()) {
+ getInheritableMethodsOnSuperclassesAndThisClass(clazz, methodsBySignature);
+ }
+ int size = methodsBySignature.size();
+ if (size == 0) {
+ return new JMethod[0];
+ } else {
+ Iterator<Method> leafMethods = methodsBySignature.values().iterator();
+ JMethod[] jMethods = new JMethod[size];
+ for (int i = 0; i < size; i++) {
+ Method method = leafMethods.next();
+ jMethods[i] = adaptMethod(method, adaptJavaClass(method.getDeclaringClass()));
+ }
+ return jMethods;
+ }
+ }
+
+ protected void getInheritableMethodsOnSuperinterfacesAndMaybeThisInterface(
+ Class<?> clazz,
+ Map<String, Method> methodsBySignature) {
+
+ // Recurse first so that more derived methods will clobber less derived
+ // methods.
+ Class<?>[] superIntfs = clazz.getInterfaces();
+ for (Class<?> superIntf : superIntfs) {
+ getInheritableMethodsOnSuperinterfacesAndMaybeThisInterface(
+ superIntf,
+ methodsBySignature);
+ }
+
+ Method[] declaredMethods = clazz.getMethods();
+ for (Method method : declaredMethods) {
+ String sig = computeInternalSignature(method);
+ Method existing = methodsBySignature.get(sig);
+ if (existing != null) {
+ Class<?> existingType = existing.getDeclaringClass();
+ Class<?> thisType = method.getDeclaringClass();
+ if (thisType.isAssignableFrom(existingType)) {
+ // The existing method is in a more-derived type, so don't replace it.
+ continue;
+ }
+ }
+ methodsBySignature.put(sig, method);
+ }
+ }
+
+ protected void getInheritableMethodsOnSuperclassesAndThisClass(
+ Class<?> clazz,
+ Map<String, Method> methodsBySignature) {
+
+ // Recurse first so that more derived methods will clobber less derived
+ // methods.
+ Class<?> superClass = clazz.getSuperclass();
+ if (superClass != null) {
+ getInheritableMethodsOnSuperclassesAndThisClass(
+ superClass,
+ methodsBySignature);
+ }
+
+ Method[] declaredMethods = clazz.getMethods();
+ for (Method method : declaredMethods) {
+ // Ensure that this method is inheritable.
+ if (Modifier.isPrivate(method.getModifiers())
+ || Modifier.isStatic(method.getModifiers())) {
+ // We cannot inherit this method, so skip it.
+ continue;
+ }
+
+ // We can override this method, so record it.
+ String sig = computeInternalSignature(method);
+ methodsBySignature.put(sig, method);
+ }
+ }
+
+ private String computeInternalSignature(Method method) {
+ StringBuilder sb = new StringBuilder();
+ sb.setLength(0);
+ sb.append(method.getName());
+ Class<?>[] params = method.getParameterTypes();
+ for (Class<?> param : params) {
+ sb.append("/");
+ sb.append(param.getName());
+ }
+ return sb.toString();
+ }
+ });
// TODO(rdamazio): Mock out other methods as needed
// TODO(rdamazio): Figure out what to do with reflections that GWT allows
diff --git a/user/test/com/google/gwt/uibinder/rebind/model/OwnerFieldClassTest.java b/user/test/com/google/gwt/uibinder/rebind/model/OwnerFieldClassTest.java
index 8e47183..adf5ea8 100644
--- a/user/test/com/google/gwt/uibinder/rebind/model/OwnerFieldClassTest.java
+++ b/user/test/com/google/gwt/uibinder/rebind/model/OwnerFieldClassTest.java
@@ -430,6 +430,39 @@
}
/**
+ * Interface with a setter for testing.
+ */
+ @SuppressWarnings("unused")
+ // We know this method is unused.
+ private interface SetterTestInterface {
+ void setMeow(int x);
+ }
+
+ /**
+ * Class with overridden setters for testing.
+ */
+ @SuppressWarnings("unused")
+ // We know these methods are unused
+ private interface OverriddenSetterTestInterface extends SetterTestInterface {
+ void setMoo(int x);
+ }
+
+ public void testOwnerFieldClass_overriddenInterface() throws Exception {
+ JClassType setterType = gwtTypeAdapter.adaptJavaClass(OverriddenSetterTestInterface.class);
+ OwnerFieldClass settersClass = OwnerFieldClass.getFieldClass(setterType,
+ MortalLogger.NULL, uiBinderCtx);
+ assertEquals(setterType, settersClass.getRawType());
+
+ // setMeow is in the base interface.
+ JMethod meowSetter = settersClass.getSetter("meow");
+ assertMethod(meowSetter, "setMeow", JPrimitiveType.INT);
+
+ // setMeow is in the base interface.
+ JMethod mooSetter = settersClass.getSetter("moo");
+ assertMethod(mooSetter, "setMoo", JPrimitiveType.INT);
+ }
+
+ /**
* Class with a {@link UiChild}-annotated methods.
*/
@SuppressWarnings("unused")
diff --git a/user/test/com/google/gwt/uibinder/test/client/FooIsWidget.java b/user/test/com/google/gwt/uibinder/test/client/FooIsWidget.java
new file mode 100644
index 0000000..90b81ec
--- /dev/null
+++ b/user/test/com/google/gwt/uibinder/test/client/FooIsWidget.java
@@ -0,0 +1,26 @@
+/*
+ * 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.uibinder.test.client;
+
+import com.google.gwt.user.client.ui.HasVisibility;
+import com.google.gwt.user.client.ui.IsWidget;
+
+/**
+ * An implementation of IsWidget to show that methods are recognized properly.
+ */
+public interface FooIsWidget extends HasVisibility, IsWidget {
+ void addStyleName(String style);
+}
diff --git a/user/test/com/google/gwt/uibinder/test/client/FooIsWidgetImpl.java b/user/test/com/google/gwt/uibinder/test/client/FooIsWidgetImpl.java
new file mode 100644
index 0000000..a51cca3
--- /dev/null
+++ b/user/test/com/google/gwt/uibinder/test/client/FooIsWidgetImpl.java
@@ -0,0 +1,24 @@
+/*
+ * 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.uibinder.test.client;
+
+import com.google.gwt.user.client.ui.Label;
+
+/**
+ * Implementation of {@link FooIsWidget}. A simple {@link Label} provides enough functionality.
+ */
+public class FooIsWidgetImpl extends Label implements FooIsWidget {
+}
diff --git a/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java b/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java
index 00aca23..dc3cb59 100644
--- a/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java
+++ b/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java
@@ -76,7 +76,7 @@
* The fields are {@code final} and we test that they've correctly been
* modified by the template.
*
- * @see http://code.google.com/p/google-web-toolkit/issues/detail?id=7740
+ * @see "http://code.google.com/p/google-web-toolkit/issues/detail?id=7740"
*/
public void testProvidedWidgetWithCustomInitializer() {
// Custom parser: should use the provided header, as the one from the
@@ -681,6 +681,12 @@
assertTrue(url, url.endsWith(".svg"));
}
+ public void testIsWidget() {
+ FooIsWidget isWidget = widgetUi.fooIsWidget;
+ assertEquals("gwt-Label " + widgetUi.myStyle.menuBar(), isWidget.asWidget().getStyleName());
+ assertEquals(false, isWidget.isVisible());
+ }
+
/**
* Assert that the expect strings are found in body, and in the order given.
* WARNING: both body and expected are normalized to lower case, to get around
diff --git a/user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.java b/user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.java
index 9d9ac3e..6b4b60c 100644
--- a/user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.java
+++ b/user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.java
@@ -219,6 +219,7 @@
@UiField Element myElementWithTagName;
@UiField DataResource embeddedSvgData;
@UiField DataResource linkedSvgData;
+ @UiField(provided = true) FooIsWidget fooIsWidget = new FooIsWidgetImpl();
ValueChangeEvent<Double> doubleValueChangeEvent;
diff --git a/user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.ui.xml b/user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.ui.xml
index aed3634..05bd807 100644
--- a/user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.ui.xml
+++ b/user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.ui.xml
@@ -712,6 +712,7 @@
<element-with-tagname ui:field='myElementWithTagName'/>
<js-element-type ui:field='myJsElementType'/>
+ <demo:FooIsWidget ui:field='fooIsWidget' addStyleNames="{myStyle.menuBar}" visible="false" />
</gwt:HTMLPanel>
</gwt:Dock>
</gwt:DockPanel>
