Revert "Revert "Stop using star projection in KSP types""
This reverts commit ef7210e012fa7eca70a6a0570b86d18a2660442d.
Reason for revert: Fixed conflicting test
Bug: 204415667
Test: XTypeTest
Change-Id: I5ea529fa37222c77bc52ace326ed8075ac6e8697
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KSTypeExt.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KSTypeExt.kt
index 8e89adfe..b9f6920 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KSTypeExt.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KSTypeExt.kt
@@ -116,10 +116,8 @@
* Turns a KSTypeArgument into a TypeName in java's type system.
*/
internal fun KSTypeArgument.typeName(
- param: KSTypeParameter,
resolver: Resolver
): TypeName = typeName(
- param = param,
resolver = resolver,
typeArgumentTypeLookup = TypeArgumentTypeLookup()
)
@@ -147,7 +145,6 @@
}
private fun KSTypeArgument.typeName(
- param: KSTypeParameter,
resolver: Resolver,
typeArgumentTypeLookup: TypeArgumentTypeLookup
): TypeName {
@@ -163,7 +160,7 @@
// explicit *
WildcardTypeName.subtypeOf(TypeName.OBJECT)
} else {
- param.typeName(resolver, typeArgumentTypeLookup)
+ WildcardTypeName.subtypeOf(type.typeName(resolver, typeArgumentTypeLookup))
}
}
else -> resolveTypeName()
@@ -185,9 +182,8 @@
): TypeName {
return if (this.arguments.isNotEmpty()) {
val args: Array<TypeName> = this.arguments
- .mapIndexed { index, typeArg ->
+ .map { typeArg ->
typeArg.typeName(
- param = this.declaration.typeParameters[index],
resolver = resolver,
typeArgumentTypeLookup = typeArgumentTypeLookup
)
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspProcessingEnv.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspProcessingEnv.kt
index 3222698..3e11419 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspProcessingEnv.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspProcessingEnv.kt
@@ -105,7 +105,7 @@
return resolver.findClass(kotlinTypeName)?.let {
wrap(
allowPrimitives = KspTypeMapper.isJavaPrimitiveType(qName),
- ksType = it.asStarProjectedType()
+ ksType = it.asType(emptyList())
)
}
}
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeArgumentType.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeArgumentType.kt
index 91c26a8..24432f2 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeArgumentType.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeArgumentType.kt
@@ -47,7 +47,7 @@
}
override val typeName: TypeName by lazy {
- typeArg.typeName(typeParam, env.resolver)
+ typeArg.typeName(env.resolver)
}
override fun boxed(): KspTypeArgumentType {
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeElement.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeElement.kt
index 09bb50d..0805475 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeElement.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeElement.kt
@@ -72,7 +72,7 @@
override val type: KspType by lazy {
env.wrap(
- ksType = declaration.asStarProjectedType(),
+ ksType = declaration.asType(emptyList()),
allowPrimitives = false
)
}
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/OverrideVarianceResolver.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/OverrideVarianceResolver.kt
index b2f3cf8..728b5e2 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/OverrideVarianceResolver.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/OverrideVarianceResolver.kt
@@ -157,6 +157,8 @@
typeRef = myTypeRef.inheritVariance(overridee.type),
variance = if (overridee.variance == Variance.STAR) {
Variance.COVARIANT
+ } else if (overridee.variance == Variance.INVARIANT) {
+ param.variance
} else {
overridee.variance
}
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/ResolverExt.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/ResolverExt.kt
index 9b9a9eb..c63e831 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/ResolverExt.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/ResolverExt.kt
@@ -33,7 +33,7 @@
"cannot find class $qName"
}
-internal fun Resolver.requireType(qName: String) = requireClass(qName).asStarProjectedType()
+internal fun Resolver.requireType(qName: String) = requireClass(qName).asType(emptyList())
internal fun Resolver.requireContinuationClass() = requireClass("kotlin.coroutines.Continuation")
diff --git a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XElementTest.kt b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XElementTest.kt
index b231e22..e542d74 100644
--- a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XElementTest.kt
+++ b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XElementTest.kt
@@ -197,12 +197,7 @@
}
validateElement(
element = it.processingEnv.requireTypeElement("foo.bar.Base"),
- tTypeName = if (it.isKsp) {
- // when inheritance resolution happens, KSP resolves them to object
- TypeName.OBJECT
- } else {
- TypeVariableName.get("T")
- },
+ tTypeName = TypeVariableName.get("T"),
rTypeName = TypeVariableName.get("R")
)
validateElement(
diff --git a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XExecutableTypeTest.kt b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XExecutableTypeTest.kt
index b1d44e3..c683ad3 100644
--- a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XExecutableTypeTest.kt
+++ b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XExecutableTypeTest.kt
@@ -59,16 +59,8 @@
val constructorElement = typeElement.getConstructors().single()
val constructorType = constructorElement.executableType
- // Assert that the XConstructorElement parameter is unresolved type, T.
- // TODO(10/06/2021): Should KSP also return T?
- if (invocation.isKsp) {
- assertThat(constructorType.parameterTypes.single().typeName)
- .isEqualTo(TypeVariableName.OBJECT)
- } else {
- assertThat(constructorType.parameterTypes.single().typeName)
- .isEqualTo(TypeVariableName.get("T"))
- }
-
+ assertThat(constructorType.parameterTypes.single().typeName)
+ .isEqualTo(TypeVariableName.get("T"))
val resolvedType = typeElement.getDeclaredMethods().single().returnType
val resolvedConstructorType = constructorElement.asMemberOf(resolvedType)
diff --git a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XTypeElementTest.kt b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XTypeElementTest.kt
index 1b35fe9..34e6c53 100644
--- a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XTypeElementTest.kt
+++ b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XTypeElementTest.kt
@@ -349,12 +349,7 @@
}
baseClass.getField("genericProp").let { field ->
- if (invocation.isKsp) {
- // ksp replaces these with Any?
- assertThat(field.type.typeName).isEqualTo(TypeName.OBJECT)
- } else {
- assertThat(field.type.typeName).isEqualTo(TypeVariableName.get("T"))
- }
+ assertThat(field.type.typeName).isEqualTo(TypeVariableName.get("T"))
}
subClass.getField("genericProp").let { field ->
diff --git a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XTypeTest.kt b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XTypeTest.kt
index f4cf6f6..8b47c88 100644
--- a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XTypeTest.kt
+++ b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XTypeTest.kt
@@ -600,6 +600,34 @@
}
/**
+ * Reproduces the first bug in b/204415667
+ */
+ @Test
+ fun starVarianceInParameter() {
+ val libSource = Source.kotlin(
+ "lib.kt",
+ """
+ class MyClass<R> {
+ fun setLists(starList: List<*>, rList: List<R>) {}
+ }
+ """.trimIndent()
+ )
+ runProcessorTest(listOf(libSource)) { invocation ->
+ val actual = invocation.processingEnv.requireTypeElement("MyClass")
+ .getDeclaredMethod("setLists").parameters.associate {
+ it.name to it.type.typeName.toString()
+ }
+ assertThat(actual["starList"]).isEqualTo("java.util.List<?>")
+ if (invocation.isKsp) {
+ // TODO b/204415667 resolve variance properly in KSP
+ assertThat(actual["rList"]).isEqualTo("java.util.List<R>")
+ } else {
+ assertThat(actual["rList"]).isEqualTo("java.util.List<? extends R>")
+ }
+ }
+ }
+
+ /**
* Dumps the typename with its bounds in a given depth.
* This makes tests more readable.
*/
diff --git a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/ksp/KspFieldElementTest.kt b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/ksp/KspFieldElementTest.kt
index 95e0d2f..5fcb830 100644
--- a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/ksp/KspFieldElementTest.kt
+++ b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/ksp/KspFieldElementTest.kt
@@ -157,27 +157,12 @@
val base = invocation.processingEnv.requireTypeElement("Base")
val t = base.getField("t")
val listOfR = base.getField("listOfR")
- if (invocation.isKsp) {
- // KSP replaces unspecified type parameters with Any? while javac keeps them as is.
- // This might be an issue when detecting errors but besides that it should be OK.
- // It other words, it shouldn't be an issue in proper code. It will only have an
- // impact when Room wants to report `unbound generics` errors as we won't
- // recognize them and instead assume they were set as Object.
- // see: UNBOUND_GENERICS errors in ProcessorErrors)
- assertThat(t.type.typeName).isEqualTo(TypeName.OBJECT)
- } else {
- assertThat(t.type.typeName).isEqualTo(TypeVariableName.get("T"))
- }
- val typeVariableName = if (invocation.isKsp) {
- "E" // ksp reads from class declaration
- } else {
- "R" // javac reads from variable declaration
- }
+ assertThat(t.type.typeName).isEqualTo(TypeVariableName.get("T"))
assertThat(listOfR.type.typeName)
.isEqualTo(
ParameterizedTypeName.get(
List::class.className(),
- TypeVariableName.get(typeVariableName)
+ TypeVariableName.get("R")
)
)