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

Don't crash if the root of a trace is a TypeParameter #427

Merged
merged 3 commits into from
Sep 10, 2024

Conversation

eygraber
Copy link
Contributor

Fixes #426

I'm not sure if this is the best solution, or why the type parameter is included in the CycleDetector. An alternate option is to catch this in the CycleDetector and just filter it out. Another option is to catch any exceptions from toTypeName() and use typeRef.toString() instead.

@eygraber
Copy link
Contributor Author

eygraber commented Sep 2, 2024

@evant this is making debugging pretty difficult in one of my projects. Any chance it can get merged in soon?

@evant evant force-pushed the fix-trace-root-type-parameter branch from 6ea5548 to e72dd9b Compare September 10, 2024 21:30
Comment on lines 534 to 535
} else if (type.declaration is KSTypeParameter) {
typeRef.toString()
Copy link
Owner

@evant evant Sep 10, 2024

Choose a reason for hiding this comment

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

This won't completely fix this as a type param can be arbitrarily nested, for example changing your test from

val c: C

to

val c: () -> C

will cause the same exception.

Copy link
Owner

@evant evant Sep 10, 2024

Choose a reason for hiding this comment

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

maybe we could wrap toTypeName() in a try/catch and fallback to typeRef.toString() for now?

As an aside, I really wish ksp's toString() properly showed type params, so we wouldn't need to rely on kotlinpoet at all. I submitted a pr to fix this but it's been ignored 😐 google/ksp#1737

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the test to reflect that and changed to try catch.

@evant
Copy link
Owner

evant commented Sep 10, 2024

detekt errors, ./gradlew detekt --auto-correct should fix

@eygraber
Copy link
Contributor Author

Done

@evant evant merged commit a0e077c into evant:main Sep 10, 2024
2 checks passed
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.

NoSuchElementException when tracing a missing dependency and the root is a type parameter
2 participants