Ensure 1.0 does not preclude supporting both retention levels well later

[overheadhunter]

Hi all,

as @cpovirk asked for feedback from library vendors in the 0.2.0 release notes, I'd like to share my view on the retention topic discussed in #28. For the sake of reflection (e.g. Spring was mentioned), it was decided to go with runtime retention.

At the same time, @kevinb9n concluded:

If the need for an altered form of the core artifact with the retention reduced (or tool that does that munging in a build) becomes compelling it can be filed separately.

For my use case having runtime retention is not a blocker but still unfortunate. Let me explain:

From my understanding the whole purpose this lib was to support static code analysis and IDEs, not to add yet another set of annotations to be used by DI framworks. I was hoping for a lib that allows library vendors to communicate a method's contract in machine-readable form without imposing further runtime dependencies upon downstream users. Right now, org.jetbrains.annotations is the only option fulfilling this goal, that I'm aware of.

You will probably argue that jspecify is extremely small, but it adds up over time. You can see the mess in Guava, where we get JSR-305, checkerframework and errorprone for free. Now let me ask you: While IDE support for Guava's methods is great (and it would still work the same with class retention), how many of you have ever reflected on Guava's Lists.newArrayList(...)?

Whether reflection is required or not basically depends on who is supposed to be the target audience. As mentioned above, frameworks like Spring were decided to be in scope (which will due to compatibility concerns probably lead to adding yet another alternative dependency instead of replacing existing options - sadly your docs strictly forbid sending you a certain XKCD comic 😉 - but that's a different topic).

So, as suggested by @kevinb9n, may I indeed ask for any plans to have a set of annotations with class retention for the folks who just want to better check correctness of code? Maybe you can offer two flavours of this lib?

[kevinb9n]

Thanks for filing this!

Currently #28 represents a "working decision" to use all runtime retention. We still have time to reverse that if needed, but there's something more important to make clear first.

No matter which retention we use, we need to make sure that nothing we do in 1.0 closes the door permanently on the use cases for the opposite kind (i.e. renders them unable to ever use JSpecify annotations, or always miserable when they do). If choices we make "paint us into a corner", I'd see that as a serious release-blocking bug.

We can use this issue here to try to think through any risks like that we can see.

If, in the end, the only way we privileged one group is by releasing their ideal artifact a little sooner than the other, that sounds like a fine outcome.

[kevinb9n]

From my understanding the whole purpose this lib was to support static code analysis and IDEs, not to add yet another set of annotations to be used by DI framworks.

That is the motivation to introduce a library of nullness annotations, but if we do this without succesfully obsoleting all the existing nullness annotations, including those sometimes used at runtime, I think people will not be very happy and they will send us web comics.

I was hoping for a lib that allows library vendors to communicate a method's contract in machine-readable form without imposing further runtime dependencies upon downstream users.

What I've gathered is that each time we dig into whether this really does impose downstream dependencies the answer has ended up being no.

The interpretation I've cobbled together, which might be wrong, is this:

As far as I can tell, no matter which retention (class or runtime) we use:

• The references TO our annotation types in your compiled class files look very nearly the same.
• Compiling another library against your jar should not require our jar to also be present.
• Using your jar at runtime should not require our jar to be present either. Even if iterating all annotations found on an element, it should simply skip ones whose declarations aren't present.

This would suggest that the only difference class retention makes is when the jar is present at runtime, and someone is actively querying for our annotation types, and they would have gotten positive results. Then class retention makes it simply refuse to acknowledge that the annotations are there. I can't think of a reason to do that unless we want to make a strong statement that the annotation types make no sense at runtime, which we don't.

You will probably argue that jspecify is extremely small, but it adds up over time.

I don't think this is true in any meaningful, appreciable sense. What's adding up to problems are the large jars full of implementation code. We are a drop in the bucket. However...

You can see the mess in Guava, where we get JSR-305, checkerframework and errorprone for free.

You are definitely not getting "checkerframework" or "errorprone" which are large artifacts full of implementation code that Guava does not depend on at all. You must mean their respective annotation jars. First, our work will make those three go away and be replaced by one. Second, again, as far as I can tell, if you're getting those in your own class path "for free" when you don't want them, something must be configured wrong on one end or the other. Sorry, my teammates who are home for the weekend have better details on this than I do.

Whether reflection is required or not basically depends on who is supposed to be the target audience.

No, this is a mischaracterization of the decision. It's not a "target audience" question. We are not aware of a reason to restrict the retention of the annotations at all, that holds up under scrutiny. The kinds of problems it causes turn out to be tool bugs and tool misconfigurations. Meanwhile, if we locked the retention down to "class", we wouldn't be "causing problems" for certain tools, we'd be making their use cases utterly impossible. That is quite lopsided.

Having said all that,

This issue is still about making sure that we don't preclude a possibility of supporting both retention levels if necessary. At this point, I'm not aware of any concrete reason to think this might happen, so I'm leaning toward closing the bug for lack of evidence of concern.

[overheadhunter]

• Compiling another library against your jar should not require our jar to also be present.
• Using your jar at runtime should not require our jar to be present either. Even if iterating all annotations found on an element, it should simply skip ones whose declarations aren't present.

If this is the case, I have indeed no concerns. I just wasn't aware it is possible to load a class when imports (edit: annotations) are missing.

[overheadhunter]

At this point, I'm not aware of any concrete reason to think this might happen, so I'm leaning toward closing the bug for lack of evidence of concern.

I did a little more research and feel the urge to point you to these discussions:

• Reintroduce compile dependency on apiguardian-api in Maven POMs junit-team/junit5#1105
• Unnecessary-looking dependencies google/guava#2824
• Many warning messages related to errorprone "CompatibleWith" google/guava#2721
• NoClassDefFoundError in IntelliJ because AutoService has class retention google/auto#1032

TLDR: Users of downstream libraries repeatedly report issues when annotations with runtime retention are omitted from dependencies. In many cases the behaviour can be linked to deficiencies in javac or Maven, nevertheless we have to deal with it.

With that in mind, are you sure, you'll be able to deliver the goals set in your aforementioned three bullet points?

[kevinb9n]

The sentence you quoted had more context:

This issue is still about making sure that we don't preclude a possibility of supporting both retention levels if necessary. At this point, I'm not aware of any concrete reason to think this might happen, so I'm leaning toward closing the bug for lack of evidence of concern.

I've made this issue about that question. If there's a reason to think we might be prevented from adding that artifact post-release, hopefully we'll find it soon. (@sbrannen and @cpovirk, either of you knows more about this problem than ten of me.)

The question of whether the very first artifact should be class-retention is still #28 and that can still be reopened if really necessary.

[kevinb9n]

An example of what could cause trouble: if, with two parallel artifacts, we would feel strongly that they should have had parallel naming and structure, without either looking like the "primary" one and the other the "altered" one, then we might really regret having already made a release that hogged the "primary" position. But it's just not looking that way to me. It makes sense to me that the unrestricted one would be the main one forevermore.

[cpovirk]

Sorry, I've been busy unnecessarily confusing myself with other things.... So maybe it's just as well that I didn't try to tackle this also confusing topic during that time :)

Here's my attempt at a summary response:

• I recommend including any annotation artifacts that your dependencies use on your own classpath, but you can often get away without it if you prefer.
• That is my recommendation for both runtime-retention and class-retention annotations: I'm not personally specifically aware of any cases in which incomplete classpaths would be safer with class-retention annotations than with runtime-retention annotations—at least in the case of most annotations, including JSpecify annotations.
• Unrelated to incomplete classpaths, there are still some edge cases in which class retention can offer advantages.
• I hope that we can address those edge cases by helping to improve tools, offering official class-retention annotations, and/or letting users define their own class-retention aliases for our annotations.

I'll try to flesh all that out, maybe as soon as today (but don't hold your breath :)). But let me know if there's something else that I should be responding to or if I'm otherwise missing the point.

[cpovirk]

• I recommend including any annotation artifacts that your dependencies use on your own classpath, but you can often get away without it if you prefer.

My attempt to summarize google/guava#2824 is https://github.com/google/guava/wiki/UseGuavaInYourBuild#what-about-guavas-own-dependencies. It's mostly a link dump, but it may be useful for people who are interested in what kinds of problems an incomplete classpath can produce.

[edit: And see also the point about @Implies at the end of this post.]

• That is my recommendation for both runtime-retention and class-retention annotations: I'm not personally specifically aware of any cases in which incomplete classpaths would be safer with class-retention annotations than with runtime-retention annotations—at least in the case of most annotations, including JSpecify annotations.

For starters: It makes sense that compile-time checks wouldn't care which retention the annotations have.

The more interesting case is, of course, runtime. Now as Kevin said, the difference is theoretically just that you can't see class-retention annotations and can see runtime-retention annotations, provided that they're available on the classpath.

In practice, apparently there are cases in which omitting one annotation from the classpath can cause problems for reflection on other annotations. But my impression my some limited testing is that that's a problem only when an annotation defines an enum element, which we are unlikely to do for various reasons. (In particular, though the linked JDK bug says plenty about "classes," I don't think that an annotation with a Class element (as @Implies would do) leads to problems.)

• Unrelated to incomplete classpaths, there are still some edge cases in which class retention can offer advantages.

The two that have remained on my mind:

• Runtime-retention annotations appear in Android dex files, increasing code size. However, our @Nullable is a type-use annotation, which is not included in dex files. So the main fear for Android would be heavy use of the declaration annotation @NullMarked. It would be interesting to have real-world numbers for its impact on binary size.
• Android has some particular problem with trying to put annotated classes into the main dex. But it appears to be rare, may be fixable, and may become a non-issue once native multidex is near-universal.

I hope that we can address those edge cases by helping to improve tools, offering official class-retention annotations, and/or letting users define their own class-retention aliases for our annotations.

• Improving tools could involve fixing the Android main-dex problem I mentioned. It could also include offering tools to remove runtime-retention annotations from bytecode. (One advantage of that approach is that it also helps non-Android users who are concerned about the bytecode size of type-use @Nullable annotations—though perhaps tools like Proguard cover this well already.)
• By "offering official class-retention annotations," I mean something like org.jspecify.nullness.classretention.Nullable (name and package TBD!), which I believe is in line with what kevinb9n has been describing on this thread.
• "Letting users define their own class-retention aliases for our annotations" is another reference to providing an @Implies annotation: That would let users define their own @Nullable annotation and tell tools to treat it equivalently to ours. (That approach, coincidentally, suggests another reason to keep all dependencies on your classpath: If any annotation you see might be an alias for another annotation, then you need to be able to read the annotation's .class file in order to find out!)

[kevinb9n]

Thanks for all the information.

I don't think we have seen a compelling reason to worry that releasing a runtime-retention artifact will preclude future solutions to these kinds of problems. At the worst I think we just end up with a second artifact too. This issue can be reopened if things start to look worse.