Fix getter/setter/field logic for abstract properties
This CL fixes a few issues in KspTypeElement:
* We would report fields for abstract properties, which we shouldn't.
* We would NOT report getters/setters for abstract properties, we should.
* We should NOT report getters/setters for JVMField annotated properties but
we were because KSP generates synthetic accessors for them.
Bug: 160322705
Test: KspTypeElementTest.fieldsInAbstractClass
Change-Id: I64e939544701e895f14d24a124c086e4b33139d6
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeElement.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeElement.kt
index f9605db..d733e31 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeElement.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeElement.kt
@@ -31,6 +31,7 @@
import com.google.devtools.ksp.getDeclaredFunctions
import com.google.devtools.ksp.getDeclaredProperties
import com.google.devtools.ksp.isOpen
+import com.google.devtools.ksp.isPrivate
import com.google.devtools.ksp.symbol.ClassKind
import com.google.devtools.ksp.symbol.KSClassDeclaration
import com.google.devtools.ksp.symbol.KSFunctionDeclaration
@@ -132,7 +133,7 @@
it.isStatic()
}
} else {
- _declaredProperties
+ _declaredProperties.filter { !it.isAbstract() }
}
val selectedNames = myPropertyFields.mapTo(mutableSetOf()) {
it.name
@@ -167,12 +168,17 @@
// until room generates kotlin code
return@mapNotNull null
}
+
val setter = it.declaration.setter
- val needsSetter = if (setter != null) {
- // kapt does not generate synthetics for private fields/setters so we won't either
- !setter.modifiers.contains(Modifier.PRIVATE)
- } else {
- isInterface() && it.declaration.isMutable
+ val needsSetter = when {
+ it.declaration.hasJvmFieldAnnotation() -> {
+ // jvm fields cannot have accessors but KSP generates synthetic accessors for
+ // them. We check for JVM field first before checking the setter
+ false
+ }
+ it.declaration.isPrivate() -> false
+ setter != null -> !setter.modifiers.contains(Modifier.PRIVATE)
+ else -> it.declaration.isMutable
}
if (needsSetter) {
KspSyntheticPropertyMethodElement.Setter(
@@ -190,12 +196,17 @@
return@mapNotNull null
}
val getter = it.declaration.getter
- val needsGetter = if (getter != null) {
- // kapt does not generate synthetics for private fields/getters so we won't either]
- !getter.modifiers.contains(Modifier.PRIVATE)
- } else {
- isInterface()
+ val needsGetter = when {
+ it.declaration.hasJvmFieldAnnotation() -> {
+ // jvm fields cannot have accessors but KSP generates synthetic accessors for
+ // them. We check for JVM field first before checking the getter
+ false
+ }
+ it.declaration.isPrivate() -> false
+ getter != null -> !getter.modifiers.contains(Modifier.PRIVATE)
+ else -> true
}
+
if (needsGetter) {
KspSyntheticPropertyMethodElement.Getter(
env = env,
diff --git a/room/compiler-processing/src/test/java/androidx/room/compiler/processing/ksp/KspTypeElementTest.kt b/room/compiler-processing/src/test/java/androidx/room/compiler/processing/ksp/KspTypeElementTest.kt
index 869c1b1..6780ed9 100644
--- a/room/compiler-processing/src/test/java/androidx/room/compiler/processing/ksp/KspTypeElementTest.kt
+++ b/room/compiler-processing/src/test/java/androidx/room/compiler/processing/ksp/KspTypeElementTest.kt
@@ -329,6 +329,48 @@
}
@Test
+ fun fieldsInAbstractClass() {
+ val src = Source.kotlin(
+ "Foo.kt",
+ """
+ abstract class MyAbstractClass {
+ @JvmField
+ var jvmVar: Int = 0
+ abstract var abstractVar: Int
+ var nonAbstractVar: Int = 0
+ }
+ """.trimIndent()
+ )
+ runProcessorTest(sources = listOf(src)) { invocation ->
+ val element = invocation.processingEnv.requireTypeElement("MyAbstractClass")
+ assertThat(
+ element.getAllFieldNames()
+ ).containsExactly(
+ "nonAbstractVar", "jvmVar"
+ )
+ assertThat(
+ element.getDeclaredMethods().map { it.name }
+ ).containsExactly(
+ "getAbstractVar", "setAbstractVar",
+ "getNonAbstractVar", "setNonAbstractVar"
+ )
+ element.getMethod("getAbstractVar").let {
+ assertThat(it.isAbstract()).isTrue()
+ }
+ element.getMethod("setAbstractVar").let {
+ assertThat(it.isAbstract()).isTrue()
+ }
+
+ element.getMethod("getNonAbstractVar").let {
+ assertThat(it.isAbstract()).isFalse()
+ }
+ element.getMethod("setNonAbstractVar").let {
+ assertThat(it.isAbstract()).isFalse()
+ }
+ }
+ }
+
+ @Test
fun declaredAndInstanceMethods() {
val src = Source.kotlin(
"Foo.kt",
