summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYigit Boyar <yboyar@google.com>2019-12-09 13:13:12 -0800
committerYigit Boyar <yboyar@google.com>2019-12-11 01:33:16 +0000
commit7b4935443fcaa04edacad37c7e274fba7063629e (patch)
tree4dae7f5ef4c00253bc546287d69dbac896e4c5f4
parent0eb8d900edad41225c2fb6caa146b9d47661662f (diff)
downloaddata-binding-7b4935443fcaa04edacad37c7e274fba7063629e.tar.gz
Ban calling get / getValue on an observable
This is unnecessary and also causes an infinite loop in the compiler. It is flat out wrong so it is much better to just detect and print a proper error. Bug: 144604674 Test: ObservableGetDetectionTest Change-Id: I5ca8acaa0696b91aa9806f831e4661bb7c2442ac
-rw-r--r--compilationTests/src/test/java/androidx/databinding/compilationTest/BaseCompilationTest.java8
-rw-r--r--compilationTests/src/test/java/androidx/databinding/compilationTest/ObservableGetDetectionTest.kt74
-rw-r--r--compiler/src/main/java/android/databinding/tool/expr/Expr.java4
-rw-r--r--compilerCommon/src/main/java/android/databinding/tool/processing/ErrorMessages.java7
4 files changed, 93 insertions, 0 deletions
diff --git a/compilationTests/src/test/java/androidx/databinding/compilationTest/BaseCompilationTest.java b/compilationTests/src/test/java/androidx/databinding/compilationTest/BaseCompilationTest.java
index 984df41c..1895fd26 100644
--- a/compilationTests/src/test/java/androidx/databinding/compilationTest/BaseCompilationTest.java
+++ b/compilationTests/src/test/java/androidx/databinding/compilationTest/BaseCompilationTest.java
@@ -92,6 +92,14 @@ public class BaseCompilationTest {
copyResourceTo(name, new File(testFolder, path));
}
+ protected void writeFile(String path, String contents) throws IOException {
+ File targetFile = new File(testFolder, path);
+ FileUtils.forceMkdir(targetFile.getParentFile());
+ try(FileOutputStream fos = new FileOutputStream(targetFile)) {
+ IOUtils.write(contents, fos);
+ }
+ }
+
protected void copyResourceTo(String name, String path, Map<String, String> replacements)
throws IOException {
copyResourceTo(name, new File(testFolder, path), replacements);
diff --git a/compilationTests/src/test/java/androidx/databinding/compilationTest/ObservableGetDetectionTest.kt b/compilationTests/src/test/java/androidx/databinding/compilationTest/ObservableGetDetectionTest.kt
new file mode 100644
index 00000000..b3be8874
--- /dev/null
+++ b/compilationTests/src/test/java/androidx/databinding/compilationTest/ObservableGetDetectionTest.kt
@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2019 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 androidx.databinding.compilationTest
+
+import android.databinding.tool.processing.ErrorMessages
+import org.hamcrest.CoreMatchers.not
+import org.hamcrest.MatcherAssert.assertThat
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+
+@RunWith(Parameterized::class)
+class ObservableGetDetectionTest(
+ private val type: String,
+ private val getter: String
+) : BaseCompilationTest(true) {
+ @Test
+ fun detectGetterCallsOnObservables() {
+ prepareProject()
+ writeFile("/app/src/main/res/layout/observable_get.xml",
+ """
+ <layout xmlns:android="http://schemas.android.com/apk/res/android"
+ xmlns:bind="http://schemas.android.com/apk/res-auto">
+ <data>
+ <variable name="myVariable" type="$type"/>
+ </data>
+ <TextView
+ android:layout_width="wrap_content"
+ android:layout_height="wrap_content"
+ android:text="@{myVariable.$getter}"/>
+ </layout>
+ """.trimIndent())
+ val result = runGradle("assembleDebug")
+ assertThat(result.error, result.resultCode, not(0))
+ val expected = ErrorMessages.GETTER_ON_OBSERVABLE.format("myVariable.$getter")
+ assertTrue(result.error, result.bindingExceptions.any {
+ it.createHumanReadableMessage().contains(expected)
+ })
+ }
+
+ companion object {
+ @Parameterized.Parameters(name = "{0}")
+ @JvmStatic
+ fun params() = arrayOf(
+ "ObservableByte",
+ "ObservableBoolean",
+ "ObservableChar",
+ "ObservableShort",
+ "ObservableInt",
+ "ObservableLong",
+ "ObservableFloat",
+ "ObservableDouble",
+ "ObservableField&lt;String>",
+ "ObservableParcelable").map {
+ arrayOf("androidx.databinding.$it", "get()")
+ } + arrayOf(
+ arrayOf("androidx.lifecycle.LiveData&lt;String>", "getValue()")
+ )
+ }
+} \ No newline at end of file
diff --git a/compiler/src/main/java/android/databinding/tool/expr/Expr.java b/compiler/src/main/java/android/databinding/tool/expr/Expr.java
index f972af65..c221069c 100644
--- a/compiler/src/main/java/android/databinding/tool/expr/Expr.java
+++ b/compiler/src/main/java/android/databinding/tool/expr/Expr.java
@@ -936,6 +936,10 @@ abstract public class Expr implements VersionProvider, LocationScopeProvider {
expr = unwrapped;
}
+ if (unwrapped == this) {
+ L.e(ErrorMessages.GETTER_ON_OBSERVABLE, this.toString());
+ return;
+ }
if (unwrapped != null) {
child.getParents().remove(this);
unwrapped.getParents().add(this);
diff --git a/compilerCommon/src/main/java/android/databinding/tool/processing/ErrorMessages.java b/compilerCommon/src/main/java/android/databinding/tool/processing/ErrorMessages.java
index e9f8eab2..bba79571 100644
--- a/compilerCommon/src/main/java/android/databinding/tool/processing/ErrorMessages.java
+++ b/compilerCommon/src/main/java/android/databinding/tool/processing/ErrorMessages.java
@@ -102,4 +102,11 @@ public class ErrorMessages {
public static final String DUPLICATE_VIEW_OR_INCLUDE_ID =
"<%s id='%s'> conflicts with another tag that has the same ID";
+
+ public static final String GETTER_ON_OBSERVABLE =
+ "Calling `%s` is not allowed.\n" +
+ "\n" +
+ "Data binding will automatically listen to observable fields (e.g. a LiveData or an Observable subclass) " +
+ "and update the UI automatically. Please strip the method call from the expression, " +
+ "e.g. `foo.get()` -> `foo`.";
}