Repository navigation
Add typed annotation values for KSValueArgument - #3117
Yong-yuan-X wants to merge 6 commits into
Conversation
|
Hey @Yong-yuan-X, are you still interested in contributing this PR? |
|
Yes, still interested! Sorry for the delay — I’ve been quite busy with my job search recently. I’ve picked this back up and will continue working on the PR over the next couple of days. |
jaschdoc
left a comment
There was a problem hiding this comment.
Thank you for the PR! Looks good but I have a handful of minor changes that'd be great to have before merging it.
| check(klass?.type?.declaration?.simpleName?.asString() == "String") { | ||
| "Unexpected typed class reference: $klass" | ||
| } | ||
| results.add("KotlinAnnotated TypedAnnotation klass: ReflectionClassReference(String)") |
There was a problem hiding this comment.
Let's add all of this in a new test processor. Then we can discuss what to put in the test processor. Ideally, we should cover every single branch.
There was a problem hiding this comment.
I’ve moved the typedValue() checks into AnnotationArrayTypedValueProcessor with a separate fixture. The current tests cover array-valued arguments with nested annotations, plus String, Int, enum, and class-reference values.
My proposed next step is to extend this processor with tests for the remaining primitive conversions, checking both the wrapper type and the wrapped value.
For branches requiring specific inputs, one option would be to use KSValueArgument instances with controlled value inputs to exercise both Array and Collection paths, unsigned values, and the null/unsupported-value fallback. Would you prefer keeping these controlled-input cases in the processor or moving them into a separate unit test?
|
Thanks for the thoughtful and detailed review! I really appreciate the care you’ve put into reviewing this PR. Updated in The API check and the annotation array tests passed locally across both AA and PSI, with new features enabled and disabled. I’ll follow up in the existing review thread about the remaining test coverage. |
Refs #3008
This PR implements the initial
KSAnnotationValueandKSPrimitiveTypeADT proposed in #3008, with aKSValueArgument.typedValue()extension. It provides a typed view of annotation values without changing the existingKSValueArgument.valueAPI or itsArray/Collectionbehavior.It adds a separate test processor and fixture for typedValue() while keeping the existing annotation array test focused on KSValueArgument.value. The new test covers arrays, nested annotations, a string, an integer, an enum, and a class reference across the AA and PSI test suites.
This follows the proposed starting design. Feedback on the API shape and conversion semantics is welcome.
Tested locally:
:api:checkApiAAConfiguredUnitTestSuite.testAnnotationArrayValueTypePsiConfiguredUnitTestSuite.testAnnotationArrayValueType