Fix modifiers of properties & accessors
This CL fixes a bug in KSP XProcessing implementation where
the modifiers for properties and its accessors were not being
reported the way javap sees them. More specifically, for a
public property, we would report it as public even though the
generated .class file will have it as private.
Also fixed how accessors calculate modifiers.
Bug: 160322705
Test: XExecutableElementTest, KspFieldElementTest
Change-Id: I586dfa1309bdb74b8ca010b4c060a856665fe089
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KSAnnotatedExt.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KSAnnotatedExt.kt
index 1c6e4e4..5e66046 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KSAnnotatedExt.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KSAnnotatedExt.kt
@@ -22,6 +22,10 @@
it.annotationType.resolve().declaration.qualifiedName?.asString() == "kotlin.jvm.JvmStatic"
}
+internal fun KSAnnotated.isJvmField() = annotations.any {
+ it.annotationType.resolve().declaration.qualifiedName?.asString() == "kotlin.jvm.JvmField"
+}
+
internal fun KSAnnotated.isJvmDefault() = annotations.any {
it.annotationType.resolve().declaration.qualifiedName?.asString() == "kotlin.jvm.JvmDefault"
}
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspExecutableElement.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspExecutableElement.kt
index 259f26d..c1570e1 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspExecutableElement.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspExecutableElement.kt
@@ -33,7 +33,7 @@
declaration = declaration
),
XExecutableElement,
- XHasModifiers by KspHasModifiers(declaration),
+ XHasModifiers by KspHasModifiers.create(declaration),
XAnnotated by KspAnnotated.create(
env = env,
delegate = declaration,
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspFieldElement.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspFieldElement.kt
index 5bdbddd..deb3153 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspFieldElement.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspFieldElement.kt
@@ -31,7 +31,7 @@
val containing: KspTypeElement
) : KspElement(env, declaration),
XFieldElement,
- XHasModifiers by KspHasModifiers(declaration),
+ XHasModifiers by KspHasModifiers.create(declaration),
XAnnotated by KspAnnotated.create(env, declaration, FIELD) {
override val equalityItems: Array<out Any?> by lazy {
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspHasModifiers.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspHasModifiers.kt
index a01effdf..34cf55c 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspHasModifiers.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspHasModifiers.kt
@@ -21,14 +21,18 @@
import com.google.devtools.ksp.isPrivate
import com.google.devtools.ksp.isProtected
import com.google.devtools.ksp.isPublic
+import com.google.devtools.ksp.symbol.KSClassDeclaration
import com.google.devtools.ksp.symbol.KSDeclaration
+import com.google.devtools.ksp.symbol.KSFunctionDeclaration
+import com.google.devtools.ksp.symbol.KSPropertyAccessor
+import com.google.devtools.ksp.symbol.KSPropertyDeclaration
import com.google.devtools.ksp.symbol.Modifier
/**
* Implementation of [XHasModifiers] for ksp declarations.
*/
-class KspHasModifiers(
- private val declaration: KSDeclaration
+sealed class KspHasModifiers(
+ protected val declaration: KSDeclaration
) : XHasModifiers {
override fun isPublic(): Boolean {
return declaration.isPublic()
@@ -57,4 +61,79 @@
override fun isFinal(): Boolean {
return !declaration.isOpen()
}
-}
\ No newline at end of file
+
+ private class Declaration(declaration: KSDeclaration) : KspHasModifiers(declaration)
+
+ private class PropertyField(
+ declaration: KSDeclaration
+ ) : KspHasModifiers(declaration) {
+ private val jvmField by lazy {
+ declaration.isJvmField()
+ }
+
+ override fun isPublic(): Boolean {
+ return jvmField && super.isPublic()
+ }
+
+ override fun isProtected(): Boolean {
+ return jvmField && super.isProtected()
+ }
+
+ override fun isPrivate(): Boolean {
+ return if (jvmField) {
+ super.isPrivate()
+ } else {
+ // it is always private unless it is a jvm field
+ true
+ }
+ }
+ }
+
+ /**
+ * Handles accessor visibility when there is an accessor declared in code.
+ * We cannot simply merge modifiers of the property and the accessor as the visibility rules
+ * of the declaration is more complicated than just looking at modifiers.
+ */
+ private class PropertyFieldAccessor(
+ private val accessor: KSPropertyAccessor
+ ) : KspHasModifiers(accessor.receiver) {
+ override fun isPublic(): Boolean {
+ return accessor.modifiers.contains(Modifier.PUBLIC) ||
+ (!isPrivate() && !isProtected() && super.isPublic())
+ }
+
+ override fun isProtected(): Boolean {
+ return accessor.modifiers.contains(Modifier.PROTECTED) ||
+ (!isPrivate() && super.isProtected())
+ }
+
+ override fun isPrivate(): Boolean {
+ return accessor.modifiers.contains(Modifier.PRIVATE) ||
+ super.isPrivate()
+ }
+ }
+
+ companion object {
+ fun createForSyntheticAccessor(
+ property: KSPropertyDeclaration,
+ accessor: KSPropertyAccessor?
+ ): XHasModifiers {
+ if (accessor != null) {
+ return PropertyFieldAccessor(accessor)
+ }
+ return Declaration(property)
+ }
+
+ fun create(owner: KSPropertyDeclaration): XHasModifiers {
+ return PropertyField(owner)
+ }
+
+ fun create(owner: KSFunctionDeclaration): XHasModifiers {
+ return Declaration(owner)
+ }
+
+ fun create(owner: KSClassDeclaration): XHasModifiers {
+ return Declaration(owner)
+ }
+ }
+}
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 8730aa1..91f4c17 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
@@ -43,7 +43,7 @@
override val declaration: KSClassDeclaration
) : KspElement(env, declaration),
XTypeElement,
- XHasModifiers by KspHasModifiers(declaration),
+ XHasModifiers by KspHasModifiers.create(declaration),
XAnnotated by KspAnnotated.create(env, declaration, NO_USE_SITE) {
override val name: String by lazy {
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/synthetic/KspSyntheticPropertyMethodElement.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/synthetic/KspSyntheticPropertyMethodElement.kt
index 7b15858..97c42f8 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/synthetic/KspSyntheticPropertyMethodElement.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/synthetic/KspSyntheticPropertyMethodElement.kt
@@ -35,6 +35,7 @@
import androidx.room.compiler.processing.ksp.KspProcessingEnv
import androidx.room.compiler.processing.ksp.KspTypeElement
import androidx.room.compiler.processing.ksp.overrides
+import com.google.devtools.ksp.symbol.KSPropertyAccessor
import java.util.Locale
/**
@@ -47,11 +48,13 @@
*/
internal sealed class KspSyntheticPropertyMethodElement(
val env: KspProcessingEnv,
- val field: KspFieldElement
+ val field: KspFieldElement,
+ accessor: KSPropertyAccessor?
) : XMethodElement,
XEquality,
- XHasModifiers by KspHasModifiers(
- field.declaration
+ XHasModifiers by KspHasModifiers.createForSyntheticAccessor(
+ field.declaration,
+ accessor
) {
// NOTE: modifiers of the property are not necessarily my modifiers.
// that being said, it only matters if it is private in which case KAPT does not generate the
@@ -98,7 +101,8 @@
field: KspFieldElement
) : KspSyntheticPropertyMethodElement(
env = env,
- field = field
+ field = field,
+ accessor = field.declaration.getter
),
XAnnotated by KspAnnotated.create(
env = env,
@@ -148,7 +152,8 @@
field: KspFieldElement
) : KspSyntheticPropertyMethodElement(
env = env,
- field = field
+ field = field,
+ accessor = field.declaration.setter
),
XAnnotated by KspAnnotated.create(
env = env,
diff --git a/room/compiler-processing/src/test/java/androidx/room/compiler/processing/XExecutableElementTest.kt b/room/compiler-processing/src/test/java/androidx/room/compiler/processing/XExecutableElementTest.kt
index 92d3afe..bb3b019 100644
--- a/room/compiler-processing/src/test/java/androidx/room/compiler/processing/XExecutableElementTest.kt
+++ b/room/compiler-processing/src/test/java/androidx/room/compiler/processing/XExecutableElementTest.kt
@@ -236,6 +236,12 @@
private set
get() = TODO()
private val prop4:String = ""
+ protected var prop5:String = ""
+ var prop6: String
+ get // this cannot be protected, https://youtrack.jetbrains.com/issue/KT-3110
+ protected set
+ protected var prop7: String
+ private set
}
""".trimIndent()
)
@@ -246,21 +252,37 @@
}
assertThat(methodNames).containsNoneIn(
listOf(
- "setX", "setProp3", "setZ", "setProp4", "getProp4"
+ "setX", "setProp1", "setProp3", "setZ", "setProp4", "getProp4", "setProp7"
)
)
- listOf("getX", "getProp1", "getProp2", "getProp3").forEach {
- klass.getMethod(it).let { method ->
- assertThat(method.returnType.typeName).isEqualTo(invocation.types.string)
- assertThat(method.parameters).isEmpty()
+ listOf("getX", "getProp1", "getProp2", "getProp3", "getProp5", "getProp6")
+ .forEach {
+ klass.getMethod(it).let { method ->
+ assertThat(method.returnType.typeName).isEqualTo(invocation.types.string)
+ assertThat(method.parameters).isEmpty()
+ }
}
- }
listOf("setY", "setProp2").forEach {
klass.getMethod(it).let { method ->
assertThat(method.returnType.typeName).isEqualTo(invocation.types.voidOrUnit)
assertThat(method.parameters.first().type.typeName).isEqualTo(
invocation.types.string
)
+ assertThat(method.isPublic()).isTrue()
+ }
+ }
+ listOf("getProp5", "getProp7").forEach {
+ klass.getMethod(it).let { method ->
+ assertThat(method.isProtected()).isTrue()
+ assertThat(method.isPublic()).isFalse()
+ assertThat(method.isPrivate()).isFalse()
+ }
+ }
+ listOf("setProp5", "setProp6").forEach {
+ klass.getMethod(it).let { method ->
+ assertThat(method.isProtected()).isTrue()
+ assertThat(method.isPublic()).isFalse()
+ assertThat(method.isPrivate()).isFalse()
}
}
}
diff --git a/room/compiler-processing/src/test/java/androidx/room/compiler/processing/ksp/KspFieldElementTest.kt b/room/compiler-processing/src/test/java/androidx/room/compiler/processing/ksp/KspFieldElementTest.kt
index 382e5b9..e5a0592 100644
--- a/room/compiler-processing/src/test/java/androidx/room/compiler/processing/ksp/KspFieldElementTest.kt
+++ b/room/compiler-processing/src/test/java/androidx/room/compiler/processing/ksp/KspFieldElementTest.kt
@@ -35,17 +35,50 @@
"""
class Foo {
val intField: Int = 0
+ @JvmField
+ val jvmField: Int = 0
+ protected val protectedField: Int = 0
+ @JvmField
+ protected val protectedJvmField: Int = 0
}
""".trimIndent()
)
runKspTest(sources = listOf(src), succeed = true) { invocation ->
val fooElement = invocation.processingEnv.requireTypeElement("Foo")
- fooElement
- .getField("intField").let { field ->
- assertThat(field.name).isEqualTo("intField")
- assertThat(field.kindName()).isEqualTo("property")
- assertThat(field.enclosingTypeElement).isEqualTo(fooElement)
- }
+ fooElement.getField("intField").let { field ->
+ assertThat(field.name).isEqualTo("intField")
+ assertThat(field.kindName()).isEqualTo("property")
+ assertThat(field.enclosingTypeElement).isEqualTo(fooElement)
+ assertThat(field.isPublic()).isFalse()
+ assertThat(field.isProtected()).isFalse()
+ // from java, it is private
+ assertThat(field.isPrivate()).isTrue()
+ }
+ fooElement.getField("jvmField").let { field ->
+ assertThat(field.name).isEqualTo("jvmField")
+ assertThat(field.kindName()).isEqualTo("property")
+ assertThat(field.enclosingTypeElement).isEqualTo(fooElement)
+ assertThat(field.isPublic()).isTrue()
+ assertThat(field.isProtected()).isFalse()
+ assertThat(field.isPrivate()).isFalse()
+ }
+ fooElement.getField("protectedField").let { field ->
+ assertThat(field.name).isEqualTo("protectedField")
+ assertThat(field.kindName()).isEqualTo("property")
+ assertThat(field.enclosingTypeElement).isEqualTo(fooElement)
+ assertThat(field.isPublic()).isFalse()
+ assertThat(field.isProtected()).isFalse()
+ // from java, it is private
+ assertThat(field.isPrivate()).isTrue()
+ }
+ fooElement.getField("protectedJvmField").let { field ->
+ assertThat(field.name).isEqualTo("protectedJvmField")
+ assertThat(field.kindName()).isEqualTo("property")
+ assertThat(field.enclosingTypeElement).isEqualTo(fooElement)
+ assertThat(field.isPublic()).isFalse()
+ assertThat(field.isProtected()).isTrue()
+ assertThat(field.isPrivate()).isFalse()
+ }
}
}
