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

TypeArguments: eliminate StackOverflow while retaining accurate types #340

Merged
merged 3 commits into from
Sep 26, 2022

Conversation

xRodney
Copy link

@xRodney xRodney commented Sep 12, 2022

Fixes #333.

While the original fix replaced the recursive mapping V -> List<V> with V -> List<?>, here I take a more subtle approach. The mapping is replaced with V -> List<V>[replaced=true], where he [replaced=true] is an attribute. The visitor never replaces class refs with this attribute and while doing so also removes it, so that the resulting type tree is clean.

Because the goal of this is to be usable in crd-generator, I also added a new convenience method RichTypeDef apply(ClassRef) (I actually renamed a pre-existing private method with the same name to make room for it). This can process the types all the way from a generic reference (as opposed to from a definition)

Related: fabric8io/kubernetes-client#4357

@xRodney xRodney marked this pull request as ready for review September 12, 2022 07:38
@iocanel iocanel self-requested a review September 12, 2022 18:04
Copy link
Collaborator

@iocanel iocanel left a comment

Choose a reason for hiding this comment

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

lgtm

@xRodney
Copy link
Author

xRodney commented Sep 13, 2022

Hello @iocanel and thanks a lot for your review. Would you please mind merging and releasing it?

@xRodney
Copy link
Author

xRodney commented Sep 21, 2022

Hello again @iocanel,
I realize you might be busy, but is there a chance you could have another look on this? Or at least a rough estimate when you'd be less busy? We are blocked by fabric8io/kubernetes-client#4357.
Thanks a lot.

@iocanel iocanel merged commit 0aeaae6 into sundrio:main Sep 26, 2022
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.

StackOverflowError on TypeArguments.apply(typeDef) for MultiMaps & other non-trivial generics
2 participants