Read property accessor meteadata in KAPT
This CL updates KotlinMetadataElement to also read property accessors
and synthesize KmFunction for them so that JavacExecutableElement can
find them.
This does not have any impact right now but will eventually be used to
support internal properties properly.
Note that, accessors are not functions in metadata. As such, I'm
synthesizing a KmFunction from KmAccessor, which may not be 100%
accurate. When in doubt, I preferred 0 flags (instead of flags of
properties etc). Since accessors are not always defined, I also made the
KmValueParameter's name nullable.
I've also added name and jvmName to KmFunction to distinguish between
the name in source vs the name generated by the kotlin compiler. This
will again be used for supporting internal properties once we add
relevant API to XMethodElement.
Bug: 205289020
Test: KotlinMetadataElementTest
Change-Id: If85f0832ee91bcdff25a660b11e1a38e625fc60a
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/kotlin/KotlinClassMetadataUtils.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/kotlin/KotlinClassMetadataUtils.kt
index 30f0cac..ca4d05a 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/kotlin/KotlinClassMetadataUtils.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/kotlin/KotlinClassMetadataUtils.kt
@@ -16,6 +16,7 @@
package androidx.room.compiler.processing.javac.kotlin
+import androidx.room.compiler.processing.util.sanitizeAsJavaParameterName
import kotlinx.metadata.ClassName
import kotlinx.metadata.Flag
import kotlinx.metadata.Flags
@@ -25,15 +26,19 @@
import kotlinx.metadata.KmExtensionType
import kotlinx.metadata.KmFunctionExtensionVisitor
import kotlinx.metadata.KmFunctionVisitor
+import kotlinx.metadata.KmPropertyExtensionVisitor
import kotlinx.metadata.KmPropertyVisitor
import kotlinx.metadata.KmTypeParameterVisitor
import kotlinx.metadata.KmTypeVisitor
import kotlinx.metadata.KmValueParameterVisitor
import kotlinx.metadata.KmVariance
import kotlinx.metadata.jvm.JvmConstructorExtensionVisitor
+import kotlinx.metadata.jvm.JvmFieldSignature
import kotlinx.metadata.jvm.JvmFunctionExtensionVisitor
import kotlinx.metadata.jvm.JvmMethodSignature
+import kotlinx.metadata.jvm.JvmPropertyExtensionVisitor
import kotlinx.metadata.jvm.KotlinClassMetadata
+import java.util.Locale
// represents a function or constructor
internal interface KmExecutable {
@@ -44,6 +49,14 @@
* Represents the kotlin metadata of a function
*/
internal data class KmFunction(
+ /**
+ * Name of the function in byte code
+ */
+ val jvmName: String,
+ /**
+ * Name of the function in source code
+ */
+ val name: String,
val descriptor: String,
private val flags: Flags,
override val parameters: List<KmValueParameter>,
@@ -65,7 +78,9 @@
internal data class KmProperty(
val name: String,
- val type: KmType
+ val type: KmType,
+ val getter: KmFunction?,
+ val setter: KmFunction?
) {
val typeParameters
get() = type.typeArguments
@@ -118,7 +133,7 @@
override fun visitFunction(flags: Flags, name: String): KmFunctionVisitor {
return object : KmFunctionVisitor() {
- lateinit var descriptor: String
+ lateinit var methodSignature: JvmMethodSignature
val parameters = mutableListOf<KmValueParameter>()
lateinit var returnType: KmType
@@ -137,7 +152,7 @@
}
return object : JvmFunctionExtensionVisitor() {
override fun visit(signature: JvmMethodSignature?) {
- descriptor = signature!!.asString()
+ methodSignature = signature!!
}
}
}
@@ -149,7 +164,16 @@
}
override fun visitEnd() {
- result.add(KmFunction(descriptor, flags, parameters, returnType))
+ result.add(
+ KmFunction(
+ name = name,
+ jvmName = methodSignature.name,
+ descriptor = methodSignature.asString(),
+ flags = flags,
+ parameters = parameters,
+ returnType = returnType
+ )
+ )
}
}
}
@@ -239,13 +263,45 @@
getterFlags: Flags,
setterFlags: Flags
): KmPropertyVisitor {
+ var setterParam: KmValueParameter? = null
+ var getter: JvmMethodSignature? = null
+ var setter: JvmMethodSignature? = null
return object : KmPropertyVisitor() {
lateinit var returnType: KmType
override fun visitEnd() {
result.add(
KmProperty(
type = returnType,
- name = name
+ name = name,
+ setter = setter?.let { setterSignature ->
+ // setter parameter visitor may not be invoked when not declared
+ // explicitly
+ val param = setterParam ?: KmValueParameter(
+ // kotlinc will set this to set-? but it is better to not expose
+ // it here since it is not valid name
+ name = "set-?".sanitizeAsJavaParameterName(0),
+ type = returnType,
+ flags = 0
+ )
+ KmFunction(
+ jvmName = setterSignature.name,
+ name = computeSetterName(name),
+ descriptor = setterSignature.asString(),
+ flags = 0,
+ parameters = listOf(param),
+ returnType = KM_VOID_TYPE
+ )
+ },
+ getter = getter?.let { getterSignature ->
+ KmFunction(
+ jvmName = getterSignature.name,
+ name = computeGetterName(name),
+ descriptor = getterSignature.asString(),
+ flags = flags,
+ parameters = emptyList(),
+ returnType = returnType
+ )
+ }
)
)
}
@@ -255,6 +311,35 @@
returnType = it
}
}
+
+ override fun visitSetterParameter(
+ flags: Flags,
+ name: String
+ ): KmValueParameterVisitor {
+ return ValueParameterReader(
+ name = name,
+ flags = flags
+ ) {
+ setterParam = it
+ }
+ }
+
+ override fun visitExtensions(type: KmExtensionType): KmPropertyExtensionVisitor? {
+ if (type != JvmPropertyExtensionVisitor.TYPE) {
+ return null
+ }
+ return object : JvmPropertyExtensionVisitor() {
+ override fun visit(
+ jvmFlags: Flags,
+ fieldSignature: JvmFieldSignature?,
+ getterSignature: JvmMethodSignature?,
+ setterSignature: JvmMethodSignature?
+ ) {
+ getter = getterSignature
+ setter = setterSignature
+ }
+ }
+ }
}
}
}
@@ -388,3 +473,47 @@
}
}
}
+
+/**
+ * Computes the getter name based on the property. Note that this might be different than the
+ * JVM name for internal properties.
+ * See: https://kotlinlang.org/docs/java-to-kotlin-interop.html#properties
+ */
+private fun computeGetterName(
+ propName: String
+): String {
+ return if (propName.startsWith("is")) {
+ propName
+ } else {
+ val capitalizedName = propName.replaceFirstChar {
+ if (it.isLowerCase()) it.titlecase(
+ Locale.US
+ ) else it.toString()
+ }
+ "get$capitalizedName"
+ }
+}
+
+/**
+ * Computes the getter name based on the property. Note that this might be different than the
+ * JVM name for internal properties.
+ * See: https://kotlinlang.org/docs/java-to-kotlin-interop.html#properties
+ */
+private fun computeSetterName(propName: String): String {
+ return if (propName.startsWith("is")) {
+ "set${propName.substring(2)}"
+ } else {
+ val capitalizedName = propName.replaceFirstChar {
+ if (it.isLowerCase()) it.titlecase(
+ Locale.US
+ ) else it.toString()
+ }
+ "set$capitalizedName"
+ }
+}
+
+private val KM_VOID_TYPE = KmType(
+ flags = 0,
+ typeArguments = emptyList(),
+ extendsBound = null
+)
\ No newline at end of file
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/kotlin/KotlinMetadataElement.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/kotlin/KotlinMetadataElement.kt
index 10b7be7..adb2631 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/kotlin/KotlinMetadataElement.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/kotlin/KotlinMetadataElement.kt
@@ -71,7 +71,23 @@
"must pass an element type of method"
}
val methodSignature = method.descriptor
- return functionList.firstOrNull { it.descriptor == methodSignature }
+ functionList.firstOrNull { it.descriptor == methodSignature }?.let {
+ return it
+ }
+ // might be a property getter or setter
+ return propertyList.firstNotNullOfOrNull { property ->
+ when {
+ property.getter?.descriptor == methodSignature -> {
+ property.getter
+ }
+ property.setter?.descriptor == methodSignature -> {
+ property.setter
+ }
+ else -> {
+ null
+ }
+ }
+ }
}
fun getConstructorMetadata(method: ExecutableElement): KmConstructor? {
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/util/NamingUtils.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/util/NamingUtils.kt
index 1d96f2d..7889069 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/util/NamingUtils.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/util/NamingUtils.kt
@@ -27,5 +27,5 @@
): String = if (this != null && SourceVersion.isName(this)) {
this
} else {
- "arg$argIndex"
+ "p$argIndex"
}
\ No newline at end of file
diff --git a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/FallbackLocationInformationTest.kt b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/FallbackLocationInformationTest.kt
index b4cbf09..fa4306c 100644
--- a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/FallbackLocationInformationTest.kt
+++ b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/FallbackLocationInformationTest.kt
@@ -49,10 +49,10 @@
class JavaSubject {
String field1;
// naming this arg0 because javac cannot read the real param name after compilation
- JavaSubject(int arg0) {
+ JavaSubject(int p0) {
}
// naming this arg0 because javac cannot read the real param name after compilation
- void method1(int arg0) {}
+ void method1(int p0) {}
}
""".trimIndent()
)
@@ -109,7 +109,7 @@
assertThat(
propSetter.parameters.first().fallbackLocationText
).isEqualTo(
- "arg0 in foo.bar.KotlinSubject.setProp(java.lang.String)"
+ "p0 in foo.bar.KotlinSubject.setProp(java.lang.String)"
)
}
diff --git a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XExecutableElementTest.kt b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XExecutableElementTest.kt
index 7a03f87..a30674a 100644
--- a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XExecutableElementTest.kt
+++ b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XExecutableElementTest.kt
@@ -416,6 +416,7 @@
protected set
protected var prop7: String
private set
+ internal var prop8: String
}
""".trimIndent()
)
@@ -429,19 +430,24 @@
"setX", "setProp1", "setProp3", "setZ", "setProp4", "getProp4", "setProp7"
)
)
- listOf("getX", "getProp1", "getProp2", "getProp3", "getProp5", "getProp6").forEach {
+ listOf("getX", "getProp1", "getProp2", "getProp3", "getProp5", "getProp6",
+ "getProp8\$main").forEach {
klass.getMethod(it).let { method ->
assertThat(method.returnType.typeName).isEqualTo(String::class.typeName())
assertThat(method.parameters).isEmpty()
+ assertThat(method.returnType.nullability).isEqualTo(XNullability.NONNULL)
}
}
- listOf("setY", "setProp2").forEach {
+ listOf("setY", "setProp2", "setProp8\$main").forEach {
klass.getMethod(it).let { method ->
assertThat(method.returnType.typeName).isEqualTo(TypeName.VOID)
assertThat(method.parameters.first().type.typeName).isEqualTo(
String::class.typeName()
)
assertThat(method.isPublic()).isTrue()
+ assertThat(method.parameters.first().type.nullability).isEqualTo(
+ XNullability.NONNULL
+ )
}
}
listOf("getProp5", "getProp7").forEach {
diff --git a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/javac/kotlin/KotlinMetadataElementTest.kt b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/javac/kotlin/KotlinMetadataElementTest.kt
index 6825149..79b3ae2 100644
--- a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/javac/kotlin/KotlinMetadataElementTest.kt
+++ b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/javac/kotlin/KotlinMetadataElementTest.kt
@@ -16,22 +16,29 @@
package androidx.room.compiler.processing.javac.kotlin
+import androidx.room.compiler.processing.XNullability
import androidx.room.compiler.processing.javac.JavacProcessingEnv
+import androidx.room.compiler.processing.javac.nullability
import androidx.room.compiler.processing.util.Source
import androidx.room.compiler.processing.util.compileFiles
import androidx.room.compiler.processing.util.runJavaProcessorTest
import androidx.room.compiler.processing.util.runKaptTest
+import androidx.room.compiler.processing.util.sanitizeAsJavaParameterName
import com.google.common.truth.Truth.assertThat
+import com.google.common.truth.Truth.assertWithMessage
import org.junit.AssumptionViolatedException
import org.junit.Test
import org.junit.runner.RunWith
-import org.junit.runners.JUnit4
+import org.junit.runners.Parameterized
import javax.annotation.processing.ProcessingEnvironment
+import javax.lang.model.element.ExecutableElement
import javax.lang.model.element.TypeElement
import javax.lang.model.util.ElementFilter
-@RunWith(JUnit4::class)
-class KotlinMetadataElementTest {
+@RunWith(Parameterized::class)
+class KotlinMetadataElementTest(
+ private val preCompiled: Boolean
+) {
@Test
fun constructorParameters() {
@@ -197,7 +204,7 @@
"suspendFunction" to true,
"functionWithParams" to false,
"suspendFunctionWithParams" to false,
- "getConstructorParam" to null // synthetic getter for constructor property
+ "getConstructorParam" to false // synthetic getter for constructor property
)
}
}
@@ -307,6 +314,250 @@
}
@Test
+ fun accessors() {
+ val src = Source.kotlin(
+ "Kotlin.kt",
+ """
+ @JvmInline
+ value class ValueClass(val value: String)
+ class Subject {
+ val immutableProperty: String = ""
+ var mutableProperty: String = ""
+ var isProperty: String? = ""
+ var customSetter: String= ""
+ get(): String = ""
+ set(myValue) { field = myValue }
+ var privateSetter: String = ""
+ private set
+ internal var internalProp: String? = ""
+ internal var isInternalProp2: String = ""
+ // these won't show up in KAPT stubs since they don't have a valid JVM name but
+ // we are still testing them for consistency as they'll show up in metadata
+ var valueProp: ValueClass? = null
+ // these won't show up in KAPT stubs since they don't have a valid JVM name but
+ // we are still testing them for consistency as they'll show up in metadata
+ internal var internalValueProp: ValueClass = ValueClass("?")
+ }
+ """.trimIndent()
+ )
+ simpleRun(listOf(src)) { invocation ->
+ val (element, metadata) = getMetadataElement(
+ invocation,
+ "Subject"
+ )
+
+ fun assertSetter(
+ kmFunction: KmFunction?,
+ name: String,
+ jvmName: String,
+ paramNullable: Boolean
+ ) {
+ checkNotNull(kmFunction)
+ assertWithMessage(kmFunction.toString()).apply {
+ that(kmFunction.name).isEqualTo(name)
+ that(kmFunction.jvmName).isEqualTo(jvmName)
+ // void is non null
+ that(kmFunction.returnType.nullability).isEqualTo(XNullability.NONNULL)
+ that(kmFunction.parameters).hasSize(1)
+ that(
+ kmFunction.parameters.single().isNullable()
+ ).isEqualTo(paramNullable)
+ }
+ }
+
+ fun assertSetter(
+ metadata: KotlinMetadataElement,
+ method: ExecutableElement,
+ name: String,
+ jvmName: String,
+ paramNullable: Boolean
+ ) {
+ val kmFunction = metadata.getFunctionMetadata(method)
+ assertSetter(
+ kmFunction = kmFunction,
+ name = name,
+ jvmName = jvmName,
+ paramNullable = paramNullable
+ )
+ val paramName = kmFunction!!.parameters.single().name
+ val javacElementName = method.parameters.single().simpleName.toString()
+ assertWithMessage(
+ kmFunction.toString()
+ ).that(
+ paramName
+ ).isEqualTo(
+ javacElementName.sanitizeAsJavaParameterName(0)
+ )
+ }
+
+ fun assertGetter(
+ kmFunction: KmFunction?,
+ name: String,
+ jvmName: String,
+ returnsNullable: Boolean
+ ) {
+ checkNotNull(kmFunction)
+ assertWithMessage(kmFunction.toString()).apply {
+ that(kmFunction.name).isEqualTo(name)
+ that(kmFunction.jvmName).isEqualTo(jvmName)
+ that(kmFunction.returnType.isNullable()).isEqualTo(returnsNullable)
+ that(kmFunction.parameters).isEmpty()
+ }
+ }
+ assertGetter(
+ kmFunction = metadata.getFunctionMetadata(
+ element.getDeclaredMethod("getImmutableProperty")
+ ),
+ name = "getImmutableProperty",
+ jvmName = "getImmutableProperty",
+ returnsNullable = false
+ )
+ assertGetter(
+ kmFunction = metadata.getFunctionMetadata(
+ element.getDeclaredMethod("getMutableProperty")
+ ),
+ name = "getMutableProperty",
+ jvmName = "getMutableProperty",
+ returnsNullable = false
+ )
+ assertSetter(
+ metadata = metadata,
+ method = element.getDeclaredMethod("setMutableProperty"),
+ name = "setMutableProperty",
+ jvmName = "setMutableProperty",
+ paramNullable = false
+ )
+ assertSetter(
+ metadata = metadata,
+ method = element.getDeclaredMethod("setProperty"),
+ name = "setProperty",
+ jvmName = "setProperty",
+ paramNullable = true
+ )
+ assertGetter(
+ kmFunction = metadata.getFunctionMetadata(
+ element.getDeclaredMethod("isProperty")
+ ),
+ name = "isProperty",
+ jvmName = "isProperty",
+ returnsNullable = true
+ )
+ assertGetter(
+ kmFunction = metadata.getFunctionMetadata(
+ element.getDeclaredMethod("getInternalProp\$main")
+ ),
+ name = "getInternalProp",
+ jvmName = "getInternalProp\$main",
+ returnsNullable = true
+ )
+ assertSetter(
+ metadata = metadata,
+ method = element.getDeclaredMethod("setInternalProp\$main"),
+ name = "setInternalProp",
+ jvmName = "setInternalProp\$main",
+ paramNullable = true
+ )
+ assertGetter(
+ kmFunction = metadata.getFunctionMetadata(
+ element.getDeclaredMethod("isInternalProp2\$main")
+ ),
+ name = "isInternalProp2",
+ jvmName = "isInternalProp2\$main",
+ returnsNullable = false
+ )
+ assertSetter(
+ metadata = metadata,
+ method = element.getDeclaredMethod("setInternalProp2\$main"),
+ name = "setInternalProp2",
+ jvmName = "setInternalProp2\$main",
+ paramNullable = false
+ )
+ // read custom setter name properly
+ metadata.getFunctionMetadata(
+ element.getDeclaredMethod("setCustomSetter")
+ ).let { kmFunction ->
+ checkNotNull(kmFunction)
+ assertThat(
+ kmFunction.parameters.single().name
+ ).isEqualTo("myValue")
+ }
+ // tests value class properties. They won't show up in KAPT stubs since they don't have
+ // valid java source names but we still validate them here for consistency. Maybe one
+ // day we'll change Javac element to include these if we support Kotlin codegen in KAPT
+ metadata.getPropertyMetadata("valueProp").let { valueProp ->
+ assertGetter(
+ kmFunction = valueProp?.getter,
+ name = "getValueProp",
+ jvmName = "getValueProp-4LjoGxk",
+ returnsNullable = true
+ )
+ assertSetter(
+ kmFunction = valueProp?.setter,
+ name = "setValueProp",
+ jvmName = "setValueProp-d8IPsTA",
+ paramNullable = true
+ )
+ }
+ metadata.getPropertyMetadata("internalValueProp").let { valueProp ->
+ assertGetter(
+ kmFunction = valueProp?.getter,
+ name = "getInternalValueProp",
+ jvmName = "getInternalValueProp-NMFyIOA\$main",
+ returnsNullable = false
+ )
+ assertSetter(
+ kmFunction = valueProp?.setter,
+ name = "setInternalValueProp",
+ jvmName = "setInternalValueProp-FVOWAsA\$main",
+ paramNullable = false
+ )
+ }
+ }
+ }
+
+ @Test
+ fun internalMethodName() {
+ val src = Source.kotlin(
+ "Kotlin.kt",
+ """
+ class Subject {
+ internal fun internalFun() {}
+ fun normalFun() {}
+ // there is no test case for functions receiving/returning value classes because
+ // they are not visible through KAPT
+ }
+ """.trimIndent()
+ )
+ simpleRun(listOf(src)) { invocation ->
+ val (element, metadata) = getMetadataElement(
+ invocation,
+ "Subject"
+ )
+ metadata.getFunctionMetadata(
+ element.getDeclaredMethod("internalFun\$main")
+ ).let { functionMetadata ->
+ assertThat(
+ functionMetadata?.jvmName
+ ).isEqualTo("internalFun\$main")
+ assertThat(
+ functionMetadata?.name
+ ).isEqualTo("internalFun")
+ }
+
+ metadata.getFunctionMetadata(
+ element.getDeclaredMethod("normalFun")
+ ).let { functionMetadata ->
+ assertThat(
+ functionMetadata?.jvmName
+ ).isEqualTo("normalFun")
+ assertThat(
+ functionMetadata?.name
+ ).isEqualTo("normalFun")
+ }
+ }
+ }
+
+ @Test
fun genericParameterNullability() {
val src = Source.kotlin(
"Subject.kt",
@@ -470,6 +721,9 @@
@Test
fun withoutKotlinInClasspath() {
+ if (preCompiled) {
+ throw AssumptionViolatedException("this test doesn't care for precompiled code")
+ }
val libSource = Source.kotlin(
"lib.kt",
"""
@@ -505,11 +759,17 @@
private fun TypeElement.getConstructors() = ElementFilter.constructorsIn(enclosedElements)
+ @Suppress("NAME_SHADOWING") // intentional
private fun simpleRun(
sources: List<Source> = emptyList(),
handler: (ProcessingEnvironment) -> Unit
) {
- runKaptTest(sources) {
+ val (sources, classpath) = if (preCompiled) {
+ emptyList<Source>() to compileFiles(sources)
+ } else {
+ sources to emptyList()
+ }
+ runKaptTest(sources = sources, classpath = classpath) {
val processingEnv = it.processingEnv
if (processingEnv !is JavacProcessingEnv) {
throw AssumptionViolatedException("This test only works for java/kapt compilation")
@@ -522,4 +782,10 @@
processingEnv.elementUtils.getTypeElement(qName).let {
it to KotlinMetadataElement.createFor(it)!!
}
+
+ companion object {
+ @JvmStatic
+ @Parameterized.Parameters(name = "preCompiled_{0}")
+ fun params() = arrayOf(false, true)
+ }
}
\ No newline at end of file
