Merge "Read property accessor meteadata in KAPT" into androidx-main
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
