[AGP DSL] Automatically support += in Groovy
Groovy `foo += bar` maps to something like setFoo(getFoo().plus(bar))
Bug: 140406102
Test: Expanded DslDecoratorUnitTest.
Change-Id: I68269f74b2542d874ef1d1a8371292626565df61
diff --git a/build-system/gradle-core/src/main/java/com/android/build/gradle/internal/dsl/decorator/DslDecorator.kt b/build-system/gradle-core/src/main/java/com/android/build/gradle/internal/dsl/decorator/DslDecorator.kt
index 17cbffc..1a31587 100644
--- a/build-system/gradle-core/src/main/java/com/android/build/gradle/internal/dsl/decorator/DslDecorator.kt
+++ b/build-system/gradle-core/src/main/java/com/android/build/gradle/internal/dsl/decorator/DslDecorator.kt
@@ -18,6 +18,7 @@
import com.android.build.gradle.internal.dsl.AgpDslLockedException
import com.android.build.gradle.internal.dsl.Lockable
+import com.android.utils.usLocaleCapitalize
import com.android.utils.usLocaleDecapitalize
import com.google.common.annotations.VisibleForTesting
import com.google.common.base.Stopwatch
@@ -148,9 +149,7 @@
for (field in abstractProperties) {
createFieldBackedGetters(classWriter, generatedClass, field)
- if (field.settersToGenerate.isNotEmpty()) {
- createFieldBackedSetters(classWriter, generatedClass, field, lockable)
- }
+ createFieldBackedSetters(classWriter, generatedClass, field, lockable)
}
if (lockable) {
@@ -332,14 +331,21 @@
property: ManagedProperty,
lockable: Boolean
) {
- for (setter in property.settersToGenerate) {
- createFieldBackedSetter(
- classWriter,
- generatedClass,
- property,
- lockable,
- setter
- )
+ when (property.supportedPropertyType) {
+ is SupportedPropertyType.Val -> {
+ createGroovyMutatingSetter(classWriter, generatedClass, property)
+ }
+ is SupportedPropertyType.Var -> {
+ for (setter in property.settersToGenerate) {
+ createFieldBackedSetter(
+ classWriter,
+ generatedClass,
+ property,
+ lockable,
+ setter
+ )
+ }
+ }
}
}
@@ -353,7 +359,6 @@
val type = property.supportedPropertyType
check(type.implementationType == setter.argumentTypes[0]) {
"Currently only setters that use the same type are supported."
- // TODO(b/140406102): Support setters for groovy +=
}
// Mark bridge methods as synthetic.
val access = if(type.type == setter.argumentTypes[0]) property.access else property.access.or(Opcodes.ACC_SYNTHETIC)
@@ -385,6 +390,52 @@
}
}
+ /**
+ * To allow groovy field += bar, we need to generate a set(...) method.
+ *
+ * Mutability is not controlled here, it depends on the underlying lockable collection type.
+ */
+ private fun createGroovyMutatingSetter(
+ classWriter: ClassWriter,
+ generatedClass: Type,
+ property: ManagedProperty
+ ) {
+ val extraSetter = Method(
+ "set${property.name.usLocaleCapitalize()}",
+ Type.VOID_TYPE,
+ arrayOf(property.supportedPropertyType.type)
+ )
+ GeneratorAdapter(Opcodes.ACC_PUBLIC.or(Opcodes.ACC_SYNTHETIC), extraSetter, null, null, classWriter).apply {
+ // val newList = ArrayList(argument) // Take a copy so e.g. field = field doesn't clear the field!
+ // __backingField.clear()
+ // __backingField.addAll(newList)
+ newInstance(ARRAY_LIST)
+ dup()
+ loadArg(0)
+ checkCast(COLLECTION)
+ invokeConstructor(
+ ARRAY_LIST,
+ ARRAY_LIST_COPY_CONSTRUCTOR
+ )
+ loadThis()
+ getField(generatedClass, property.backingFieldName,
+ property.supportedPropertyType.implementationType
+ )
+ dup()
+ invokeVirtual(
+ property.supportedPropertyType.implementationType,
+ CLEAR
+ )
+ swap()
+ invokeVirtual(
+ property.supportedPropertyType.implementationType,
+ ADD_ALL
+ )
+ returnValue()
+ endMethod()
+ }
+ }
+
private fun createFieldBackedGetters(
classWriter: ClassWriter,
generatedClass: Type,
@@ -425,6 +476,12 @@
private val LOCK_METHOD = Method("lock", Type.VOID_TYPE, arrayOf())
private val LOCKED_EXCEPTION = Type.getType(AgpDslLockedException::class.java)
+ private val COLLECTION = Type.getType(Collection::class.java)
+ private val ARRAY_LIST = Type.getType(ArrayList::class.java)
+ private val ARRAY_LIST_COPY_CONSTRUCTOR = Method("<init>", Type.VOID_TYPE, arrayOf(COLLECTION))
+ private val CLEAR = Method("clear", Type.VOID_TYPE, arrayOf())
+ private val ADD_ALL = Method("addAll", Type.BOOLEAN_TYPE, arrayOf(COLLECTION))
+
// Use reflection to avoid needing to compile against java11 APIs yet.
private val privateLookupInMethod by lazy(LazyThreadSafetyMode.PUBLICATION) {
MethodHandles::class.java.getDeclaredMethod(
diff --git a/build-system/gradle-core/src/test/java/com/android/build/gradle/internal/dsl/decorator/DslDecoratorUnitTest.kt b/build-system/gradle-core/src/test/java/com/android/build/gradle/internal/dsl/decorator/DslDecoratorUnitTest.kt
index 554dab6..97ca0a2 100644
--- a/build-system/gradle-core/src/test/java/com/android/build/gradle/internal/dsl/decorator/DslDecoratorUnitTest.kt
+++ b/build-system/gradle-core/src/test/java/com/android/build/gradle/internal/dsl/decorator/DslDecoratorUnitTest.kt
@@ -20,6 +20,7 @@
import com.android.build.gradle.internal.dsl.Lockable
import com.google.common.truth.Truth.assertThat
import com.google.common.truth.Truth.assertWithMessage
+import groovy.util.Eval
import org.gradle.api.provider.Property
import org.junit.Test
import java.lang.reflect.Modifier
@@ -317,4 +318,22 @@
.getDeclaredConstructor().newInstance()
assertThat(decorated.foo).isEqualTo("hello")
}
+
+ interface WithList {
+ val list: MutableList<String>
+ }
+
+ @Test
+ fun `check groovy setter generation`() {
+ val decorated = DslDecorator(listOf(SupportedPropertyType.Val.List))
+ .decorate(WithList::class)
+ val withList = decorated.getDeclaredConstructor().newInstance()
+ Eval.me("withList", withList, "withList.list += ['one', 'two']")
+ assertThat(withList.list).containsExactly("one", "two").inOrder()
+ Eval.me("withList", withList, "withList.list += 'three'")
+ assertThat(withList.list).containsExactly("one", "two", "three").inOrder()
+ // Check self-assignment preserves values
+ Eval.me("withList", withList, "withList.list = withList.list")
+ assertThat(withList.list).containsExactly("one", "two", "three").inOrder()
+ }
}
