Junit5: Reintroduce compile dependency on apiguardian-api in Maven POMs

[sbrannen]

Overview

Consider reverting the changes in #1065.

See discussions on Gitter and elsewhere.

• https://youtrack.jetbrains.com/issue/IDEA-178888
• #1064
• #1065
• #1104
• #1210

Deliverables

• [x] Decide how to handle _optional_ dependencies such as the API Guardian and "nullability" annotations.
• [x] Reintroduce compile dependency on apiguardian-api in generated Maven POMs.
• [x] Document change in release notes and user guide as appropriate.

[marcphilipp]

Team decision:

Try if we can get rid of these warnings by removing the static imports of API.Status enum constants.

[sormuras]

I doubt that will work. What would work is to let status() return a simple String.

[marcphilipp]

I tried removing the static imports but it doesn't help. I see the following possibilities:

1. Make the dependency mandatory again
2. Change @API to use RetentionPolicy.SOURCE and stop using reflection to analyze it but rather some library that parses Java source code and provides access to annotations on the resulting AST
3. Introduce meta-annotations in apiguardian-api, e.g. @Stable(since = "1.0", consumers = "org.junit.*") which would be annotated with @API(status = Stable).
4. Use strings instead of the Status enum

@junit-team/junit-lambda Thoughts?

[luolong]

Not part of JUnit team, but if option 3 solves the problem, then I would vote for that...

[sormuras]

+1 for option 3 as well ... maybe w/o the @API(status = Stable) part, as that may re-introduce an enum-dependency again.

Replace @org.apiguardian.api.API with:

• @org.apiguardian.api.Internal
• @org.apiguardian.api.Deprecated
  ... collides with java.lang.Deprecated when used with an import statement?
• @org.apiguardian.api.Experimental
• @org.apiguardian.api.Maintained
• @org.apiguardian.api.Stable

[sormuras]

Option 4 can be provided along with option 3. Something like:

@org.apiguardian.api.API(status="INTERNAL")

The string values could be mapped to well-known annotation at inspection-time (ignoring characters case) and unknown string values can be collected and reported by an inspection tool. That enables custom categories.

[mkobit]

I like the individual annotations in @org.apiguardian.api.Stable presented above, but that could be because it seems like the approach that most projects take. Some examples being:

• Apache Flink
• Apache Hadoop (I think Yetus came from these)
• Apache Yetus follows a similar style (here and here)
• Gradle @Incubating
• Guava @Beta

[kashike]

Guava made their annotation dependencies non-optional earlier this year - it might be worth looking over what was discussed there, if not done so already.

I, too, like the individual annotation suggestion as mentioned above - the only issue with it is the collision with java.lang.Deprecated.

[marcphilipp]

To fix the collision, we could call them @StableAPI, @InternalAPI, @DeprecatedAPI etc.

[smoyer64]

Is there a reason that the dependency on the API enums can't be made <scope>provided</scope> in the Maven POMs? I'd have to do some digging but I believe I've done that before.

[sbrannen]

To fix the collision, we could call them @StableAPI, @InternalAPI, @DeprecatedAPI etc.

Yes, that would definitely _work_, technically speaking; however, that would require that we release version 2.0 of the API Guardian.

I'm not a fan of doing that, so I'd like to continue to investigate other options before we do anything that drastic.

[sbrannen]

Is there a reason that the dependency on the API enums can't be made <scope>provided</scope> in the Maven POMs?

That's certainly an interesting idea.... and clever if it works. 😉

I'd have to do some digging but I believe I've done that before.

If you could let us know your findings, that would be great!

[smoyer64]

I've created a sample project that scopes apiguardian-api as provided per the discussion above at https://github.com/selesy/apiguardian-provided. This project provides a library that's annotated with apiguardian-api and a consuming application that does NOT include in it's dependencies.

Compile both the annotated library and application using the following command at the top-level directory:

mvn clean install

Switch to the consuming application's directory and view the project's resolved dependency hierarchy using the following command and noting that apiguardian-api is not included:

mvn dependency:resolve

This command shows the following output:

[INFO] Scanning for projects...
[INFO]                                                                         
[INFO] ------------------------------------------------------------------------
[INFO] Building Consuming application 0.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO] 
[INFO] --- maven-dependency-plugin:2.8:resolve (default-cli) @ consuming-application ---
[INFO] 
[INFO] The following files have been resolved:
[INFO]    apiguardian-provided:annotated-library:jar:0.1-SNAPSHOT:compile
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 0.749 s
[INFO] Finished at: 2017-11-14T10:13:56-05:00
[INFO] Final Memory: 14M/298M
[INFO] ------------------------------------------------------------------------

And verify that the consuming application executes properly with the following command:

mvn exec:java -Dexec.mainClass=com.example.ConsumingApplication

This command results in the following output:

[INFO] Scanning for projects...
[INFO]                                                                         
[INFO] ------------------------------------------------------------------------
[INFO] Building Consuming application 0.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO] 
[INFO] --- exec-maven-plugin:1.6.0:java (default-cli) @ consuming-application ---
Just saying hi
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 0.620 s
[INFO] Finished at: 2017-11-14T10:20:30-05:00
[INFO] Final Memory: 11M/298M
[INFO] ------------------------------------------------------------------------

I've also verified that switching the apiguardian-api's scope to compile does indeed add the JAR back into the classpath. Did I miss anything?

[marcphilipp]

Thanks for the experiment, @smoyer64! I'm not an expert on the provided scope. If I've understood it correctly, it will be added to the compilation classpath, but not to the runtime classpath. Is that correct?

[smoyer64]

Yes ... but some further testing (added to the above referenced project) shows that it doesn't work the way I thought it did. Unfortunately, I also can't find that this behavior is part of the specification.

The JVM specification defines a runtimeVisibleAnnotations attribute for each type that's available in the classloader according to the paragraphs at https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7.16. What isn't described is that apparently this list is culled of annotation types that aren't available via the classloader. So if you reflect upon the class using getAnnotations(), the @API annotation isn't in the list. Since this annotation isn't returned, you also don't have access to it's API.Status attribute. To be honest, I thought @API would be in the list but accessing it would cause a ClassNotFoundException (https://docs.oracle.com/javase/8/docs/api/java/lang/ClassNotFoundException.html).

Since the @API annotation isn't on the classpath of the application including an annotated library, reflecting directly upon the annotation using getAnnotation(API.class) causes a compile-time exception. This behavior is defined by the language specification at https://docs.oracle.com/javase/specs/jls/se8/jls8.pdf#%5B%7B%22num%22%3A2532%2C%22gen%22%3A0%7D%2C%7B%22name%22%3A%22XYZ%22%7D%2C72%2C325.736%2Cnull%5D.

Since the specifications state that annotations don't affect the run-time behavior of the program, the behavior described above makes sense, so I feel pretty safe marking apiguardian-api as <scope>provided</scope>. One important note is that tools that want to verify that no unsupported methods are being used in the consuming application project won't find the annotation on any element types. These tools should therefore include apiguardian-api as a compile-scoped dependency.

[sbrannen]

reflecting directly upon the annotation using getAnnotation(API.class) causes a compile-time exception.

That's a different issue.

The builds that publish warnings about the Status enum not being present are in fact not referencing API or API.Status in compiled code. So that's what we are trying to get to the bottom of.

I haven't had a chance to look into your provided work, but at a glance it seems like it could be quite feasible.

Thanks for investigating and sharing your findings!

[smoyer64]

That's a different issue.

Yes ... more proof that provided scope didn't include it on the classpath ;) What I meant is that it's going to be obvious that the annotation and enum are missing if someone is specifically trying to use these types.

[sbrannen]

What I meant is that it's going to be obvious that the annotation and enum are missing if someone is specifically trying to use these types.

👍

[sbrannen]

Unfortunately, I also can't find that this behavior is part of the specification.

AFAIK, provided is a Maven invention that only pertains to dependency management and what is included in the generated artifacts (JAR, WAR, etc.).

Thus, Java doesn't know anything about provided in this context.

[sbrannen]

In other words, javac only cares about what is present on the classpath at compile time, not how it got there. 😉

[marcphilipp]

Does provided also work for Gradle projects?

[sbrannen]

My assumption is that Gradle treats provided like optional and therefore ignores it, but I'd have to investigate to verify that.

[sbrannen]

And when I say "ignores it", I mean in a way that doesn't help us. 😇

[smoyer64]

@sbrannen - That was a very diplomatic way of saying I didn't understand the original problem ;)

[sbrannen]

That was a very diplomatic way of saying I didn't understand the original problem ;)

Ahhhh... well... that wasn't my intention. I was actually just _thinking out loud_ in response to Marc's question.

[smoyer64]

No worries ... one of the comments earlier made me wonder whether the warnings would actually be solved using provided. I'm a bit of a Gradle newb so everything looks like a Maven to me!

[sbrannen]

After further consideration, I believe the best we can do is revert the changes in #1065.

Even if this "bug" (or perhaps just an unintentional inconvenience) in the Java compiler does get addressed, that may never get implemented in Java 8 and may potentially not show up for public consumption until Java 10 or Java 11.

In summary, reverting the changes in #1065 is IMHO the _lesser of the two evils_ (where # 1 is reverting and # 2 is releasing API Guardian 2.0 with breaking changes).

@junit-team/junit-lambda, any objections to reverting the changes in #1065?

[marcphilipp]

No objections from me.

[sbrannen]

No objections from me.

👍

@marcphilipp, this issue is currently assigned to you. Do you want to tackle this one?

[marcphilipp]

I'm on it.