Adopt jspecify + NullAway null-checking on fdb-relational-core

fdb-relational-core/fdb-relational-core.gradle

@@ -21,6 +21,7 @@
 plugins {
     id 'java-test-fixtures'
     alias(libs.plugins.jmh)
+    alias(libs.plugins.errorprone)
 }
 
 apply from: rootProject.file('gradle/antlr.gradle')
@@ -36,6 +37,7 @@ dependencies {
     compileOnly(libs.autoService)
     annotationProcessor(libs.autoService)
     implementation(libs.jsr305)
+    compileOnly(libs.jspecify)
     implementation(libs.guava)
     implementation(libs.antlr)
     implementation(libs.log4j.core)
@@ -44,6 +46,9 @@ dependencies {
 
     antlr(libs.antlr)
 
+    errorprone(libs.errorprone.core)
+    errorprone(libs.nullaway)
+
     testImplementation(libs.bundles.test.impl)
     testImplementation(libs.bundles.test.runtime)
     testCompileOnly(libs.bundles.test.compileOnly)
@@ -58,6 +63,7 @@ dependencies {
     testFixturesImplementation(libs.protobuf)
     testFixturesImplementation(libs.assertj)
     testFixturesImplementation(libs.jsr305)
+    testFixturesCompileOnly(libs.jspecify)
     // This is included just for ResultSetAssert. It does a bunch of this
     // Type.Record record = ProtobufDdlUtil.recordFromDescriptor(((Message) colValue).getDescriptorForType());
     testFixturesImplementation(project(coreProject))
@@ -73,6 +79,25 @@ tasks.withType(Test).configureEach { theTask ->
     theTask.testFramework.options.includeEngines.add('junit-jupiter')
 }
 
+// jspecify + NullAway null-checking, scoped to this module. See @NullMarked package-info.java
+// files under com.apple.foundationdb.relational.{api,recordlayer,transactionbound,util} in this
+// module (fdb-relational-core also has non-@NullMarked test-only packages, e.g.
+// com.apple.foundationdb.relational.memory / .autotest under src/test, which fall outside these
+// prefixes and are therefore left unchecked). Generated ANTLR parser code
+// (com.apple.foundationdb.relational.generated) and generated protobuf code
+// (com.apple.foundationdb.relational.continuation, com.apple.foundationdb.relational.copy,
+// com.apple.foundationdb.record) already live outside these package prefixes, so no
+// UnannotatedSubPackages carve-out is needed.
+tasks.withType(JavaCompile).configureEach {
+    options.errorprone {
+        disableAllChecks = true
+        error("NullAway")
+        option("NullAway:AnnotatedPackages", "com.apple.foundationdb.relational.api,com.apple.foundationdb.relational.recordlayer,com.apple.foundationdb.relational.transactionbound,com.apple.foundationdb.relational.util")
+        option("NullAway:JSpecifyMode", "true")
+        option("NullAway:AcknowledgeRestrictiveAnnotations", "true")
+    }
+}
+
 publishing {
     publications {
         library(MavenPublication) {

fdb-relational-core/src/jmh/java/com/apple/foundationdb/relational/api/SetSchemaBenchmark.java

@@ -101,6 +101,10 @@ public static void main(String[] args) throws RunnerException {
     }
 
     @State(Scope.Thread)
+    // NullAway.Init is suppressed here because connection follows the standard JMH benchmark
+    // lifecycle: it is left unset by the constructor and is always populated by init() (a @Setup
+    // method) before any benchmark method that uses it runs.
+    @SuppressWarnings("NullAway.Init")
     public static class RelationalConnHolder {
         private Connection connection;
 

fdb-relational-core/src/jmh/java/com/apple/foundationdb/relational/recordlayer/BenchmarkConnHolder.java

@@ -32,6 +32,10 @@
  * Shared JMH thread-scoped connection holder for relational benchmarks.
  * Subclasses supply the database URI and schema; this class manages the connection lifecycle.
  */
+// NullAway.Init is suppressed here because connection follows the standard JMH benchmark
+// lifecycle: it is left unset by the constructor and is always populated by init() (a @Setup
+// method) before any benchmark method that uses it runs.
+@SuppressWarnings("NullAway.Init")
 abstract class BenchmarkConnHolder {
 
     protected final URI dbUri;

fdb-relational-core/src/jmh/java/com/apple/foundationdb/relational/recordlayer/EmbeddedRelationalBenchmark.java

@@ -35,6 +35,7 @@
 import com.apple.foundationdb.relational.recordlayer.catalog.StoreCatalogProvider;
 import com.apple.foundationdb.relational.recordlayer.ddl.RecordLayerMetadataOperationsFactory;
 import com.apple.foundationdb.relational.recordlayer.query.cache.RelationalPlanCache;
+import org.jspecify.annotations.Nullable;
 import org.openjdk.jmh.annotations.Level;
 import org.openjdk.jmh.annotations.Scope;
 import org.openjdk.jmh.annotations.State;
@@ -68,6 +69,10 @@ public abstract class EmbeddedRelationalBenchmark {
 
     static final String schemaTemplateName = "RestaurantTemplate";
 
+    // NullAway.Init is suppressed here because driver/keySpace/fdbDatabase/catalog follow the
+    // standard JMH benchmark lifecycle: they are left unset by the constructor and are always
+    // populated by up() before any benchmark method that uses them runs.
+    @SuppressWarnings("NullAway.Init")
     public static class Driver {
         RelationalDriver driver;
         KeySpace keySpace;
@@ -76,15 +81,15 @@ public static class Driver {
         private final String templateName;
         private final String templateDef;
 
-        private final RelationalPlanCache planCache;
+        private final @Nullable RelationalPlanCache planCache;
         public StoreCatalog catalog;
 
 
         public Driver() {
             this(schemaTemplateName, templateDefinition);
         }
 
-        public Driver(RelationalPlanCache planCache) {
+        public Driver(@Nullable RelationalPlanCache planCache) {
             this(schemaTemplateName, templateDefinition, planCache);
         }
 
@@ -94,7 +99,7 @@ public Driver(String templateName, String templateDef) {
             this.planCache = null;
         }
 
-        public Driver(String templateName, String templateDef, RelationalPlanCache planCache) {
+        public Driver(String templateName, String templateDef, @Nullable RelationalPlanCache planCache) {
             this.templateName = templateName;
             this.templateDef = templateDef;
             this.planCache = planCache;

fdb-relational-core/src/jmh/java/com/apple/foundationdb/relational/recordlayer/IndexScanVsQueryBenchmark.java

@@ -85,6 +85,10 @@
 @OutputTimeUnit(TimeUnit.MILLISECONDS)
 @State(Scope.Benchmark)
 @API(API.Status.EXPERIMENTAL)
+// NullAway.Init is suppressed here because indexHintLabel/indexHintCategory follow the standard
+// JMH benchmark lifecycle: they are left unset by the constructor and are always populated by
+// trialUp() (a @Setup method) before any benchmark method that uses them runs.
+@SuppressWarnings("NullAway.Init")
 public class IndexScanVsQueryBenchmark extends EmbeddedRelationalBenchmark {
 
     static final String schema = "bench";

fdb-relational-core/src/jmh/java/com/apple/foundationdb/relational/recordlayer/RecordLayerScanBenchmark.java

@@ -67,6 +67,9 @@
 @API(API.Status.EXPERIMENTAL)
 public class RecordLayerScanBenchmark extends RelationalScanBenchmark {
 
+    // NullAway does not reliably track @Nullable on byte[] parameters, so it flags the (legitimate)
+    // null continuation argument to scanRecords below even though that method declares it @Nullable.
+    @SuppressWarnings("NullAway")
     @Override
     public void scan(Blackhole bh, RelationalConnHolder ignored) throws RelationalException {
         FDBDatabase fdbDb = FDBDatabaseFactory.instance().getDatabase();

fdb-relational-core/src/jmh/java/com/apple/foundationdb/relational/recordlayer/RelationalScanBenchmark.java

@@ -70,6 +70,10 @@
 @OutputTimeUnit(TimeUnit.MILLISECONDS)
 @State(Scope.Benchmark)
 @API(API.Status.EXPERIMENTAL)
+// NullAway.Init is suppressed here because accessor follows the standard JMH benchmark lifecycle:
+// it is left unset by the constructor and is always populated by trialUp() (a @Setup method)
+// before any benchmark method that uses it runs.
+@SuppressWarnings("NullAway.Init")
 public class RelationalScanBenchmark extends EmbeddedRelationalBenchmark {
 
     static final String schema = "putAndScan";
@@ -182,6 +186,10 @@ public static void main(String[] args) throws RunnerException {
     }
 
     @State(Scope.Thread)
+    // NullAway.Init is suppressed here because connection follows the standard JMH benchmark
+    // lifecycle: it is left unset by the constructor and is always populated by init() (a @Setup
+    // method) before any benchmark method that uses it runs.
+    @SuppressWarnings("NullAway.Init")
     public static class RelationalConnHolder {
         private Connection connection;
 

fdb-relational-core/src/jmh/java/com/apple/foundationdb/relational/recordlayer/SimplePlanCachingBenchmark.java

@@ -29,6 +29,7 @@
 import com.apple.foundationdb.relational.api.exceptions.RelationalException;
 import com.apple.foundationdb.relational.recordlayer.query.cache.RelationalPlanCache;
 import com.apple.foundationdb.relational.recordlayer.util.ExceptionUtil;
+import org.jspecify.annotations.Nullable;
 import org.openjdk.jmh.annotations.Benchmark;
 import org.openjdk.jmh.annotations.BenchmarkMode;
 import org.openjdk.jmh.annotations.Fork;
@@ -74,6 +75,10 @@
 @Threads(Threads.MAX)
 @OutputTimeUnit(TimeUnit.MILLISECONDS)
 @API(API.Status.EXPERIMENTAL)
+// NullAway.Init is suppressed here because cacheType/driver follow the standard JMH benchmark
+// lifecycle: cacheType is populated by JMH via @Param reflection and driver is populated by
+// trialUp() (a @Setup method), not by the constructor.
+@SuppressWarnings("NullAway.Init")
 public class SimplePlanCachingBenchmark extends EmbeddedRelationalBenchmark {
     static final String dbName = "/BENCHMARKS/SimplePlanCaching";
 
@@ -124,7 +129,7 @@ public void repeatedRead(Blackhole bh) throws SQLException {
         }
     }
 
-    private RelationalPlanCache getPlanCache() {
+    private @Nullable RelationalPlanCache getPlanCache() {
         switch (cacheType) {
             case "NONE":
                 return null;

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/api/EmbeddedRelationalArray.java

@@ -25,8 +25,8 @@
 import com.apple.foundationdb.relational.api.metadata.DataType;
 import com.apple.foundationdb.relational.util.Assert;
 
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
+import org.jspecify.annotations.Nullable;
+
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.List;
@@ -38,19 +38,19 @@ static RelationalArrayBuilder newBuilder() {
         return new Builder();
     }
 
-    static RelationalArrayBuilder newBuilder(@Nonnull DataType elementType) {
+    static RelationalArrayBuilder newBuilder(DataType elementType) {
         return new Builder(elementType);
     }
 
     class Builder implements RelationalArrayBuilder {
 
         @Nullable private DataType elementType;
-        @Nonnull private final List<Object> elements = new ArrayList<>();
+        private final List<Object> elements = new ArrayList<>();
 
         private Builder() {
         }
 
-        private Builder(@Nonnull DataType elementType) {
+        private Builder(DataType elementType) {
             this.elementType = elementType;
         }
 
@@ -63,7 +63,7 @@ public EmbeddedRelationalArray build() throws SQLException {
         }
 
         @Override
-        public Builder addAll(@Nonnull Object... values) throws SQLException {
+        public Builder addAll(Object... values) throws SQLException {
             for (var value : values) {
                 if (value == null) {
                     throw new RelationalException("Cannot add NULL to an array.", ErrorCode.DATATYPE_MISMATCH).toSqlException();
@@ -86,22 +86,22 @@ public Builder addLong(long value) throws SQLException {
         }
 
         @Override
-        public Builder addString(@Nonnull String value) throws SQLException {
+        public Builder addString(String value) throws SQLException {
             return addField(value, DataType.Primitives.STRING.type());
         }
 
         @Override
-        public Builder addBytes(@Nonnull byte[] value) throws SQLException {
+        public Builder addBytes(byte[] value) throws SQLException {
             return addField(value, DataType.Primitives.BYTES.type());
         }
 
         @Override
-        public Builder addUuid(@Nonnull UUID uuid) throws SQLException {
+        public Builder addUuid(UUID uuid) throws SQLException {
             return addField(uuid, DataType.Primitives.UUID.type());
         }
 
         @Override
-        public Builder addObject(@Nonnull Object obj) throws SQLException {
+        public Builder addObject(Object obj) throws SQLException {
             if (obj instanceof RelationalStruct) {
                 return addStruct((RelationalStruct) obj);
             }
@@ -111,8 +111,7 @@ public Builder addObject(@Nonnull Object obj) throws SQLException {
             return addField(obj, DataType.getDataTypeFromObject(obj));
         }
 
-        @Nonnull
-        private Builder addField(@Nonnull Object value, @Nonnull DataType type) throws SQLException {
+        private Builder addField(Object value, DataType type) throws SQLException {
             try {
                 checkType(type);
             } catch (RelationalException ve) {

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/api/EmbeddedRelationalDriver.java

@@ -26,8 +26,8 @@
 import com.apple.foundationdb.relational.api.exceptions.ErrorCode;
 import com.apple.foundationdb.relational.api.exceptions.RelationalException;
 
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
+import org.jspecify.annotations.Nullable;
+
 import java.net.URI;
 import java.sql.Connection;
 import java.sql.SQLException;
@@ -43,7 +43,10 @@ public class EmbeddedRelationalDriver implements RelationalDriver {
 
     private EmbeddedRelationalEngine engine;
 
-    public EmbeddedRelationalDriver(@Nullable EmbeddedRelationalEngine engine) throws SQLException {
+    // engine is dereferenced unconditionally below (e.g. engine.getStorageClusters()) with no null check,
+    // and no caller in this codebase ever passes null, so @Nullable here was never actually honored;
+    // require it, matching actual usage.
+    public EmbeddedRelationalDriver(EmbeddedRelationalEngine engine) throws SQLException {
         this.engine = engine;
     }
 
@@ -53,15 +56,19 @@ public Connection connect(String url, Properties info) throws SQLException {
     }
 
     @Override
-    public RelationalConnection connect(@Nonnull URI url,
-                                        @Nonnull Options connectionOptions) throws SQLException {
+    public RelationalConnection connect(URI url,
+                                        Options connectionOptions) throws SQLException {
         return connect(url, null, connectionOptions);
     }
 
-    @SuppressWarnings("PMD.CloseResource") // returns connection outliving auto-closeable object. Should consider refactoring
-    public RelationalConnection connect(@Nonnull URI url,
+    @SuppressWarnings({"PMD.CloseResource", "NullAway"})
+    // PMD: returns connection outliving auto-closeable object. Should consider refactoring
+    // NullAway: per the java.sql.Driver#connect(String, Properties) contract this method mirrors, returning
+    // null here for a URL this driver doesn't understand is correct, expected behavior (relied upon by
+    // java.sql.DriverManager when trying multiple registered drivers), not a bug.
+    public RelationalConnection connect(URI url,
                                         @Nullable Transaction existingTransaction,
-                                        @Nonnull Options connectionOptions) throws SQLException {
+                                        Options connectionOptions) throws SQLException {
         final var urlString = url.toString();
         if (!acceptsURL(urlString)) {
             return null;

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/api/EmbeddedRelationalEngine.java

@@ -26,7 +26,6 @@
 import com.codahale.metrics.MetricRegistry;
 import com.codahale.metrics.MetricSet;
 
-import javax.annotation.Nonnull;
 import java.util.Collection;
 import java.util.List;
 
@@ -42,7 +41,7 @@ public class EmbeddedRelationalEngine {
     //TODO(bfines) eventually we need to move StoreCatalog into StorageCluster
     @SpotBugsSuppressWarnings(value = "CT_CONSTRUCTOR_THROW", justification = "May be refactored as embedded takes over transaction lifetime")
     public EmbeddedRelationalEngine(List<StorageCluster> fdbClusters,
-                                  @Nonnull MetricRegistry metricRegistry) {
+                                  MetricRegistry metricRegistry) {
         if (fdbClusters.isEmpty()) {
             throw new IllegalArgumentException("Must specify at least one FDB cluster to connect to");
         }

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/api/EmbeddedRelationalStruct.java

@@ -22,10 +22,9 @@
 
 import com.apple.foundationdb.relational.api.metadata.DataType;
 import com.apple.foundationdb.relational.recordlayer.ArrayRow;
-import com.apple.foundationdb.relational.util.SpotBugsSuppressWarnings;
 
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
+import org.jspecify.annotations.Nullable;
+
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.List;
@@ -37,7 +36,6 @@ static RelationalStructBuilder newBuilder() {
         return new Builder();
     }
 
-    @SpotBugsSuppressWarnings(value = "NP_METHOD_PARAMETER_TIGHTENS_ANNOTATION", justification = "False positive: RelationalStructBuilder's parameters are jspecify @Nullable (fdb-relational-api), while this class still uses javax.annotation.Nullable (fdb-relational-core not yet migrated); SpotBugs does not recognize the two annotations as equivalent across the module boundary, including on the covariant-return bridge methods synthesized for addString/addUuid/addObject, which it can't otherwise be annotated.")
     class Builder implements RelationalStructBuilder {
 
         final List<DataType.StructType.Field> fields = new ArrayList<>();
@@ -101,13 +99,13 @@ public RelationalStructBuilder addObject(String fieldName, @Nullable Object obj)
         }
 
         @Override
-        public Builder addStruct(String fieldName, @Nonnull RelationalStruct struct) throws SQLException {
+        public Builder addStruct(String fieldName, RelationalStruct struct) throws SQLException {
             addField(fieldName, struct.getMetaData().getRelationalDataType(), struct);
             return this;
         }
 
         @Override
-        public Builder addArray(String fieldName, @Nonnull RelationalArray array) throws SQLException {
+        public Builder addArray(String fieldName, RelationalArray array) throws SQLException {
             addField(fieldName, array.getMetaData().asRelationalType(), array);
             return this;
         }
@@ -117,7 +115,7 @@ public Builder addInt(String fieldName, int i) {
             return addField(fieldName, DataType.Primitives.INTEGER.type(), i);
         }
 
-        private Builder addField(@Nonnull String fieldName, @Nonnull DataType type, @Nullable Object o) {
+        private Builder addField(String fieldName, DataType type, @Nullable Object o) {
             fields.add(DataType.StructType.Field.from(fieldName, type, fields.size() + 1));
             elements.add(o);
             return this;