Skip to content

Commit

Permalink
Fix type names for inline types on the KSP side
Browse files Browse the repository at this point in the history 
For inline value types used as type arguments we should generate boxed type names. For example:

For type `kotlin.collections.List<kotlin.UInt>`, we should generate `java.util.List<kotlin.UInt>` instead of `java.util.List<java.lang.Integer>`.

This also applies to generic value types. So for type `kotlin.collections.List<kotlin.Result<kotlin.Int>>`, we should generate `java.util.List<kotlin.Result<java.lang.Integer>>` instead of `java.util.List<java.lang.Object>`.

Bug: google/dagger#4096
Test: XTypeTest
Change-Id: I3fcd8cb8e5b8f81bb503eb278cd2b8d39b1e991e
  • Loading branch information
@kuanyingchou
kuanyingchou committed Oct 17, 2023
1 parent 4d1d187 commit 446445c
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 45 deletions.
14 changes: 11 additions & 3 deletions ...compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/DefaultKspType.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,17 @@ internal class DefaultKspType(
) : KspType(env, ksType, originalKSAnnotations, scope, typeAlias) {

override fun resolveJTypeName(): JTypeName {
// always box these. For primitives, typeName might return the primitive type but if we
// wanted it to be a primitive, we would've resolved it to [KspPrimitiveType].
return ksType.asJTypeName(env.resolver).tryBox()
// Always box these unless for inline value classes. For primitives, typeName might return
// the primitive type but if we wanted it to be a primitive, we would've resolved it to
// [KspPrimitiveType]. Inline value classes with primitive values won't be resolved to
// [KspPrimitiveType] because we need boxed name for Kotlin and unboxed name for Java.
return if (ksType.declaration.isValueClass()) {
// Don't box inline value classes, e.g. the type name for `UInt` should be `int`,
// not `Integer`, if used directly.
ksType.asJTypeName(env.resolver)
} else {
ksType.asJTypeName(env.resolver).tryBox()
}
}

override fun resolveKTypeName(): KTypeName {
Expand Down
20 changes: 12 additions & 8 deletions ...piler-processing/src/main/java/androidx/room/compiler/processing/ksp/KSTypeJavaPoetExt.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import androidx.room.compiler.processing.javac.kotlin.typeNameFromJvmSignature
import androidx.room.compiler.processing.tryBox
import androidx.room.compiler.processing.util.ISSUE_TRACKER_LINK
import com.google.devtools.ksp.KspExperimental
import com.google.devtools.ksp.isAnnotationPresent
import com.google.devtools.ksp.processing.Resolver
import com.google.devtools.ksp.symbol.KSDeclaration
import com.google.devtools.ksp.symbol.KSName
Expand All @@ -30,7 +29,6 @@ import com.google.devtools.ksp.symbol.KSTypeAlias
import com.google.devtools.ksp.symbol.KSTypeArgument
import com.google.devtools.ksp.symbol.KSTypeParameter
import com.google.devtools.ksp.symbol.KSTypeReference
import com.google.devtools.ksp.symbol.Modifier
import com.google.devtools.ksp.symbol.Variance
import com.squareup.kotlinpoet.javapoet.JClassName
import com.squareup.kotlinpoet.javapoet.JParameterizedTypeName
Expand Down Expand Up @@ -98,13 +96,14 @@ private fun KSDeclaration.asJTypeName(
val qualified = qualifiedName?.asString() ?: return ERROR_JTYPE_NAME
val pkg = getNormalizedPackageName()

// We want to map Kotlin types to equivalent Java types if there is one (e.g.
// kotlin.String to java.lang.String or kotlin.collections.List to java.util.List).
// Note: To match KAPT behavior, a type annotated with @JvmInline is only replaced with the
// underlying type if the inline type is used directly (e.g. MyInlineType) rather than in the
// type args of another type, (e.g. List<MyInlineType>).
val isInlineUsedDirectly =
(isAnnotationPresent(JvmInline::class) || modifiers.contains(Modifier.INLINE)) &&
typeResolutionContext.originalType?.declaration?.qualifiedName?.asString() == qualified
if (pkg == "kotlin" || pkg.startsWith("kotlin.") || isInlineUsedDirectly) {
val isInline = isValueClass()
val isKotlinType = pkg == "kotlin" || pkg.startsWith("kotlin.")
if ((isInline && isUsedDirectly(typeResolutionContext)) || (!isInline && isKotlinType)) {
val jvmSignature = resolver.mapToJvmSignature(this)
if (!jvmSignature.isNullOrBlank()) {
return jvmSignature.typeNameFromJvmSignature()
Expand Down Expand Up @@ -180,9 +179,9 @@ private fun KSType.asJTypeName(
return if (declaration is KSTypeAlias) {
replaceTypeAliases(resolver).asJTypeName(resolver, typeResolutionContext)
} else if (this.arguments.isNotEmpty() && !resolver.isJavaRawType(this) &&
// Excluding generic value classes otherwise we may generate something
// Excluding generic value classes used directly otherwise we may generate something
// like `Object<String>`.
!declaration.isValueClass()) {
!(declaration.isValueClass() && declaration.isUsedDirectly(typeResolutionContext))) {
val args: Array<JTypeName> = this.arguments
.map { typeArg -> typeArg.asJTypeName(resolver, typeResolutionContext) }
.map { it.tryBox() }
Expand Down Expand Up @@ -234,3 +233,8 @@ private fun createModifiableTypeVariableName(
name,
bounds
) as JTypeVariableName

private fun KSDeclaration.isUsedDirectly(typeResolutionContext: TypeResolutionContext): Boolean {
val qualified = qualifiedName?.asString()
return typeResolutionContext.originalType?.declaration?.qualifiedName?.asString() == qualified
}
150 changes: 116 additions & 34 deletions room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XTypeTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1848,16 +1848,7 @@ class XTypeTest {
"KotlinClass.kt",
"""
@JvmInline value class PackageName(val value: String)
@JvmInline value class MyResult<T>(val value: T)
class KotlinClass {
// @JvmName disables name mangling for functions that use inline classes and
// make them visible to Java:
// https://kotlinlang.org/docs/inline-classes.html#calling-from-java-code
@JvmName("getResult")
fun getResult(): MyResult<String> = TODO()
@JvmName("setResult")
fun setResult(result: MyResult<String>) { }
fun getPackageNames(): Set<PackageName> = emptySet()
fun setPackageNames(pkgNames: Set<PackageName>) { }
}
Expand All @@ -1869,31 +1860,6 @@ class XTypeTest {
) { invocation ->
val kotlinElm = invocation.processingEnv.requireTypeElement("KotlinClass")

kotlinElm.getDeclaredMethodByJvmName("getResult").apply {
assertThat(returnType.asTypeName().java.toString())
.isEqualTo("java.lang.Object")
if (invocation.isKsp) {
assertThat(returnType.asTypeName().kotlin.toString())
.isEqualTo("MyResult<kotlin.String>")
} else {
// Can't generate Kotlin code with KAPT
assertThat(returnType.asTypeName().kotlin.toString())
.isEqualTo("androidx.room.compiler.codegen.Unavailable")
}
}
kotlinElm.getDeclaredMethodByJvmName("setResult").apply {
assertThat(parameters.single().type.asTypeName().java.toString())
.isEqualTo("java.lang.Object")
if (invocation.isKsp) {
assertThat(parameters.single().type.asTypeName().kotlin.toString())
.isEqualTo("MyResult<kotlin.String>")
} else {
// Can't generate Kotlin code with KAPT
assertThat(parameters.single().type.asTypeName().kotlin.toString())
.isEqualTo("androidx.room.compiler.codegen.Unavailable")
}
}

kotlinElm.getMethodByJvmName("getPackageNames").apply {
assertThat(returnType.typeName.toString())
.isEqualTo("java.util.Set<PackageName>")
Expand All @@ -1909,4 +1875,120 @@ class XTypeTest {
}
}
}

@Test
fun jvmTypes(@TestParameter isPrecompiled: Boolean) {
val kotlinSrc = Source.kotlin(
"KotlinClass.kt",
"""
@JvmInline value class MyInlineClass(val value: Int)
@JvmInline value class MyGenericInlineClass<T: Number>(val value: T)
class KotlinClass {
// @JvmName disables name mangling for functions that use inline classes directly
// and make them visible to Java:
// https://kotlinlang.org/docs/inline-classes.html#calling-from-java-code
@JvmName("kotlinValueClassDirectUsage")
fun kotlinValueClassDirectUsage(): UInt = TODO()
fun kotlinValueClassIndirectUsage(): List<UInt> = TODO()
fun kotlinNonValueClassDirectUsage(): String = TODO()
fun kotlinNonValueClassIndirectUsage(): List<String> = TODO()
@JvmName("kotlinGenericValueClassDirectUsage")
fun kotlinGenericValueClassDirectUsage(): Result<Int> = TODO()
fun kotlinGenericValueClassIndirectUsage(): List<Result<Int>> = TODO()
@JvmName("nonKotlinValueClassDirectUsage")
fun nonKotlinValueClassDirectUsage(): MyInlineClass = TODO()
fun nonKotlinValueClassIndirectUsage(): List<MyInlineClass> = TODO()
@JvmName("nonKotlinGenericValueClassDirectUsage")
fun nonKotlinGenericValueClassDirectUsage(): MyGenericInlineClass<Int> = TODO()
fun nonKotlinGenericValueClassIndirectUsage(): List<MyGenericInlineClass<Int>> = TODO()
}
""".trimIndent()
)
runProcessorTest(
sources = if (isPrecompiled) { emptyList() } else { listOf(kotlinSrc) },
classpath = if (isPrecompiled) { compileFiles(listOf(kotlinSrc)) } else { emptyList() }
) { invocation ->
val kotlinElm = invocation.processingEnv.requireTypeElement("KotlinClass")
kotlinElm.getMethodByJvmName("kotlinValueClassDirectUsage").apply {
assertThat(returnType.asTypeName().java.toString())
.isEqualTo("int")
if (invocation.isKsp) {
assertThat(returnType.asTypeName().kotlin.toString())
.isEqualTo("kotlin.UInt")
}
}
kotlinElm.getMethodByJvmName("kotlinValueClassIndirectUsage").apply {
assertThat(returnType.asTypeName().java.toString())
.isEqualTo("java.util.List<kotlin.UInt>")
if (invocation.isKsp) {
assertThat(returnType.asTypeName().kotlin.toString())
.isEqualTo("kotlin.collections.List<kotlin.UInt>")
}
}
kotlinElm.getMethodByJvmName("kotlinNonValueClassDirectUsage").apply {
assertThat(returnType.asTypeName().java.toString())
.isEqualTo("java.lang.String")
if (invocation.isKsp) {
assertThat(returnType.asTypeName().kotlin.toString())
.isEqualTo("kotlin.String")
}
}
kotlinElm.getMethodByJvmName("kotlinNonValueClassIndirectUsage").apply {
assertThat(returnType.asTypeName().java.toString())
.isEqualTo("java.util.List<java.lang.String>")
if (invocation.isKsp) {
assertThat(returnType.asTypeName().kotlin.toString())
.isEqualTo("kotlin.collections.List<kotlin.String>")
}
}
kotlinElm.getMethodByJvmName("kotlinGenericValueClassDirectUsage").apply {
assertThat(returnType.asTypeName().java.toString())
.isEqualTo("java.lang.Object")
if (invocation.isKsp) {
assertThat(returnType.asTypeName().kotlin.toString())
.isEqualTo("kotlin.Result<kotlin.Int>")
}
}
kotlinElm.getMethodByJvmName("kotlinGenericValueClassIndirectUsage").apply {
assertThat(returnType.asTypeName().java.toString())
.isEqualTo("java.util.List<kotlin.Result<java.lang.Integer>>")
if (invocation.isKsp) {
assertThat(returnType.asTypeName().kotlin.toString())
.isEqualTo("kotlin.collections.List<kotlin.Result<kotlin.Int>>")
}
}
kotlinElm.getMethodByJvmName("nonKotlinValueClassDirectUsage").apply {
assertThat(returnType.asTypeName().java.toString())
.isEqualTo("int")
if (invocation.isKsp) {
assertThat(returnType.asTypeName().kotlin.toString())
.isEqualTo("MyInlineClass")
}
}
kotlinElm.getMethodByJvmName("nonKotlinValueClassIndirectUsage").apply {
assertThat(returnType.asTypeName().java.toString())
.isEqualTo("java.util.List<MyInlineClass>")
if (invocation.isKsp) {
assertThat(returnType.asTypeName().kotlin.toString())
.isEqualTo("kotlin.collections.List<MyInlineClass>")
}
}
kotlinElm.getMethodByJvmName("nonKotlinGenericValueClassDirectUsage").apply {
assertThat(returnType.asTypeName().java.toString())
.isEqualTo("java.lang.Number")
if (invocation.isKsp) {
assertThat(returnType.asTypeName().kotlin.toString())
.isEqualTo("MyGenericInlineClass<kotlin.Int>")
}
}
kotlinElm.getMethodByJvmName("nonKotlinGenericValueClassIndirectUsage").apply {
assertThat(returnType.asTypeName().java.toString())
.isEqualTo("java.util.List<MyGenericInlineClass<java.lang.Integer>>")
if (invocation.isKsp) {
assertThat(returnType.asTypeName().kotlin.toString())
.isEqualTo("kotlin.collections.List<MyGenericInlineClass<kotlin.Int>>")
}
}
}
}
}

0 comments on commit 446445c

Please sign in to comment.