Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support for repeatable annotations #358

Open
yigit opened this issue Mar 18, 2021 · 1 comment
Open

support for repeatable annotations #358

yigit opened this issue Mar 18, 2021 · 1 comment

Comments

@yigit
Copy link
Collaborator

yigit commented Mar 18, 2021

Since Java 8, there is way to repeat annotations by providing a container class:

e.g.

@Repeatable(RepeatableJavaAnnotation.List.class)
@Retention(RetentionPolicy.RUNTIME)
public @interface RepeatableJavaAnnotation {
    String value();
    @Retention(RetentionPolicy.RUNTIME)
    @interface List {
        RepeatableJavaAnnotation[] value();
    }
}
@RepeatableJavaAnnotation("x")
@RepeatableJavaAnnotation("y")
@RepeatableJavaAnnotation("z")
public class JavaSubject {}

When running with Java AP, you can get this list of values by querying with RepeatableJavaAnnotation.List.
That does not seem to be the case with KSP.

javap -v JavaSubject.class

SourceFile: "JavaSubject.java"
RuntimeVisibleAnnotations:
  0: #14(#15=[@#16(#15=s#17),@#16(#15=s#18),@#16(#15=s#19)])
    androidx.room.compiler.processing.testcode.RepeatableJavaAnnotation$List(
      value=[@androidx.room.compiler.processing.testcode.RepeatableJavaAnnotation(
        value="x"
      ),@androidx.room.compiler.processing.testcode.RepeatableJavaAnnotation(
        value="y"
      ),@androidx.room.compiler.processing.testcode.RepeatableJavaAnnotation(
        value="z"
      )]
    )

Normally, this could be considered OK assuming they are available in the annotations list (haven't verified but this is my guess). But when code is compiled, they do get moved into the List wrapper annotation above, which would make descriptor implementations inconsistent with declaration implementations (also possibly with PSI, haven't verified).

Btw, this might also have different behaviors between java target versions. I'm also hitting this issue even after i set target to 1.8. Will comment here once i find more information on why it is still happening.

@yigit
Copy link
Collaborator Author

yigit commented Mar 19, 2021

I wanted to prototype this but it is not trivial at all.

java repeatable annotation is declared as:

public @interface Repeatable {
    Class<? extends Annotation> value();
}

whereas kotlin repeatable is:

@Target(AnnotationTarget.ANNOTATION_CLASS)
public annotation class MustBeDocumented

notice that the java api requires you to specify a container class whereas the kotlin one does not.

Now the issue is that, when java compiles a repeatable annotation; they actually wrap it in the container.
Kotlin does not even support non-source retention repeatable annotations for java (didn't try w/ kotlin annotation yet, i assume it won't).

In KSP right now, if you are reading from a origin=class, you need to use the container whereas if you are reading from origin java, kotlin, you need to use the repeated annotation.

I wanted to send a PR where KSP does the wrapping as well to be consistent but I cant because i cannot read the target out of Repeatable (it does not have the container value :/ )

so tl;dr; i'm not even sure how we can fix this in KSP :'(
We could consider unwrapping the annotations in .class files but we cannot do that because there is no reference from the container annotation to the repeated annotation.

copybara-service bot pushed a commit to androidx/androidx that referenced this issue Mar 24, 2021
Repeatable annotation in Java AP work via a container class which we can
use to get all of the annotations.

Unfortunately, KSP does not yet support it where:
* Each annotation shows up as an element in the annotations list of
KSAnntated if it's origin is source and the wrapper annotation shows up if
origin is .class.

To avoid inconsistencies in the XProcessing APIs, I've added a
`getAnnotations(KClass<T:Annotation>)` API which will return all
repeated annotations. It automatically checks if the given annotation is
repeated and if so, uses the container class to query.

I've also renamed `toAnnotationBox` to `getAnnotation` but kept the old
one deprecated for easy migration for XProcessing clients.

This is still not perfect due to two issues in KSP and 1 issue in
Kotlin:
https://youtrack.jetbrains.com/issue/KT-12794
google/ksp#356
google/ksp#358

tl;dr; Kotlin does not support Repeatable annotations unless it has
SOURCE retention.
Moreover, due to #356, we cannot support these in .class files. It
should automatically be fixed in XProcessing when KSP side is fixed
(though it seems to be a kotlin compiler issue hence may take a while to
fix in KSP).

Bug: 160322705
Test: XAnnotationBoxTest

Change-Id: Ib1fb97861ab3a01060ca8245107698a4d46d67cf
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

No branches or pull requests

1 participant