From e12482deed7b6120b4f65da042dc73912c4c4075 Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <jrn@google.com>
Date: Mon, 7 Dec 2015 12:56:32 -0800
Subject: [PATCH] Use runtime retention for Nullable annotation

JGit's Nullable type was added[1] in the hope of being able to add
nullness annotations that (a) do not preclude building and running
with Java 7 and (b) could be shared by Gerrit, which uses a custom
Nullable type for other reasons[2].  Sharing a type is useful because
Eclipse's null analysis is only able to use one Nullable type at a
time in a given workspace (so for this analysis to function in a
workspace used to develop Gerrit, JGit and Gerrit would need to use
the same Nullable type).

The new Nullable type has CLASS instead of RUNTIME retention because
there wasn't any obvious use for the annotation at run time.

Gerrit uses the Nullable annotation to communicate with Guice.  Guice
injection happens at runtime, so it needs to be able to read the
@Nullable annotations at run time[3].  Otherwise Guice produces
provisioning errors, such as

 3) null returned by binding at com.google.gerrit.lucene.LuceneChangeIndex$Factory.create()
  but parameter 7 of com.google.gerrit.lucene.LuceneChangeIndex.<init>() is not @Nullable

Switch to RUNTIME retention to avoid this.

While at it, update the javadoc to explain more clearly how this
annotation relates to other Nullable types[4].  This should make it
clearer why JGit needed another Nullable type:

 A. Avoiding dependency on Java 8
 B. RUNTIME retention to allow Guice to read the annotation at run time
 C. Named Nullable so Guice can recognize the annotation
 D. Not an addition to Java EE's javax.annotation package, to avoid
    the split-package problem[2] that prevents the annotation from
    being readable at run time when loaded from an OSGi container
 E. Avoiding heavyweight dependencies, deprecated dependencies, and
    dependencies on package internals

 org.checkerframework.checker.nullness.qual.Nullable: A
 com.sun.istack.internal.Nullable: B, E
 *.CheckForNull, *.NullAllowed, etc: C
 edu.umd.cs.findbugs.annotations.Nullable: B, E
 javax.annotation.Nullable: D
 org.eclipse.jdt.annotation.Nullable: B
 org.jetbrains.annotations.Nullable: B
 org.jmlspecs.annotation.Nullable: E
 android.annotation.Nullable, android.support.annotation.Nullable: E

[1] https://git.eclipse.org/r/59993
[2] https://gerrit-review.googlesource.com/50112
[3] https://github.com/google/guice/blob/master/core/src/com/google/inject/internal/Nullability.java
[4] https://github.com/typetools/checker-framework/blob/5832a01f1/checker/src/org/checkerframework/checker/nullness/NullnessAnnotatedTypeFactory.java#L118
    http://types.cs.washington.edu/checker-framework/current/checker-framework-manual.html#nullness-related-work

Change-Id: I6c482653d2b53e3509abb11211b67fc29cf2949c
Signed-off-by: Jonathan Nieder <jrn@google.com>
---
 .../eclipse/jgit/annotations/Nullable.java    | 39 +++++++++++++++++--
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/annotations/Nullable.java b/org.eclipse.jgit/src/org/eclipse/jgit/annotations/Nullable.java
index 4275dc4fa..7b9156710 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/annotations/Nullable.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/annotations/Nullable.java
@@ -54,15 +54,46 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 /**
- * JGit's replacement for the {@code javax.annotations.Nullable}.
+ * Marks types that can hold the value {@code null} at run time.
  * <p>
- * Denotes that a local variable, parameter, field, method return value can be
- * {@code null}.
+ * Unlike {@code org.eclipse.jdt.annotation.Nullable}, this has run-time
+ * retention, allowing the annotation to be recognized by
+ * <a href="https://github.com/google/guice/wiki/UseNullable">Guice</a>. Unlike
+ * {@code javax.annotation.Nullable}, this does not involve importing new classes
+ * to a standard (Java EE) package, so it can be deployed in an OSGi container
+ * without running into
+ * <a href="http://wiki.osgi.org/wiki/Split_Packages">split-package</a>
+ * <a href="https://gerrit-review.googlesource.com/50112">problems</a>.
+ * <p>
+ * You can use this annotation to qualify a type in a method signature or local
+ * variable declaration. The entity whose type has this annotation is allowed to
+ * hold the value {@code null} at run time. This allows annotation based null
+ * analysis to infer that
+ * <ul>
+ * <li>Binding a {@code null} value to the entity is legal.
+ * <li>Dereferencing the entity is unsafe and can trigger a
+ * {@code NullPointerException}.
+ * </ul>
+ * <p>
+ * To avoid a dependency on Java 8, this annotation does not use
+ * {@link Target @Target} {@code TYPE_USE}. That may change when JGit starts
+ * requiring Java 8.
+ * <p>
+ * <b>Warning:</b> Please do not use this annotation on arrays. Different
+ * annotation processors treat {@code @Nullable Object[]} differently: some
+ * treat it as an array of nullable objects, for consistency with versions of
+ * {@code Nullable} defined with {@code @Target} {@code TYPE_USE}, while others
+ * treat it as a nullable array of objects. JGit therefore avoids using this
+ * annotation on arrays altogether.
+ *
+ * @see <a href=
+ *      "http://types.cs.washington.edu/checker-framework/current/checker-framework-manual.html#faq-array-syntax-meaning">
+ *      The checker-framework manual</a>
  *
  * @since 4.2
  */
 @Documented
-@Retention(RetentionPolicy.CLASS)
+@Retention(RetentionPolicy.RUNTIME)
 @Target({ FIELD, METHOD, PARAMETER, LOCAL_VARIABLE })
 public @interface Nullable {
 	// marker annotation with no members