Get AnnotationByType's invocation handler supports java array value #1330
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Meanwhile, seems test-utils not support java type support, so I am not sure how to make a UT for this |
|
I am not sure if this fix is right. It looks to me that the cause of the bug is that we are expecting an array type in the code which is not the case for single value, therefore we should instead handle single value case. Have you tested your fix since you do not have a test in this PR? Regarding the test though, our test suite should support Java types, there are lots of test cases in our code that targets specifically for Java, can you elaborate your difficulties in writing a test so we can figure it out? |
|
Yes, you can follow the example in this #1329 to reproduce this issue, because for java annotation, we can pass single value as an array to anntation’s value, which will make KSAnnotation argument value return single value for it and ClassCast issue occurs, so it is a workaround for java type. |
|
here is an example that tests annotations declared in Java source. |
|
I got it, I will try to follow this example to add some test case, thanks for your help |
|
@neetopia Thanks for your help, I have added one test case for this situation and it will throw as follow now I have checked that this MR can fix this issue and pass all the test case locally |
|
Looks like there is a lint error. Make sure at least lint passes, you can check by running |
larryxiao625
force-pushed
the
p/xiaozihan.larryxiao/fix_invocation_handler_for_java_type
branch
from
15df9ab
to
ae19967
Compare
|
Thanks for you remind, I have correct all the lint issue and pass all necessary checks locally~ |
neetopia
approved these changes
Mar 3, 2023
|
Thanks for your PR! |
|
This will now actually fail in a case like this (only for mpp projects though, see below): annotation class Anno(val array: Array<String> = [])
@Anno
class Annotated
// processor code:
val clazz: KSClassDeclaration = TODO("get KSClassDeclaration for Annotated")
val array = clazz.getAnnotationsByType(Anno::class)
.first()
.array // fails here
println(array.contentToString())this fails with the following: The reason this fails is this code: // argument.value returns null for default values in mpp
// because of https://github.com/google/ksp/issues/885
// |||
// ||| fall back to jvm reflection
// ||| which returns the array default
// vvv vvv
when (val result = argument.value ?: method.defaultValue) {
is Proxy -> result
is List<*> -> {
// ...
}
// result is now of type Array<String>, so we end up in this else
else -> {
when {
// this is the case, we end up in this branch
method.returnType.isArray -> {
// but now this check returns false, result is of type Array<String>
if (result !is Array<*>) {
// ...
} else {
// now this exception get's thrown, while we should just return result
throw IllegalStateException("unhandled value type, $ExceptionMessage")
}
}
// ... |
|
Let me check this issue~ |
JailedBird
added a commit
to JailedBird/WMRouterKspCompiler
that referenced
this pull request
Feb 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To fix #1329