Skip to content

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

Merged
evant merged 3 commits into
evant:mainfrom
eygraber:fix-trace-root-type-parameter
Sep 10, 2024
Merged

Don't crash if the root of a trace is a TypeParameter#427
evant merged 3 commits into
evant:mainfrom
eygraber:fix-trace-root-type-parameter

Conversation

@eygraber

Copy link
Copy Markdown
Collaborator

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

eygraber commented Sep 2, 2024

Copy link
Copy Markdown
Collaborator Author

@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()

@evant evant Sep 10, 2024

Copy link
Copy Markdown
Owner

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.

@evant evant Sep 10, 2024

Copy link
Copy Markdown
Owner

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
Copy Markdown
Collaborator 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

evant commented Sep 10, 2024

Copy link
Copy Markdown
Owner

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

@eygraber

Copy link
Copy Markdown
Collaborator Author

Done

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