Update to JSpecify 1.0 and use JSpecify annotations in NullAway code (#…

…1000)

* Update our JSpecify dependence to 1.0
* Change NullAway source code to use JSpecify annotations (test inputs
were left alone)
* Fix position of type use `@Nullable` annotations to be next to type
and enable the `AnnotationPosition` Error Prone check
* Update README to encourage use of JSpecify

README.md

@@ -25,10 +25,9 @@ plugins {
 dependencies {
   errorprone "com.uber.nullaway:nullaway:<NullAway version>"
 
-  // Optional, some source of nullability annotations.
-  // Not required on Android if you use the support 
-  // library nullability annotations.
-  compileOnly "com.google.code.findbugs:jsr305:3.0.2"
+  // Some source of nullability annotations; JSpecify recommended,
+  // but others supported as well.
+  api "org.jspecify:jspecify:1.0.0"
 
   errorprone "com.google.errorprone:error_prone_core:<Error Prone version>"
 }
@@ -48,7 +47,7 @@ tasks.withType(JavaCompile) {
 
 Let's walk through this script step by step.  The `plugins` section pulls in the [Gradle Error Prone plugin](https://github.com/tbroyer/gradle-errorprone-plugin) for Error Prone integration.
 
-In `dependencies`, the first `errorprone` line loads NullAway, and the `compileOnly` line loads a [JSR 305](https://jcp.org/en/jsr/detail?id=305) library which provides a suitable `@Nullable` annotation (`javax.annotation.Nullable`).  NullAway allows for any `@Nullable` annotation to be used, so, e.g., `@Nullable` from the Android Support Library or JetBrains annotations is also fine. The second `errorprone` line sets the version of Error Prone is used.
+In `dependencies`, the first `errorprone` line loads NullAway, and the `api` line loads the [JSpecify](https://jspecify.dev) library which provides suitable nullability annotations, e.g., `org.jspecify.annotations.Nullable`.  NullAway allows for any `@Nullable` annotation to be used, so, e.g., `@Nullable` from the AndroidX annotations Library or JetBrains annotations is also fine. The second `errorprone` line sets the version of Error Prone is used.
 
 Finally, in the `tasks.withType(JavaCompile)` section, we pass some configuration options to NullAway.  First `check("NullAway", CheckSeverity.ERROR)` sets NullAway issues to the error level (it's equivalent to the `-Xep:NullAway:ERROR` standard Error Prone argument); by default NullAway emits warnings.  Then, `option("NullAway:AnnotatedPackages", "com.uber")` (equivalent to the `-XepOpt:NullAway:AnnotatedPackages=com.uber` standard Error Prone argument) tells NullAway that source code in packages under the `com.uber` namespace should be checked for null dereferences and proper usage of `@Nullable` annotations, and that class files in these packages should be assumed to have correct usage of `@Nullable` (see [the docs](https://github.com/uber/NullAway/wiki/Configuration) for more detail).  NullAway requires at least the `AnnotatedPackages` configuration argument to run, in order to distinguish between annotated and unannotated code.  See [the configuration docs](https://github.com/uber/NullAway/wiki/Configuration) for other useful configuration options.  For even simpler configuration of NullAway options, use the [Gradle NullAway plugin](https://github.com/tbroyer/gradle-nullaway-plugin).
 
@@ -58,7 +57,7 @@ We recommend addressing all the issues that Error Prone reports, particularly th
 
 Versions 3.0.0 and later of the Gradle Error Prone Plugin [no longer support Android](https://github.com/tbroyer/gradle-errorprone-plugin/releases/tag/v3.0.0).  So if you're using a recent version of this plugin, you'll need to add some further configuration to run Error Prone and NullAway.  Our [sample app `build.gradle` file](https://github.com/uber/NullAway/blob/master/sample-app/build.gradle) shows one way to do this, but your Android project may require tweaks.  Alternately, 2.x versions of the Gradle Error Prone Plugin still support Android and may still work with your project.
 
-Beyond that, compared to the Java configuration, the `com.google.code.findbugs:jsr305:3.0.2` dependency can be removed; you can use the `android.support.annotation.Nullable` annotation from the Android Support library instead.
+Beyond that, compared to the Java configuration, the JSpecify dependency can be removed; you can use the `androidx.annotation.Nullable` annotation from the AndroidX annotation library instead.
 
 #### Annotation Processors / Generated Code
 

build.gradle

@@ -82,6 +82,7 @@ subprojects { project ->
             check("UnusedException", CheckSeverity.ERROR)
             check("UnnecessaryFinal", CheckSeverity.ERROR)
             check("PreferredInterfaceType", CheckSeverity.ERROR)
+            check("AnnotationPosition", CheckSeverity.ERROR)
             // To enable auto-patching, uncomment the line below, replace [CheckerName] with
             // the checker(s) you want to apply patches for (comma-separated), and above, disable
             // "-Werror"

gradle/dependencies.gradle

@@ -77,8 +77,7 @@ def build = [
     guava                   : "com.google.guava:guava:30.1-jre",
     javaparser              : "com.github.javaparser:javaparser-core:3.26.0",
     javaxValidation         : "javax.validation:validation-api:2.0.1.Final",
-    jspecify                : "org.jspecify:jspecify:0.3.0",
-    jsr305Annotations       : "com.google.code.findbugs:jsr305:3.0.2",
+    jspecify                : "org.jspecify:jspecify:1.0.0",
     commonsIO               : "commons-io:commons-io:2.11.0",
     wala                    : [
         "com.ibm.wala:com.ibm.wala.util:${versions.wala}",
@@ -112,7 +111,8 @@ def test = [
     rxjava2                 : "io.reactivex.rxjava2:rxjava:2.1.2",
     commonsLang3            : "org.apache.commons:commons-lang3:3.8.1",
     commonsLang             : "commons-lang:commons-lang:2.6",
-    lombok                  : "org.projectlombok:lombok:1.18.24",
+    jsr305Annotations       : "com.google.code.findbugs:jsr305:3.0.2",
+    lombok                  : "org.projectlombok:lombok:1.18.34",
     springBeans             : "org.springframework:spring-beans:5.3.7",
     springContext           : "org.springframework:spring-context:5.3.7",
     grpcCore                : "io.grpc:grpc-core:1.15.1", // Should upgrade, but this matches our guava version

guava-recent-unit-tests/build.gradle

@@ -32,7 +32,7 @@ dependencies {
     testImplementation(deps.build.errorProneTestHelpers) {
         exclude group: "junit", module: "junit"
     }
-    testImplementation deps.build.jsr305Annotations
+    testImplementation deps.test.jsr305Annotations
     testImplementation "com.google.guava:guava:31.1-jre"
 
     errorProneOldest deps.build.errorProneCheckApiOld

jar-infer/test-java-lib-jarinfer/build.gradle

@@ -53,7 +53,7 @@ dependencies {
     compileOnly deps.apt.autoService
     annotationProcessor deps.apt.autoService
     compileOnly project(":nullaway")
-    implementation deps.build.jsr305Annotations
+    api deps.build.jspecify
 }
 
 jar.dependsOn ":jar-infer:jar-infer-cli:assemble"

jdk-recent-unit-tests/build.gradle

@@ -39,7 +39,7 @@ dependencies {
     testImplementation(deps.build.errorProneTestHelpers) {
         exclude group: "junit", module: "junit"
     }
-    testImplementation deps.build.jsr305Annotations
+    testImplementation deps.test.jsr305Annotations
     testModulePath deps.test.cfQual
 }
 

jmh/build.gradle

@@ -41,6 +41,7 @@ dependencies {
     // use the same version of Error Prone Core that we are compiling NullAway against, so we can
     // benchmark against different versions of Error Prone
     implementation deps.build.errorProneCoreForApi
+    api deps.build.jspecify
 
 
     // Source jars for our desired benchmarks

jmh/src/main/java/com/uber/nullaway/jmh/NullawayJavac.java

@@ -33,13 +33,13 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
-import javax.annotation.Nullable;
 import javax.tools.DiagnosticListener;
 import javax.tools.JavaCompiler;
 import javax.tools.JavaFileObject;
 import javax.tools.SimpleJavaFileObject;
 import javax.tools.StandardJavaFileManager;
 import javax.tools.ToolProvider;
+import org.jspecify.annotations.Nullable;
 
 /**
  * Code to run Javac with NullAway enabled, designed to aid benchmarking. Construction of {@code
@@ -53,7 +53,7 @@ public class NullawayJavac {
   //////////////////////
   private List<JavaFileObject> compilationUnits;
   private JavaCompiler compiler;
-  @Nullable private DiagnosticListener<JavaFileObject> diagnosticListener;
+  private @Nullable DiagnosticListener<JavaFileObject> diagnosticListener;
   private StandardJavaFileManager fileManager;
   private List<String> options;
 

library-model/test-library-model-generator/build.gradle

@@ -49,7 +49,7 @@ dependencies {
     compileOnly deps.apt.autoService
     annotationProcessor deps.apt.autoService
     compileOnly project(":nullaway")
-    implementation deps.build.jsr305Annotations
+    api deps.build.jspecify
 }
 
 jar.dependsOn ":library-model:library-model-generator-cli:assemble"

nullaway/build.gradle

@@ -31,7 +31,8 @@ dependencies {
     annotationProcessor deps.apt.autoValue
     compileOnly deps.apt.autoServiceAnnot
     annotationProcessor deps.apt.autoService
-    compileOnly deps.build.jsr305Annotations
+    // Using api following the guidance at https://jspecify.dev/docs/using#gradle
+    api deps.build.jspecify
     compileOnly deps.test.jetbrainsAnnotations
     compileOnly deps.apt.javaxInject
 

nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java

@@ -35,8 +35,8 @@
 import com.uber.nullaway.handlers.Handler;
 import java.util.HashMap;
 import java.util.Map;
-import javax.annotation.Nullable;
 import javax.lang.model.element.ElementKind;
+import org.jspecify.annotations.Nullable;
 
 /**
  * Provides APIs for querying whether code is annotated for nullness checking, and for related

nullaway/src/main/java/com/uber/nullaway/Config.java

@@ -26,7 +26,7 @@
 import com.sun.tools.javac.code.Symbol;
 import com.uber.nullaway.fixserialization.FixSerializationConfig;
 import java.util.Set;
-import javax.annotation.Nullable;
+import org.jspecify.annotations.Nullable;
 
 /** Provides configuration parameters for the nullability checker. */
 public interface Config {
@@ -238,8 +238,7 @@ public interface Config {
    *     return an @NonNull copy (likely through an unsafe downcast, but performing runtime checking
    *     and logging)
    */
-  @Nullable
-  String getCastToNonNullMethod();
+  @Nullable String getCastToNonNullMethod();
 
   /**
    * Gets an optional comment to add to auto-fix suppressions.

nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java

@@ -29,7 +29,7 @@
 import com.sun.tools.javac.code.Symbol;
 import com.uber.nullaway.fixserialization.FixSerializationConfig;
 import java.util.Set;
-import javax.annotation.Nullable;
+import org.jspecify.annotations.Nullable;
 
 /**
  * Dummy Config class required for the {@link NullAway} empty constructor.
@@ -175,8 +175,7 @@ public Set<String> getOptionalClassPaths() {
   }
 
   @Override
-  @Nullable
-  public String getCastToNonNullMethod() {
+  public @Nullable String getCastToNonNullMethod() {
     throw new IllegalStateException(ERROR_MESSAGE);
   }
 

nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java

@@ -61,10 +61,10 @@
 import java.util.List;
 import java.util.Set;
 import java.util.stream.StreamSupport;
-import javax.annotation.Nullable;
 import javax.lang.model.element.Element;
 import javax.lang.model.element.ElementKind;
 import javax.tools.JavaFileObject;
+import org.jspecify.annotations.Nullable;
 
 /** A class to construct error message to be displayed after the analysis finds error. */
 public class ErrorBuilder {
@@ -318,8 +318,7 @@ Description.Builder addSuppressWarningsFix(
    * <p>TODO: actually use {@link
    * com.google.errorprone.fixes.SuggestedFixes#addSuppressWarnings(VisitorState, String)} instead
    */
-  @Nullable
-  private Tree suppressibleNode(@Nullable TreePath path) {
+  private @Nullable Tree suppressibleNode(@Nullable TreePath path) {
     if (path == null) {
       return null;
     }

nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java

@@ -37,7 +37,7 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.regex.Pattern;
-import javax.annotation.Nullable;
+import org.jspecify.annotations.Nullable;
 
 /**
  * provides nullability configuration based on additional flags passed to ErrorProne via
@@ -189,12 +189,12 @@ final class ErrorProneCLIFlagsConfig implements Config {
   private final Pattern unannotatedSubPackages;
 
   /** Source code in these classes will not be analyzed for nullability issues */
-  @Nullable private final ImmutableSet<String> sourceClassesToExclude;
+  private final @Nullable ImmutableSet<String> sourceClassesToExclude;
 
   /**
    * these classes will be treated as unannotated (don't analyze *and* treat methods as unannotated)
    */
-  @Nullable private final ImmutableSet<String> unannotatedClasses;
+  private final @Nullable ImmutableSet<String> unannotatedClasses;
 
   private final Pattern fieldAnnotPattern;
   private final boolean isExhaustiveOverride;
@@ -214,7 +214,7 @@ final class ErrorProneCLIFlagsConfig implements Config {
   private final ImmutableSet<String> initializerAnnotations;
   private final ImmutableSet<String> externalInitAnnotations;
   private final ImmutableSet<String> contractAnnotations;
-  @Nullable private final String castToNonNullMethod;
+  private final @Nullable String castToNonNullMethod;
   private final String autofixSuppressionComment;
   private final ImmutableSet<String> skippedLibraryModels;
   private final ImmutableSet<String> extraFuturesClasses;
@@ -515,8 +515,7 @@ public boolean assertsEnabled() {
   }
 
   @Override
-  @Nullable
-  public String getCastToNonNullMethod() {
+  public @Nullable String getCastToNonNullMethod() {
     return castToNonNullMethod;
   }
 

nullaway/src/main/java/com/uber/nullaway/NullAway.java

@@ -110,7 +110,6 @@
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
-import javax.annotation.Nullable;
 import javax.inject.Inject;
 import javax.lang.model.element.AnnotationMirror;
 import javax.lang.model.element.Element;
@@ -122,6 +121,7 @@
 import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode;
 import org.checkerframework.nullaway.javacutil.ElementUtils;
 import org.checkerframework.nullaway.javacutil.TreeUtils;
+import org.jspecify.annotations.Nullable;
 
 /**
  * Checker for nullability errors. It assumes that any field, method parameter, or return type that
@@ -2154,8 +2154,7 @@ private Set<Element> getSafeInitMethods(
    * @param state visitor state
    * @return element of safe init function if stmt invokes that function; null otherwise
    */
-  @Nullable
-  private Element getInvokeOfSafeInitMethod(
+  private @Nullable Element getInvokeOfSafeInitMethod(
       StatementTree stmt, Symbol.ClassSymbol enclosingClassSymbol, VisitorState state) {
     Matcher<ExpressionTree> invokeMatcher =
         (expressionTree, s) -> {

nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java

@@ -51,11 +51,11 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
-import javax.annotation.Nullable;
 import javax.lang.model.element.AnnotationMirror;
 import javax.lang.model.element.AnnotationValue;
 import javax.lang.model.element.ExecutableElement;
 import org.checkerframework.nullaway.javacutil.AnnotationUtils;
+import org.jspecify.annotations.Nullable;
 
 /** Helpful utility methods for nullability analysis. */
 public class NullabilityUtil {
@@ -102,8 +102,7 @@ public static boolean lambdaParamIsImplicitlyTyped(VariableTree lambdaParameter)
    * @return closest overridden ancestor method, or <code>null</code> if method does not override
    *     anything
    */
-  @Nullable
-  public static Symbol.MethodSymbol getClosestOverriddenMethod(
+  public static Symbol.@Nullable MethodSymbol getClosestOverriddenMethod(
       Symbol.MethodSymbol method, Types types) {
     // taken from Error Prone MethodOverrides check
     Symbol.ClassSymbol owner = method.enclClass();
@@ -135,8 +134,7 @@ public static Symbol.MethodSymbol getClosestOverriddenMethod(
    * @param others also stop and return in case of any of these tree kinds
    * @return the closest enclosing method / lambda
    */
-  @Nullable
-  public static TreePath findEnclosingMethodOrLambdaOrInitializer(
+  public static @Nullable TreePath findEnclosingMethodOrLambdaOrInitializer(
       TreePath path, ImmutableSet<Tree.Kind> others) {
     TreePath curPath = path.getParentPath();
     while (curPath != null) {
@@ -169,8 +167,7 @@ public static TreePath findEnclosingMethodOrLambdaOrInitializer(
    * @param path the tree path
    * @return the closest enclosing method / lambda
    */
-  @Nullable
-  public static TreePath findEnclosingMethodOrLambdaOrInitializer(TreePath path) {
+  public static @Nullable TreePath findEnclosingMethodOrLambdaOrInitializer(TreePath path) {
     return findEnclosingMethodOrLambdaOrInitializer(path, ImmutableSet.of());
   }