https://kotlinlang.org logo
Join Slack
Powered by
I wrote some code to implement the `getAnnotation`...
# ksp
a
I wrote some code to implement the 
getAnnotation
in a separate repo. I saw that the implementation is pending for KSP and would like to know if I could help contribute there. If so I have a couple of questions. Which elements should we apply the functions to? I was thinking the KSAnnotated seems to make sense. ie 
KSAnnotated#getAnnotation
I had also thought of taking a different approach and leveraging the 
Method#getAnnotation
method by just getting a handle to the 
Method
itself from the 
KSAnnotated
, but I hadn’t done any further research into that yet. What approaches were you thinking about taking? And does my current implementation in my repo seem reasonable or is there a better approach? I had thought about just creating a PR, but I would rather get some feedback first before I invest the time. Thanks
y
that looks similar to what we have in room. https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:room/roo[…]notationBox&sq=&ss=androidx%2Fplatform%2Fframeworks%2Fsupport Though we had it before KSP for java as well. I don't fully remember why but my guess is thee types where they might not be accessible when the targeted class is not in the processor classpath. https://github.com/AlexBeggs/ksp-annotation-parser/blob/main/src/main/kotlin/io/github/alexbeggs/kspannotationparser/AnnotationParser.kt#L60
a
any thoughts about integrating this into the KSP repo?
y
i personally think it is a useful addition. that being said, due to beta stage, it might be a bit late to add.
a
I'll create a PR and maybe it can go into a later release
I'm sure there will be back and forth anyway
t
Thanks, a PR would be highly appreciated. In terms of API freeze, it should be fine since it is an extension function in utils and won't break anything. We need to review with extra care though, since it can't be removed or changed once it gets in.
a
sure understand. Adding it to the 
KSAnnotated
class as an extension function is fine?
or is there a better location for this?
t
KSAnnotated
sounds natural to me. May I know what places else you are thinking about?
a
that seemed the most logical for me. I also was considering implementing the other methods 
<https://docs.oracle.com/javase/8/docs/api/java/lang/reflect/AnnotatedElement.html>
for consistency
t
The 
getDeclaredAnnotation
family might not be needed, as in KSP annotations are not inheritable.
@Jiaxiang please correct me if I remember it wrong.
j
yes we don’t do inherited annotations
a
so even using the 
<https://docs.oracle.com/javase/8/docs/api/java/lang/reflect/Method.html#getDeclaredAnnotations-->
that wouldn’t work properly?
j
The PR is not extension function yet, are you planning to change it to be extension functions?
a
what would be the outcome? (I haven’t tried it)
j
the docu says is it 
Returns annotations that are _directly present_ on this element. This method ignores inherited annotations.
which I think matches what we do in KSP?
a
oh right, I meant the other (swapped them)
@Jiaxiang I haven’t written the PR just yet, but I was going to convert this into a PR and clean it up some more. Including adding tests in your repos format. https://github.com/AlexBeggs/ksp-annotation-parser/blob/main/src/main/kotlin/io/github/alexbeggs/kspannotationparser/AnnotationParser.kt#L11
but will relocate it to KSAnnotated
j
oh yes, I mistake another PR to be yours
a
np
j
what you mean by swapped them?
a
I believe the getAnnotations gives you inherited (no mention of it so I assume because of exclusion it is providing inherited)
j
I see, yes there is currently no support for that, it is doable via an extension method as a utility function.
a
okay I will add that in (unless I run into a problem) - assuming the Method#getAnnotations and Method#getDeclaredAnnotations work as expected
j
👍 thanks
t
Sorry for the confusing. I mean, because 
KSAnnotated.annotations
doesn't include inherited annotations, only declared annotations are available in KSP. And we don't need to name it 
declaredAnntation
because it is the only / default behavior.
a
I see
thanks for the explanation
so then I will not include the 
declared
family of methods as was stated before
t
Sounds good, thanks.
a
I have a PR https://github.com/google/ksp/pull/508 ready for review One caveat, there aren’t any tests for Java annotations. I have Kotlin annotations covered, just wasn’t able to get Java annotations working.
22 Views
Open in Slack
PreviousNext
Powered by