bbottema/simple-java-mail

Move away from Findbugs (unofficial JSR-305) annotations

bbottema opened this issue · 10 comments

bbottema commented

The widely used but aging Findbugs annotations are not compatible with Java 9 due to split package (Findbugs choose to use the same package as an existing package in the JDK). The Findbug annotations were never made official and as the RFC died (dormant), there's no hope of fixing this with JSR-305 (which never defined annotations).

The solution is to move to an alternative, which begs the question: which alternative is still maintained, compatible with Java 9 and also has runtime retention, actually has all the annotations currently used, doesn't bring transitive baggage and is supported by analyses tools and on top of all that supports a compatible license?

Currently, nobody has a clear answer:

To make some sense out of all the options available to use, here's an aspect matrix for the main contenders to help us decide:

Library Purpose License Compiles in Java 9? Runteim Ret.? Active? Clean dependency Robust null support Tool support
javax Findbugs Static analyses LGPL ❌ ✔️ ❌ ✔️ ✔️ ✔️
Spotbugs Static analyses LGPL ✔️ ❌ ✔️ ✔️ ➖ ?
JetBrains Static analyses Apache ✔️ ❌ ✔️ ✔️ ❌ ✔️
Checker framework Static analyses MIT ✔️ ✔️ ✔️ ✔️ ✔️ ✔️
Eclipse JDT Static analyses EPL ✔️ ? ✔️ ✔️ ➖ ✔️
Google's ErrorProne Static analyses Apache ✔️ ? ✔️ ✔️ ➖ ?
Lombok Code generation MIT ✔️ ? ✔️ ❌ ❌ ➖
Java Bean Validation* Runtime integrity Apache ✔️ ? ✔️ ❌ ➖ ➖
edu.umd.cs Findbugs Static analyses LGPL ✔️ ❌ ❌ ✔️ ✔️ ?
Android Static analyses ➖ ? ✔️ ➖ ❌ ✔️
Netbeans ? ? ? ? ? ? ? ?
Spring based on findbugs ➖ ➖ ? ➖ ❌ ➖ ➖

*Java Bean Validation (JSR-380) is implemented by Hibernate Validator under Apache v2
*Checker framework (and probably other) supports Lombok's null annotations

Other alternatives are switching to strict non-null Option types using some variation of Optional or the Null-object pattern, or even force Java 9 to work with Findbugs with some hackery.

Since Simple Java Mail only uses @NonNull and @Nullable, I'm thinking the Jetbrains annotations would be a comfortable transition.

bbottema commented

Until there's a clearly better alternative, for now we're going with the Intellij nullability annotations.

bbottema commented

Unfortunately, the Jetbrains annotations have a different and incompatible retention with Simple Java Mail compared to the ones from Findbugs / Spotbugs. Simple Java Mail relies on runtime availability of this information to analyse CLI requirements.

Updated the matrix.

The Checkers Framework seemed like a viable option, but it seems you need minimum Java 8 for that, while Simple Java Mail is on Java 7.

bbottema commented

If it was only for use with Spotbugs code analyses during builds, we could have rolled our own annotation classes as long as they are name Nonnull or NotNull and Nullable (since Spotbugs checks the name, not the package per se).

However, I prefer Simple Java Mail to be analyzed externally as well by something like Codacy or Code Climate which might use their own heuristics for analyses rather than Spotbugs'.

On that note, we're back to square one. Which annotation fits the bill as described in the initial post?

bbottema commented

The thread over at google/guava#2960 mentions an example of a JDK 6 project using checker-qual with replacement annotations. But this is still rather hacky.

bbottema commented

I just noticed a post by Mark Reinhold, one of the Java architects, that the old jsr305 library actually won't result in the split package problem, because they took the wide spread use of it into consideration and basically their modular structure around it.

Sooo... is this problem still a problem then.

bbottema commented

For now I'm going to go with "No". Closing until it becomes relevant again.

bbottema commented

Aaaand it's back.

bbottema commented

I've finally decided on an acceptable solution. As the Jetbrains developers suggested back in 2008, I've released an adapted version of the Jetbrains annotations (NotNull and Nullable), with retention changed to RUNTIME.

This solves the compatibility issue with a lightweight runtime annotations, released as a separate tiny library: https://github.com/bbottema/jetbrains-runtime-nullability-annotations.

I've already made the move, I just want to regression-test the CLI module, which depends on the runtime availability of these annotations (for generating the CLI options).

bbottema commented

Released in 6.0.0-rc1.

bbottema commented

6.0.0 has released as well, finally.