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

Correct generation of typealiases with type args #1162

Merged
merged 2 commits into from Sep 21, 2021

Conversation

evant
Copy link
Contributor

@evant evant commented Sep 20, 2021

If you had typealias Foo = List<String> then toTypeName() would
incorrectly generate Foo<String> instead of Foo. This is because
KSType.arguments, returns the arguments of the underlying type, not
the alias. In order to get the alias args you need to use
KSTypeReference.element.typeArguments instead.

If you had `typealias Foo = List<String>` then `toTypeName()` would
incorrectly generate `Foo<String>` instead of `Foo`. This is because
`KSType.arguments`, returns the arguments of the underlying type, _not_
the alias. In order to get the alias args you need to use
`KSTypeReference.element.typeArguments` instead.
@@ -53,7 +53,8 @@ public fun KSType.toClassName(): ClassName {
*/
@KotlinPoetKspPreview
public fun KSType.toTypeName(
typeParamResolver: TypeParameterResolver = TypeParameterResolver.EMPTY
typeParamResolver: TypeParameterResolver = TypeParameterResolver.EMPTY,
typeArguments: List<KSTypeArgument> = emptyList(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how to handle this api-wise. I just added it as an extra argument to get this info but there may be a better way.

Copy link
Collaborator

@ZacSweers ZacSweers Sep 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep the original public signature but then move the full impl with this overload to an internal separate function. We can open it up later if there's a need for it

Copy link
Collaborator

@ZacSweers ZacSweers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, I just ran into the same issue in testing this out in another repo

@@ -53,7 +53,8 @@ public fun KSType.toClassName(): ClassName {
*/
@KotlinPoetKspPreview
public fun KSType.toTypeName(
typeParamResolver: TypeParameterResolver = TypeParameterResolver.EMPTY
typeParamResolver: TypeParameterResolver = TypeParameterResolver.EMPTY,
typeArguments: List<KSTypeArgument> = emptyList(),
Copy link
Collaborator

@ZacSweers ZacSweers Sep 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep the original public signature but then move the full impl with this overload to an internal separate function. We can open it up later if there's a need for it

@ZacSweers ZacSweers merged commit 9d378ff into square:master Sep 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants