Artifact with meta-annotations for Java types enhancement #100
Conversation
dzharkov
changed the title
|
|
||
| ## Summary | ||
|
|
||
| Add a separate artifact containing meta-annotations similar to ones from [JSR-305](https://jcp.org/en/jsr/detail?id=305). |
There was a problem hiding this comment.
I'd say "covering use cases of", not just "similar"
|
|
||
| Their target set would be the following: | ||
| ElementType.METHOD, ElementType.FIELD, | ||
| ElementType.PARAMETER, ElementType.LOCAL_VARIABLE |
There was a problem hiding this comment.
It makes impossible using them with JDK 6
| annotation class MyAllParametersAreNullableByDefault | ||
| ``` | ||
| - Also it might be confusing when one alias (probably with non-empty `propagateTo`) | ||
| references another alias with non-trivial `propagateTo` argument |
There was a problem hiding this comment.
Only if they both are written in Koltin
| Basically, it should work just the same as `@TypeQualifierDefault`, e.g. it might be used like: | ||
| ```kotlin | ||
| @ApplyByDefault(MyNullable::class) | ||
| annotation class MyAllParametersAreNullableByDefault |
There was a problem hiding this comment.
Looks like other annotations should be present as well, for comarison
| [migration status](https://github.com/Kotlin/KEEP/blob/master/proposals/jsr-305-custom-nullability-qualifiers.md#undermigration-annotation). | ||
|
|
||
| ### Default qualifiers options<a name="default-qualifiers"></a> | ||
| This proposal has two options how |
There was a problem hiding this comment.
I think we had three:
- Alias, ApplyByDefault, UnderMigration
- Alias(propagateTo), UnderMigration
- Alias(propagateTo, migrationStatus)
There was a problem hiding this comment.
Your 2d and 3d options are different only by the way how the migration phase is defined.
And this is a different issue described in the next section.
|
|
||
| Probably, the annotations that are already there should be moved to a different packages: | ||
|
|
||
| - `kotlin.annotations.jvm.MigrationStatus` -> `kotlin.annotations.jvm.meta` |
There was a problem hiding this comment.
What about UnderMigration?
| This section describes proposed semantics of the new annotations and partly the layout of resulting artifact. | ||
|
|
||
| ### Root package | ||
| Root package for this artifact will be `kotlin.annotations.jvm`. |
There was a problem hiding this comment.
I am wondering if the scope of such library is not wider than just Kotlin ... For example in Spring, we use JSR 305 meta-annotations to specify Kotlin null-safety, but also for Java documentation and IDE hints purpose. Nothing prevents Java projects to use a library with kotlin.* package, but maybe something more neutral could help to cover the same scope JSR 305 was originally planned for.
There was a problem hiding this comment.
Fair point!
We'll think about it
|
|
||
| ### Details on artifact | ||
|
|
||
| There is already an artifact called [kotlin-annotations-jvm](https://mvnrepository.com/artifact/org.jetbrains.kotlin/kotlin-annotations-jvm) |
There was a problem hiding this comment.
Same open question than for the package, isn't Kotlin scope too narrow for such general nullability need?
|
|
||
| ## Remaining questions | ||
| - Should aliasing JSR-305 qualifiers like `javax.annotation.Nonnull` be allowed? | ||
| - What option is the best to choose for supporting [default qualifiers](#default-qualifiers)? |
There was a problem hiding this comment.
Option 2 looks cleaner to me, but no strong opinion here. Also something to keep in mind is that JVM seems to generates warning when a library (for example Spring) is using enum annotation attributes that are not in the classpath, which was a use case of JSR 305. Maybe this library will be in the projects classpath since it won't have JSR 305 package issue, but I prefer to mention it in order to be sure this is not an issue for projects that will use this library.
There was a problem hiding this comment.
Sorry, but I didn't get the issue with enums and classpath.
Does the warning happen while reading annotations through reflection?
Or it happens when compiling against Spring without JSR-305 in the classpath?
Is it only enum issue, i.e. is everything fine with plain no-arguments annotations?
There was a problem hiding this comment.
What I mean is that the JVM is lenient about meta-annotations not being in the classpath except when these meta-annotations are using enums not in the classpath for their attribute value.
For example if I compile a Spring project without JSR 305 dependency, and if that project is using @org.springframework.lang.NonNull, no warning when compiling because only @Nonnull and @TypeQualifierNickname meta-annotations are used.
But if I use @org.springframework.lang.Nullable, then I get the following warning during compilation: Warning:java: unknown enum constant javax.annotation.meta.When.MAYBE reason: class file for javax.annotation.meta.When not found because this annotation is meta-annotated with @Nonnull(when = When.MAYBE).
This issue very painful for such usual use case would not occur if JSR 305 was using @Nonnull(when = "MAYBE") or @Nonnull @Maybe, so I would be strongly for the new meta-annotation library you will provide avoiding the same mistake as it will allow all libraries to use your meta-annotations without forcing all users of these libraries to have crappy compilation warnings in their logs if they don't have Jetbrains lib in the classpath. IMO this is a key point for wide adoption.
There was a problem hiding this comment.
NB: AFAIK it only happens when using these annotations in a user's project that depends on Spring project but doesn't have jsr305.jar in the classpath. I.e.. when calling a piece of annotated API no warning is emitted.
| - Should there be a `migrationStatus` parameter in `@Alias` or migration status | ||
| should be tracked through `@UnderMigration` annotation? | ||
| - What is the best name for `ApplicabilityKind` enum class? | ||
| Would it be better to place it inside the `ApplyByDefault` annotation class (if it will be there) |
|
Hey, since we are actively working on Spring Framework 5.1 RC1, I would be interested to have a feedback and some visibility about the roadmap of this topic. Thanks! |
|
Hey, unfortunately at the moment I can't tell anything concrete on the roadmap, but will try to figure it out |
| [default qualifiers](https://github.com/Kotlin/KEEP/blob/master/proposals/jsr-305-custom-nullability-qualifiers.md#type-qualifier-default) | ||
| semantics can be introduced. | ||
|
|
||
| And both of them are assume using special enum class instead of `ElementType` |
There was a problem hiding this comment.
both of them are assumeboth of them assume
- Leave the single option for default qualifiers - Get rid of section about avoidance of @UnderMigration - Update "Remaining questions" section
|
@dzharkov are there any news about the roadmap? |
No description provided.