Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KSType.toTypeName incorrectly resolves typealias type argument #1304

Closed
micHar opened this issue Jul 1, 2022 · 4 comments · Fixed by #1321
Closed

KSType.toTypeName incorrectly resolves typealias type argument #1304

micHar opened this issue Jul 1, 2022 · 4 comments · Fixed by #1321
Labels

Comments

@micHar
Copy link

micHar commented Jul 1, 2022

For typealias LeAlias = Map<Int, String>, if I use KSTypeReference.toTypeName I get excessively parameterized typealias if the typealias itself is a type param. Complicated 😅, so here's an example:

original KSTypeReference -> TypeName
---
LeAlias -> LeAlias (OK)
Flow<LeAlias> -> Flow<LeAlias<Int, String>> (wrong, unexpected type params for LeAlias)

kotlinpoet version: 1.12.0

@micHar
Copy link
Author

micHar commented Jul 1, 2022

When LeAlias is directly provided to KSTypeReference.toTypeName, no type arguments are passed to KSType.toTypeName.

However, when Flow type is encountered, we fall into this line. Then KSTypeReference.toTypeName is called internally from here, and the Int, String type params are passed to KSType.toTypeName, even though it's still the same LeAlias.

Zrzut ekranu 2022-07-1 o 14 26 22

@micHar
Copy link
Author

micHar commented Jun 26, 2023

This still doesn't work in my case, although I can see that it's supposed to be fixed and there are even tests for it (the ones that I proposed above, btw).

I used 1.14.2 and 1.15.0-SNAPSHOT.
I matched the kotlin and ksp version with the main branch.

kotlin = "1.8.22
ksp = "1.8.22-1.0.11"

However I still get this:

List<Alias997<Int, String>>

from this:

typealias Alias997 = Map<Int, String>
List<Alias997>

I narrowed it down to the fact, that in internal fun KSType.toTypeName

    is KSClassDeclaration -> {
      decl.toClassName().withTypeArguments(arguments.map { it.toTypeName(typeParamResolver) })
    }

the arguments in my case is Map<Int, String> and in the kotlinpoet tests it's Alias997. arguments is part of KSP API, but I cannot understand why in my case it's a resolved type and in your case still the original typealias.

The resolution happens in KSTypeImpl (and then in KSTypeReferenceImpl)

    // TODO: fix calls to getKSTypeCached and use ksTypeArguments when available.
    override val arguments: List<KSTypeArgument> by lazy {
        (kotlinType.getAbbreviation() ?: kotlinType).arguments.map {
            KSTypeArgumentDescriptorImpl.getCached(it, Origin.SYNTHETIC, null)
        }
    }

@ZacSweers @Egorand do you have any idea what might be the cause? I would be happy to provide a fix and a test case, but I can't quite find the issue that would cause the cache to get the wrong type argument.

@Egorand
Copy link
Collaborator

Egorand commented Jul 4, 2023

Not really sure what might be causing this... Does seem like a potential KSP issue? It might be interesting to isolate a test case that doesn't involve KotlinPoet code and file a KSP issue.

@micHar
Copy link
Author

micHar commented Jul 4, 2023

I tried to do that but failed. Thought that you guys might understand the ksp internals a bit better than me. I have this test case in an open source repo, though. It's not pushed to github yet, but I could push it. Question is if you are interested in compiling a separate project to look into it, I understand you might not have time for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants