Skip to content

Add typed annotation values for KSValueArgument - #3117

Open
Yong-yuan-X wants to merge 6 commits into
google:mainfrom
Yong-yuan-X:issue-3008-annotation-value-adt
Open

Yong-yuan-X wants to merge 6 commits into
google:mainfrom
Yong-yuan-X:issue-3008-annotation-value-adt

Conversation

@Yong-yuan-X

@Yong-yuan-X Yong-yuan-X commented Aug 4, 2026 •

Copy link
Copy Markdown
Contributor

Refs #3008

This PR implements the initial KSAnnotationValue and KSPrimitiveType ADT proposed in #3008, with a KSValueArgument.typedValue() extension. It provides a typed view of annotation values without changing the existing KSValueArgument.value API or its Array/Collection behavior.

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:checkApi
  • AAConfiguredUnitTestSuite.testAnnotationArrayValueType
  • PsiConfiguredUnitTestSuite.testAnnotationArrayValueType

@jaschdoc

jaschdoc commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Hey @Yong-yuan-X, are you still interested in contributing this PR?

@Yong-yuan-X

Copy link
Copy Markdown
Contributor Author

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.

@Yong-yuan-X
Yong-yuan-X marked this pull request as ready for review September 14, 2026 06:54

@jaschdoc jaschdoc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for the PR! Looks good but I have a handful of minor changes that'd be great to have before merging it.

Comment thread api/api.base
check(klass?.type?.declaration?.simpleName?.asString() == "String") {
"Unexpected typed class reference: $klass"
}
results.add("KotlinAnnotated TypedAnnotation klass: ReflectionClassReference(String)")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

@Yong-yuan-X Yong-yuan-X Sep 22, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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?

Comment thread kotlin-analysis-api/testData/annotationArrayValueType.kt
@Yong-yuan-X

Copy link
Copy Markdown
Contributor Author

Thanks for the thoughtful and detailed review! I really appreciate the care you’ve put into reviewing this PR.

Updated in cbdc8b2 — reverted the api/api.base changes, moved the typedValue() checks into a separate test processor and fixture, and added the requested documentation.

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.

@Yong-yuan-X
Yong-yuan-X requested a review from jaschdoc October 6, 2026 14:32
Comment thread api/src/main/kotlin/com/google/devtools/ksp/symbol/KSAnnotationValue.kt Outdated
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.

3 participants