KSType.toTypeName function returns ClassName for alias type instead of ParameterizedTypeName

[Squiry]

Describe the bug
I expect calling KSType.toTypeName() on a type like ArrayList<String> to return ParameterizedTypeName with rawType=kotlin.collections.List and typeArguments=[kotlin.String], but I get just raw ClassName(kotlin.collections.List) instead.

To Reproduce
Just change this line to val parameterType = parameter.type.resolve().toTypeName(functionTypeParams).let { and smokeTest will fail with something like this:

value of:
    readText$default(...)
diff (-expected +actual):
    @@ -103,7 +103,7 @@
         nullableSetListMapArrayNullableIntWithDefault: Set<List<Map<String, Array<IntArray?>>>>?,
         aliasedName: TypeAliasName,
         genericAlias: GenericTypeAlias,
    -    parameterizedTypeAlias: ParameterizedTypeAlias<String>,
    +    parameterizedTypeAlias: ParameterizedTypeAlias,
         nestedArray: Array<Map<String, Any>>?,
       ): Unit {
       }
	at com.squareup.kotlinpoet.ksp.test.processor.TestProcessorTest.smokeTest(TestProcessorTest.kt:146)

Additional context
Right now on a master branch KSType.toTypeName return non parameterized ClassName even for non aliased types and reproducer from above will break the with much more errors. This behaviour was changed in a #1321 and I don't know if this was intended.

[ansman]

This happens to me too, but even without type aliases.

Given this file:

interface Repository<T>
class RealRepository @Inject constructor() : Repository<String>

And you get the supertypes for RealRepository and call toTypeName you get a ClassName back instead of a ParameterizedTypeName.

[Squiry]

Yeah, I haven't noticed that pull request I've mentioned was already released in 1.13.0.

[xiaojinzi123]

KSType: Single
KSDeclaration: Single
arguments: [String]
decl.classKind == ClassKind.ANNOTATION_CLASS = false

so, parameter arguments will be 'typeArguments' instead of 'KSType.arguments',

[ZacSweers]

PRs are welcome if someone wants to take a look at one. I've been traveling the past week for kotlinconf and have a busy work week so not sure when I'll have time

[ZacSweers]

@ansman I'm unable to reproduce your example with this test and modifying TestProcessor to add superinterfaces/superclass to the output file

@Test
fun regression_1513() {
  val compilation = prepareCompilation(
    kotlin(
      "Example.kt",
      """
         package test

         import com.squareup.kotlinpoet.ksp.test.processor.ExampleAnnotation

         interface Repository<T>
         @ExampleAnnotation
         class RealRepository @Inject constructor() : Repository<String>
         """,
    ),
  )

  val result = compilation.compile()
  assertThat(result.exitCode).isEqualTo(KotlinCompilation.ExitCode.OK)
  val generatedFileText = File(compilation.kspSourcesDir, "kotlin/test/TestRealRepository.kt")
    .readText()

  assertThat(generatedFileText).isEqualTo(
    """
      package test

      import kotlin.String

      public class RealRepository : Repository<String>

    """.trimIndent(),
  )
}

[ZacSweers]

@Squiry your example is not very helpful as it is intentionally calling a different API and excluding type arguments. It would fail even if we revert the changes in the PR you mention.

[Squiry]

intentionally calling a different API and excluding type arguments

Is there any other API for converting KSType to TypeName? All the type arguments are there, in KSType.

It would fail even if we revert the changes in the PR you mention.

This bug is not about mentioned PR, I've just noted that after that PR test fails even on non alias types.

[Squiry]

The problem is still there, KSType.toTypeName() still doesn't work for aliases.

[Egorand]

@Squiry:

1. Which version have you retested this on?
2. Can you please provide a reproducible test case? @ZacSweers added a number of regression tests for this issue that currently pass. If your use case is different - please provide more details so we could address it.

[Squiry]

You have fixed the regression introduced in #1321, but my problem was on the version before it, I've just mentioned that #1321 have broken method even more. Reproducer I'm giving is still valid, I've checked it on master.

Also you can notice, that KSType.toTypeName has no usage in test processor at all, you only test KSTypeReference.toTypeName that works correctly.

[Squiry]

The problem is that KSType.toTypeName passes empty list of args to internal implementation, that list is mapped and passed to withTypeArguments call that makes ClassName from any type. KSTypeReference.toTypeName passes typeArguments, so there's no such a problem with that. Looks like changing KSType.toTypeName like that should fix the problem:

public fun KSType.toTypeName(
  typeParamResolver: TypeParameterResolver = TypeParameterResolver.EMPTY,
): TypeName = toTypeName(typeParamResolver, arguments)

[PaulWoitaschek]

If you have a fix, can you submit a fix + tests as a PR?

[ansman]

Sorry for the late reply, but here is a repro. Given the code that @ZacSweers already posted, I can repro the issue with this code:

// Add interfaces
decl.superTypes
  .map { it.resolve() }
  .filter { (it.declaration as KSClassDeclaration).classKind == ClassKind.INTERFACE }
  .map { it.toTypeName() }
  .forEach { classBuilder.addSuperinterface(it) }

If I instead use the overload that uses a KSTypeReference it works just fine:

// Add interfaces
decl.superTypes
  .filter { (it.resolve().declaration as KSClassDeclaration).classKind == ClassKind.INTERFACE }
  .map { it.toTypeName() }
  .forEach { classBuilder.addSuperinterface(it) }

I believe the issue here is that KSType.toTypeName hard codes an empty list of type arguments:

public fun KSType.toTypeName(
  typeParamResolver: TypeParameterResolver = TypeParameterResolver.EMPTY,
): TypeName = toTypeName(typeParamResolver, emptyList())

[ansman]

Changing it to this, fixes the issue and all tests pass:

public fun KSType.toTypeName(
  typeParamResolver: TypeParameterResolver = TypeParameterResolver.EMPTY,
): TypeName = toTypeName(typeParamResolver, arguments)

[ZacSweers]

This should be fixed already on the main branch if you could try snapshots and confirm

[ansman]

Confirmed!

[Squiry]

I only pray for we haven't break some hidden implementation detail that relied on that emptyList behaviour. Other than that waiting for release!

[Egorand]

Yep, planning to publish another patch release this week! I'm in the process of setting up auto-releases from CI and trying to improve our workflow configuration, here's the latest attempt: #1537. Appreciate any 👀 !

[Egorand]

1.13.2 is up!