Create BR id from Callable

This CL fixes a bug where if an expression maps into a method
with a different name, we would create the BR id from the
expression instead of the referenced method.

Bug: 24973950
Change-Id: Ia57c31d926a737c9fc84775780aeb5e617769d43

9 files changed, 248 insertions, 11 deletions

diff --git a/compiler/src/main/java/android/databinding/tool/expr/FieldAccessExpr.java b/compiler/src/main/java/android/databinding/tool/expr/FieldAccessExpr.java
index 45a2afb2..516dfe72 100644
--- a/compiler/src/main/java/android/databinding/tool/expr/FieldAccessExpr.java
+++ b/compiler/src/main/java/android/databinding/tool/expr/FieldAccessExpr.java
@@ -16,19 +16,24 @@
 
 package android.databinding.tool.expr;
 
+import android.databinding.tool.ext.ExtKt;
 import android.databinding.tool.processing.Scope;
 import android.databinding.tool.reflection.Callable;
 import android.databinding.tool.reflection.Callable.Type;
 import android.databinding.tool.reflection.ModelAnalyzer;
 import android.databinding.tool.reflection.ModelClass;
 import android.databinding.tool.reflection.ModelMethod;
+import android.databinding.tool.util.BrNameUtil;
 import android.databinding.tool.util.L;
+import android.databinding.tool.util.Preconditions;
 import android.databinding.tool.writer.KCode;
 
 import java.util.List;
 
 public class FieldAccessExpr extends Expr {
     String mName;
+    // notification name for the field. Important when we map this to a method w/ different name
+    String mBrName;
     Callable mGetter;
     final boolean mIsObservableField;
     boolean mIsListener;
@@ -186,6 +191,19 @@ public class FieldAccessExpr extends Expr {
         return mName;
     }
 
+    public String getBrName() {
+        if (mIsListener) {
+            return null;
+        }
+        try {
+            Scope.enter(this);
+            Preconditions.checkNotNull(mGetter, "cannot get br name before resolving the getter");
+            return mBrName;
+        } finally {
+            Scope.exit();
+        }
+    }
+
     @Override
     public void updateExpr(ModelAnalyzer modelAnalyzer) {
         try {
@@ -241,6 +259,9 @@ public class FieldAccessExpr extends Expr {
                 observableField.getParents().add(this);
                 mGetter = mGetter.resolvedType.findGetterOrField("get", false);
                 mName = "";
+                mBrName = ExtKt.br(mName);
+            } else if (hasBindableAnnotations()) {
+                mBrName = ExtKt.br(BrNameUtil.brKey(mGetter));
             }
         }
         return mGetter.resolvedType;
diff --git a/compiler/src/main/java/android/databinding/tool/expr/MethodCallExpr.java b/compiler/src/main/java/android/databinding/tool/expr/MethodCallExpr.java
index ce1968d0..9e8e8c16 100644
--- a/compiler/src/main/java/android/databinding/tool/expr/MethodCallExpr.java
+++ b/compiler/src/main/java/android/databinding/tool/expr/MethodCallExpr.java
@@ -114,7 +114,8 @@ public class MethodCallExpr extends Expr {
             if (method.isStatic()) {
                 flags |= STATIC;
             }
-            mGetter = new Callable(Type.METHOD, method.getName(), method.getReturnType(args), flags);
+            mGetter = new Callable(Type.METHOD, method.getName(), method.getReturnType(args),
+                    method.getParameterTypes().length, flags);
         }
         return mGetter.resolvedType;
     }
diff --git a/compiler/src/main/java/android/databinding/tool/reflection/Callable.java b/compiler/src/main/java/android/databinding/tool/reflection/Callable.java
index 4bd09060..bae7f7cf 100644
--- a/compiler/src/main/java/android/databinding/tool/reflection/Callable.java
+++ b/compiler/src/main/java/android/databinding/tool/reflection/Callable.java
@@ -21,7 +21,7 @@ import java.util.List;
 
 public class Callable {
 
-    public static enum Type {
+    public enum Type {
         METHOD,
         FIELD
     }
@@ -38,10 +38,13 @@ public class Callable {
 
     private final int mFlags;
 
-    public Callable(Type type, String name, ModelClass resolvedType, int flags) {
+    private final int mParameterCount;
+
+    public Callable(Type type, String name, ModelClass resolvedType, int parameterCount, int flags) {
         this.type = type;
         this.name = name;
         this.resolvedType = resolvedType;
+        mParameterCount = parameterCount;
         mFlags = flags;
     }
 
@@ -49,6 +52,10 @@ public class Callable {
         return resolvedType.toJavaCode();
     }
 
+    public int getParameterCount() {
+        return mParameterCount;
+    }
+
     public boolean isDynamic() {
         return (mFlags & DYNAMIC) != 0;
     }
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 aa6634be..fb8e4bf6 100644
--- a/compiler/src/main/java/android/databinding/tool/reflection/ModelClass.java
+++ b/compiler/src/main/java/android/databinding/tool/reflection/ModelClass.java
@@ -386,7 +386,7 @@ public abstract class ModelClass {
     public Callable findGetterOrField(String name, boolean staticOnly) {
         if ("length".equals(name) && isArray()) {
             return new Callable(Type.FIELD, name, ModelAnalyzer.getInstance().loadPrimitive("int"),
-                    0);
+                    0, 0);
         }
         String capitalized = StringUtils.capitalize(name);
         String[] methodNames = {
@@ -415,7 +415,8 @@ public abstract class ModelClass {
                         }
                     }
                     final Callable result = new Callable(Callable.Type.METHOD, methodName,
-                            method.getReturnType(null), flags);
+                            method.getReturnType(null), method.getParameterTypes().length,
+                            flags);
                     return result;
                 }
             }
@@ -447,7 +448,7 @@ public abstract class ModelClass {
         if (publicField.isStatic()) {
             flags |= STATIC;
         }
-        return new Callable(Callable.Type.FIELD, name, fieldType, flags);
+        return new Callable(Callable.Type.FIELD, name, fieldType, 0, flags);
     }
 
     private ModelField getField(String name, boolean allowPrivate, boolean isStatic) {
diff --git a/compiler/src/main/java/android/databinding/tool/util/BrNameUtil.java b/compiler/src/main/java/android/databinding/tool/util/BrNameUtil.java
new file mode 100644
index 00000000..f79f37cc
--- /dev/null
+++ b/compiler/src/main/java/android/databinding/tool/util/BrNameUtil.java
@@ -0,0 +1,97 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * 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 android.databinding.tool.util;
+
+import android.databinding.tool.reflection.Callable;
+
+/**
+ * Central place to convert method/field names to BR observable fields
+ */
+public class BrNameUtil {
+    private static String stripPrefixFromField(String name) {
+        if (name.length() >= 2) {
+            char firstChar = name.charAt(0);
+            char secondChar = name.charAt(1);
+            if (name.length() > 2 && firstChar == 'm' && secondChar == '_') {
+                char thirdChar = name.charAt(2);
+                if (Character.isJavaIdentifierStart(thirdChar)) {
+                    return "" + Character.toLowerCase(thirdChar) +
+                            name.subSequence(3, name.length());
+                }
+            } else if ((firstChar == 'm' && Character.isUpperCase(secondChar)) ||
+                    (firstChar == '_' && Character.isJavaIdentifierStart(secondChar))) {
+                return "" + Character.toLowerCase(secondChar) + name.subSequence(2, name.length());
+            }
+        }
+        return name;
+    }
+
+    public static String brKey(Callable callable) {
+        if (callable.type == Callable.Type.FIELD) {
+            return stripPrefixFromField(callable.name);
+        }
+        CharSequence propertyName;
+        final String name = callable.name;
+        if (isGetter(callable) || isSetter(callable)) {
+            propertyName = name.subSequence(3, name.length());
+        } else if (isBooleanGetter(callable)) {
+            propertyName = name.subSequence(2, name.length());
+        } else {
+            L.e("@Bindable associated with method must follow JavaBeans convention %s", callable);
+            return null;
+        }
+        char firstChar = propertyName.charAt(0);
+        return "" + Character.toLowerCase(firstChar) +
+                propertyName.subSequence(1, propertyName.length());
+    }
+
+    private static boolean isGetter(Callable callable) {
+        return prefixes(callable.name, "get") &&
+                Character.isJavaIdentifierStart(callable.name.charAt(3)) &&
+                callable.getParameterCount() == 0 &&
+                !callable.resolvedType.isVoid();
+    }
+
+    private static boolean isSetter(Callable callable) {
+        return prefixes(callable.name, "set") &&
+                Character.isJavaIdentifierStart(callable.name.charAt(3)) &&
+                callable.getParameterCount() == 1 &&
+                callable.resolvedType.isVoid();
+    }
+
+    private static boolean isBooleanGetter(Callable callable) {
+        return prefixes(callable.name, "is") &&
+                Character.isJavaIdentifierStart(callable.name.charAt(2)) &&
+                callable.getParameterCount() == 0 &&
+                callable.resolvedType.isBoolean();
+    }
+
+    private static boolean prefixes(CharSequence sequence, String prefix) {
+        boolean prefixes = false;
+        if (sequence.length() > prefix.length()) {
+            int count = prefix.length();
+            prefixes = true;
+            for (int i = 0; i < count; i++) {
+                if (sequence.charAt(i) != prefix.charAt(i)) {
+                    prefixes = false;
+                    break;
+                }
+            }
+        }
+        return prefixes;
+    }
+}
diff --git a/compiler/src/main/kotlin/android/databinding/tool/writer/LayoutBinderWriter.kt b/compiler/src/main/kotlin/android/databinding/tool/writer/LayoutBinderWriter.kt
index daee2add..b351a2f2 100644
--- a/compiler/src/main/kotlin/android/databinding/tool/writer/LayoutBinderWriter.kt
+++ b/compiler/src/main/kotlin/android/databinding/tool/writer/LayoutBinderWriter.kt
@@ -627,13 +627,17 @@ class LayoutBinderWriter(val layoutBinder : LayoutBinder) {
                 tab("switch (fieldId) {", {
                     val accessedFields: List<FieldAccessExpr> = it.getParents().filterIsInstance(javaClass<FieldAccessExpr>())
                     accessedFields.filter { it.hasBindableAnnotations() }
-                            .groupBy { it.getName() }
+                            .groupBy { it.brName }
                             .forEach {
-                                tab("case ${it.key.br()}:") {
-                                    val field = it.value.first()
+                                // If two expressions look different but resolve to the same method,
+                                // we are not yet able to merge them. This is why we merge their
+                                // flags below.
+                                tab("case ${it.key}:") {
                                     tab("synchronized(this) {") {
-                                        mDirtyFlags.mapOr(field.invalidateFlagSet) { suffix, index ->
-                                            tab("${mDirtyFlags.localValue(index)} |= ${field.invalidateFlagSet.localValue(index)};")
+                                        val flagSet = it.value.foldRight(FlagSet()) { l, r -> l.invalidateFlagSet.or(r) }
+
+                                        mDirtyFlags.mapOr(flagSet) { suffix, index ->
+                                            tab("${mDirtyFlags.localValue(index)} |= ${flagSet.localValue(index)};")
                                         }
                                     } tab("}")
                                     tab("return true;")
diff --git a/integration-tests/TestApp/app/src/androidTestApi7/java/android/databinding/testapp/NameMappingTest.java b/integration-tests/TestApp/app/src/androidTestApi7/java/android/databinding/testapp/NameMappingTest.java
new file mode 100644
index 00000000..ce505d52
--- /dev/null
+++ b/integration-tests/TestApp/app/src/androidTestApi7/java/android/databinding/testapp/NameMappingTest.java
@@ -0,0 +1,71 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * 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 android.databinding.testapp;
+
+import android.databinding.testapp.databinding.NameMappingTestBinding;
+import android.databinding.testapp.vo.BasicObject;
+import android.test.UiThreadTest;
+import android.databinding.testapp.BR;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+
+public class NameMappingTest extends BaseDataBinderTest<NameMappingTestBinding> {
+
+    public NameMappingTest() {
+        super(NameMappingTestBinding.class);
+    }
+
+    @UiThreadTest
+    public void testChanges() {
+        initBinder();
+        final AtomicBoolean f1 = new AtomicBoolean(false);
+        final AtomicBoolean f2 = new AtomicBoolean(false);
+        BasicObject object = new BasicObject() {
+            @Override
+            public boolean isThisNameDoesNotMatchAnythingElse1() {
+                return f1.get();
+            }
+
+            @Override
+            public boolean getThisNameDoesNotMatchAnythingElse2() {
+                return f2.get();
+            }
+        };
+        mBinder.setObj(object);
+        mBinder.executePendingBindings();
+        for (int i = 0; i < 5; i ++) {
+            boolean f1New = (i & 1) != 0;
+            boolean f2New = (i & 1 << 1) != 0;
+            if (f1New != f1.get()) {
+                f1.set(f1New);
+                object.notifyPropertyChanged(BR.thisNameDoesNotMatchAnythingElse1);
+            }
+            if (f2New != f2.get()) {
+                f1.set(f2New);
+                object.notifyPropertyChanged(BR.thisNameDoesNotMatchAnythingElse2);
+            }
+            mBinder.executePendingBindings();
+            assertEquals(f2.get(), mBinder.textView.isEnabled());
+            assertEquals(f2.get(), mBinder.textView2.isEnabled());
+            assertEquals(false, mBinder.textView3.isEnabled());
+
+            assertEquals(f1.get(), mBinder.textView.isFocusable());
+            assertEquals(f1.get(), mBinder.textView2.isFocusable());
+            assertEquals(false, mBinder.textView3.isFocusable());
+        }
+    }
+}
diff --git a/integration-tests/TestApp/app/src/main/java/android/databinding/testapp/vo/BasicObject.java b/integration-tests/TestApp/app/src/main/java/android/databinding/testapp/vo/BasicObject.java
index 4ec76c5a..5f332ef6 100644
--- a/integration-tests/TestApp/app/src/main/java/android/databinding/testapp/vo/BasicObject.java
+++ b/integration-tests/TestApp/app/src/main/java/android/databinding/testapp/vo/BasicObject.java
@@ -43,6 +43,17 @@ public class BasicObject extends BaseObservable {
         notifyPropertyChanged(BR.field1);
     }
 
+    @Bindable
+    public boolean isThisNameDoesNotMatchAnythingElse1() {
+        // see: https://code.google.com/p/android/issues/detail?id=190207
+        return false;
+    }
+
+    @Bindable
+    public boolean getThisNameDoesNotMatchAnythingElse2() {
+        return false;
+    }
+
     public String boolMethod(boolean value) {
         return value ? "true" : "false";
     }
diff --git a/integration-tests/TestApp/app/src/main/res/layout/name_mapping_test.xml b/integration-tests/TestApp/app/src/main/res/layout/name_mapping_test.xml
new file mode 100644
index 00000000..1ed651b0
--- /dev/null
+++ b/integration-tests/TestApp/app/src/main/res/layout/name_mapping_test.xml
@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="utf-8"?>
+<layout>
+    <data>
+        <variable name="obj" type="android.databinding.testapp.vo.BasicObject"/>
+    </data>
+    <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+                  android:orientation="vertical"
+                  android:layout_width="match_parent"
+                  android:layout_height="match_parent">
+        <TextView android:layout_width="wrap_content" android:layout_height="wrap_content"
+                android:id="@+id/textView"
+                  android:enabled="@{obj.getThisNameDoesNotMatchAnythingElse2}"
+                  android:focusable="@{obj.isThisNameDoesNotMatchAnythingElse1}"/>
+        <TextView android:layout_width="wrap_content" android:layout_height="wrap_content"
+                  android:id="@+id/textView2"
+                  android:enabled="@{obj.thisNameDoesNotMatchAnythingElse2}"
+                  android:focusable="@{obj.thisNameDoesNotMatchAnythingElse1}"/>
+        <TextView android:layout_width="wrap_content" android:layout_height="wrap_content"
+                  android:id="@+id/textView3"
+                  android:enabled="@{obj.getThisNameDoesNotMatchAnythingElse2()}"
+                  android:focusable="@{obj.isThisNameDoesNotMatchAnythingElse1()}"/>
+
+    </LinearLayout>
+</layout>
\ No newline at end of file