summaryrefslogtreecommitdiff
path: root/compiler/src/main/java/android
diff options
context:
space:
mode:
authorGeorge Mount <mount@google.com>2017-11-27 16:53:39 -0800
committerGeorge Mount <mount@google.com>2017-11-30 13:07:42 -0800
commit0e5980008a7d2b0945ff7fe1e9409aa138adc949 (patch)
treee57c91451832f855ec54fd25c1df1dcc38ad74d9 /compiler/src/main/java/android
parent0835ba50d395456512215d72b6b4853c9710c191 (diff)
downloaddata-binding-0e5980008a7d2b0945ff7fe1e9409aa138adc949.tar.gz
Fix problem with wrong InverseBindingAdapter being chosen.
Bug: 65167377 The wrong inverse binding adapter was being chosen because priority was given improperly. This CL adjusts the way priority is given so that it is consistent between parameters accepting arguments and return values being set to values. Test: CollisionAdapterTest Change-Id: I6e5c5a77d664ddda29313e3570fc579af70a1fd4
Diffstat (limited to 'compiler/src/main/java/android')
-rw-r--r--compiler/src/main/java/android/databinding/tool/reflection/InjectedClass.java9
-rw-r--r--compiler/src/main/java/android/databinding/tool/reflection/ModelClass.java18
-rw-r--r--compiler/src/main/java/android/databinding/tool/reflection/annotation/AnnotationClass.java11
-rw-r--r--compiler/src/main/java/android/databinding/tool/store/SetterStore.java212
4 files changed, 192 insertions, 58 deletions
diff --git a/compiler/src/main/java/android/databinding/tool/reflection/InjectedClass.java b/compiler/src/main/java/android/databinding/tool/reflection/InjectedClass.java
index eb83fcd2..ca7f3e5a 100644
--- a/compiler/src/main/java/android/databinding/tool/reflection/InjectedClass.java
+++ b/compiler/src/main/java/android/databinding/tool/reflection/InjectedClass.java
@@ -16,8 +16,12 @@
package android.databinding.tool.reflection;
+import android.databinding.tool.ext.ExtKt;
import android.databinding.tool.util.StringUtils;
+import com.squareup.javapoet.ClassName;
+import com.squareup.javapoet.TypeName;
+
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
@@ -226,6 +230,11 @@ public class InjectedClass extends ModelClass {
}
@Override
+ public TypeName getTypeName() {
+ return ExtKt.toTypeName(mClassName);
+ }
+
+ @Override
public String toString() {
return "Injected Class: " + mClassName;
}
diff --git a/compiler/src/main/java/android/databinding/tool/reflection/ModelClass.java b/compiler/src/main/java/android/databinding/tool/reflection/ModelClass.java
index e4351222..d4a022af 100644
--- a/compiler/src/main/java/android/databinding/tool/reflection/ModelClass.java
+++ b/compiler/src/main/java/android/databinding/tool/reflection/ModelClass.java
@@ -16,11 +16,14 @@
package android.databinding.tool.reflection;
import android.databinding.Bindable;
+import android.databinding.tool.ext.ExtKt;
import android.databinding.tool.reflection.Callable.Type;
import android.databinding.tool.util.L;
import android.databinding.tool.util.StringUtils;
import com.google.common.collect.ImmutableMap;
+import com.squareup.javapoet.ClassName;
+import com.squareup.javapoet.TypeName;
import org.jetbrains.annotations.NotNull;
@@ -679,4 +682,19 @@ public abstract class ModelClass {
}
return fieldName;
}
+
+ public TypeName getTypeName() {
+ // implementation only so that PSI model doesn't break
+ return ExtKt.toTypeName(toJavaCode());
+ }
+
+ @Override
+ public boolean equals(Object that) {
+ if (that instanceof ModelClass) {
+ TypeName thisTypeName = getTypeName();
+ TypeName thatTypeName = ((ModelClass) that).getTypeName();
+ return thisTypeName.equals(thatTypeName);
+ }
+ return false;
+ }
}
diff --git a/compiler/src/main/java/android/databinding/tool/reflection/annotation/AnnotationClass.java b/compiler/src/main/java/android/databinding/tool/reflection/annotation/AnnotationClass.java
index a4f7eb9e..8b023f6a 100644
--- a/compiler/src/main/java/android/databinding/tool/reflection/annotation/AnnotationClass.java
+++ b/compiler/src/main/java/android/databinding/tool/reflection/annotation/AnnotationClass.java
@@ -22,6 +22,9 @@ import android.databinding.tool.reflection.ModelMethod;
import android.databinding.tool.reflection.TypeUtil;
import android.databinding.tool.util.L;
+import com.squareup.javapoet.ClassName;
+import com.squareup.javapoet.TypeName;
+
import java.util.ArrayList;
import java.util.List;
@@ -385,12 +388,8 @@ class AnnotationClass extends ModelClass {
}
@Override
- public boolean equals(Object obj) {
- if (obj instanceof AnnotationClass) {
- return getTypeUtils().isSameType(mTypeMirror, ((AnnotationClass) obj).mTypeMirror);
- } else {
- return false;
- }
+ public TypeName getTypeName() {
+ return ClassName.get(mTypeMirror);
}
@Override
diff --git a/compiler/src/main/java/android/databinding/tool/store/SetterStore.java b/compiler/src/main/java/android/databinding/tool/store/SetterStore.java
index 8c8fd4af..c73c424f 100644
--- a/compiler/src/main/java/android/databinding/tool/store/SetterStore.java
+++ b/compiler/src/main/java/android/databinding/tool/store/SetterStore.java
@@ -26,6 +26,9 @@ import android.databinding.tool.util.L;
import android.databinding.tool.util.Preconditions;
import android.databinding.tool.util.StringUtils;
+import com.android.annotations.NonNull;
+import com.android.annotations.Nullable;
+
import java.io.IOException;
import java.io.Serializable;
import java.util.ArrayList;
@@ -54,6 +57,7 @@ import javax.lang.model.type.TypeMirror;
import javax.lang.model.util.Types;
public class SetterStore {
+ private static final int ASSIGNABLE_CONVERSION = 1;
private static SetterStore sStore;
private final IntermediateV3 mStore;
@@ -718,10 +722,8 @@ public class SetterStore {
L.d("setter %s takes type %s, compared to %s",
adapters.get(key).method, adapterValueType.toJavaCode(),
valueType.toJavaCode());
- boolean isBetterView = bestViewType == null ||
- bestViewType.isAssignableFrom(adapterViewType);
- if (isBetterParameter(valueType, adapterValueType, bestValueType,
- isBetterView, imports)) {
+ if (isBetterParameter(valueType, adapterViewType, adapterValueType,
+ bestViewType, bestValueType, imports)) {
bestViewType = adapterViewType;
bestValueType = adapterValueType;
MethodDescription adapter = adapters.get(key);
@@ -774,11 +776,9 @@ public class SetterStore {
L.d("getter %s returns type %s, compared to %s",
adapters.get(key).method, adapterValueType.toJavaCode(),
valueType);
- boolean isBetterView = bestMethod.viewType == null ||
- bestMethod.viewType.isAssignableFrom(adapterViewType);
if (valueType == null ||
- isBetterParameter(adapterValueType, valueType,
- bestMethod.returnType, isBetterView, imports)) {
+ isBetterReturn(valueType, adapterViewType, adapterValueType,
+ bestMethod.viewType, bestMethod.returnType, imports)) {
bestMethod.viewType = adapterViewType;
bestMethod.returnType = adapterValueType;
InverseDescription inverseDescription = adapters.get(key);
@@ -861,8 +861,9 @@ public class SetterStore {
for (ModelMethod method : methods) {
ModelClass[] parameterTypes = method.getParameterTypes();
ModelClass param = parameterTypes[0];
- if (method.isVoid() &&
- isBetterParameter(argumentType, param, bestParameterType, true, imports)) {
+ ModelClass previousViewType = bestParameterType == null ? null : viewType;
+ if (method.isVoid() && isBetterParameter(argumentType, viewType, param,
+ previousViewType, bestParameterType, imports)) {
bestParameterType = param;
bestMethod = method;
}
@@ -890,16 +891,14 @@ public class SetterStore {
try {
ModelClass methodViewType = mClassAnalyzer.findClass(className, imports);
if (methodViewType.erasure().isAssignableFrom(viewType)) {
- boolean isBetterViewType = bestViewType == null ||
- bestViewType.isAssignableFrom(methodViewType);
final InverseDescription inverseDescription = inverseMethods.get(className);
final String name = inverseDescription.method.isEmpty() ?
trimAttributeNamespace(attribute) : inverseDescription.method;
ModelMethod method = methodViewType.findInstanceGetter(name);
ModelClass returnType = method.getReturnType(null); // no parameters
if (valueType == null || bestReturnType == null ||
- isBetterParameter(returnType, valueType, bestReturnType,
- isBetterViewType, imports)) {
+ isBetterReturn(valueType, methodViewType, returnType,
+ bestViewType, bestReturnType, imports)) {
bestDescription = inverseDescription;
bestReturnType = returnType;
bestViewType = methodViewType;
@@ -959,47 +958,156 @@ public class SetterStore {
return "set" + StringUtils.capitalize(trimAttributeNamespace(attribute));
}
- private boolean isBetterParameter(ModelClass argument, ModelClass parameter,
- ModelClass oldParameter, boolean isBetterViewTypeMatch, Map<String, String> imports) {
- // Right view type. Check the value
- if (!isBetterViewTypeMatch && oldParameter.equals(argument)) {
- return false;
- } else if (argument.equals(parameter)) {
- // Exact match
- return true;
- } else if (!isBetterViewTypeMatch &&
- ModelMethod.isBoxingConversion(oldParameter, argument)) {
- return false;
- } else if (ModelMethod.isBoxingConversion(parameter, argument)) {
- // Boxing/unboxing is second best
- return true;
- } else {
- int oldConversionLevel = ModelMethod.getImplicitConversionLevel(oldParameter);
- if (ModelMethod.isImplicitConversion(argument, parameter)) {
- // Better implicit conversion
- int conversionLevel = ModelMethod.getImplicitConversionLevel(parameter);
- return oldConversionLevel < 0 || conversionLevel < oldConversionLevel;
- } else if (oldConversionLevel >= 0) {
- return false;
- } else if (parameter.isAssignableFrom(argument)) {
- // Right type, see if it is better than the current best match.
- if (oldParameter == null) {
- return true;
- } else {
- return oldParameter.isAssignableFrom(parameter);
- }
+ /**
+ * Checks to see if one parameter is a better fit for a setter (e.g. BindingAdapter) than
+ * another for an argument. It is assumed that both view types match the targeted view.
+ * <p>
+ * Note that this has different priorities than
+ * {@link #isBetterReturn(ModelClass, ModelClass, ModelClass, ModelClass, ModelClass, Map)}
+ *
+ * @param argument The argument type being passed to the setter.
+ * @param newViewType The type of the view in the BindingAdapter or setter method.
+ * @param newParameter The parameter type of the value in the BindingAdapter or setter method.
+ * @param oldViewType The type of the view in the previous matching BindingAdapter or setter
+ * method.
+ * @param oldParameter The parameter type of the value in the previous matching BindingAdapter
+ * or setter method.
+ * @param imports Import in the binding layout file.
+ * @return {@code true} when the new BindingAdapter or setter method is a better fit than the
+ * previous one or {@code false} if {@code argument} and {@code newParameter} aren't a match
+ * or are a worse match.
+ */
+ private boolean isBetterParameter(@NonNull ModelClass argument,
+ @NonNull ModelClass newViewType, @NonNull ModelClass newParameter,
+ @Nullable ModelClass oldViewType, @Nullable ModelClass oldParameter,
+ @Nullable Map<String, String> imports) {
+ if (oldParameter == null) {
+ // just validate that it can be converted
+ return calculateConversionPriority(argument, newParameter, imports) >= 0;
+ }
+
+ int newConversion = calculateConversionPriority(argument, newParameter, imports);
+ if (newConversion < 0) {
+ return false; // Doesn't convert
+ }
+
+ boolean isSameViewType = oldViewType.equals(newViewType);
+ boolean isBetterViewType = oldViewType.isAssignableFrom(newViewType);
+
+ int oldConversion = calculateConversionPriority(argument, oldParameter, imports);
+ if (oldConversion == ASSIGNABLE_CONVERSION && newConversion == ASSIGNABLE_CONVERSION) {
+ if (isSameViewType) {
+ // more specific getter is better
+ return oldParameter.isAssignableFrom(newParameter);
} else {
- MethodDescription conversionMethod = getConversionMethod(argument, parameter,
- imports);
- if (conversionMethod != null) {
- return true;
- }
- if (getConversionMethod(argument, oldParameter, imports) != null) {
- return false;
- }
- return argument.isObject() && !parameter.isPrimitive();
+ return isBetterViewType;
+ }
+ }
+
+ if (isSameViewType) {
+ return newConversion <= oldConversion;
+ }
+
+ if (newConversion == oldConversion) {
+ return isBetterViewType;
+ }
+
+ return newConversion <= oldConversion;
+ }
+
+ /**
+ * Checks to see if one return type is a better fit for a getter (e.g. InverseBindingAdapter)
+ * than another for a method call. It is assumed that both view types match the targeted view.
+ * <p>
+ * Note that this has different priorities than
+ * {@link #isBetterParameter(ModelClass, ModelClass, ModelClass, ModelClass, ModelClass, Map)}
+ *
+ * @param expected The type that is expected from the getter.
+ * @param newViewType The type of the view in the InverseBindingAdapter or getter method.
+ * @param newReturnType The return type of the value in the InverseBindingAdapter or getter
+ * method.
+ * @param oldViewType The type of the view in the previous matching InverseBindingAdapter or
+ * getter method.
+ * @param oldReturnType The return type of the value in the previous matching
+ * InverseBindingAdapter or getter method.
+ * @param imports Import in the binding layout file.
+ * @return {@code true} when the new InverseBindingAdapter or getter method is a better fit
+ * than the previous one or {@code false} if {@code expected} and {@code newReturnType} aren't
+ * a match or are a worse match.
+ */
+ private boolean isBetterReturn(@NonNull ModelClass expected,
+ @NonNull ModelClass newViewType, @NonNull ModelClass newReturnType,
+ @Nullable ModelClass oldViewType, @Nullable ModelClass oldReturnType,
+ @Nullable Map<String, String> imports) {
+ if (oldReturnType == null) {
+ // just validate that it can be converted
+ return calculateConversionPriority(newReturnType, expected, imports) >= 0;
+ }
+
+ int newConversion = calculateConversionPriority(newReturnType, expected, imports);
+ if (newConversion < 0) {
+ return false; // Doesn't convert
+ }
+
+ boolean isSameViewType = oldViewType.equals(newViewType);
+ boolean isBetterViewType = oldViewType.isAssignableFrom(newViewType);
+
+ int oldConversion = calculateConversionPriority(oldReturnType, expected, imports);
+ if (oldConversion == ASSIGNABLE_CONVERSION && newConversion == ASSIGNABLE_CONVERSION) {
+ if (isSameViewType) {
+ // more generic getter is better (fairly arbitrary, but consistent)
+ return newReturnType.isAssignableFrom(oldReturnType);
+ } else {
+ return isBetterViewType;
}
}
+
+ if (isSameViewType) {
+ return newConversion <= oldConversion;
+ }
+
+ if (newConversion == oldConversion) {
+ return isBetterViewType;
+ }
+
+ return newConversion <= oldConversion;
+ }
+
+ /**
+ * When calling a method, the closer an argument is to the declared parameter, the
+ * more likely it is to be matched. This method returns -1 when a {@code from} does not
+ * match {@code to}, 0 for an exact match, ASSIGNABLE_CONVERSION when {@code from} is a
+ * superclass or interface of {@code to}, and higher values for progressively more inexact
+ * matches.
+ *
+ * @param from The class or interface to attempt to convert from
+ * @param to The class or interface to attempt to convert to
+ * @param imports The imports used in this binding file
+ * @return {@code -1} for no match or greater than or equal to {@code 0} for a possible match,
+ * where {@code 0} is an exact match and greater numbers are progressively worse matches.
+ */
+ private int calculateConversionPriority(@NonNull ModelClass from, @NonNull ModelClass to,
+ @Nullable Map<String, String> imports) {
+ if (to.equals(from)) {
+ return 0; // exact match
+ }
+ if (to.isAssignableFrom(from)) {
+ return ASSIGNABLE_CONVERSION;
+ }
+ if (ModelMethod.isBoxingConversion(from, to)) {
+ return 2;
+ }
+ if (ModelMethod.isImplicitConversion(from, to)) {
+ // this should be 3 - 9
+ return 3 + ModelMethod.getImplicitConversionLevel(to);
+ }
+ if (getConversionMethod(from, to, imports) != null) {
+ return 10;
+ }
+ if (from.isObject() && !to.isPrimitive()) {
+ return 11;
+ }
+ return -1;
}
private MethodDescription getConversionMethod(ModelClass from, ModelClass to,