Adopt jspecify + NullAway null-checking on fdb-relational-grpc by arnaud-lacurie · Pull Request #4532 · FoundationDB/fdb-record-layer

arnaud-lacurie · 2026-08-28T14:27:45Z

First of a two-PR stack adopting jspecify (@NullMarked/@Nullable) plus NullAway for compile-time null-checking, scoped to fdb-relational-grpc only, ahead of a possible repo-wide rollout.

This PR — fdb-relational-grpc
Adopt jspecify + NullAway null-checking on fdb-relational-jdbc #4533 — fdb-relational-jdbc (stacked on top)

NullAway wiring is scoped to this module's .gradle file only; root build.gradle and gradle/check.gradle are untouched. See inline comments for specific findings and tooling caveats.

@NullMarked

Adds org.jspecify:jspecify plus NullAway (via the net.ltgt.errorprone plugin) scoped to fdb-relational-grpc only, ahead of a possible repo-wide rollout. Both jdbc.* packages are marked @NullMarked; javax.annotation.Nonnull/Nullable usages are replaced with jspecify's @nullable (non-null is now the default). Compiling with NullAway surfaced several previously undocumented nullable returns (TypeConversion, RelationalResultSetFacade, RelationalStructFacade, RelationalRpcContinuation) and one latent bug: RelationalArrayFacadeBuilder.build() could construct a RelationalArray with null metadata if addStruct() was never called; it now fails fast with IllegalStateException instead of NPEing later.

@nonnull

…ange TypeConversion.toProtobuf(RelationalResultSet) is now explicitly @nullable (see the jspecify null-checking change on fdb-relational-grpc). SpotBugs' NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE correctly flagged that FRL.executeInternal() passed its result straight into Response.query(@nonnull ResultSet) without a null check. The result set obtained here can't actually be null in practice (guarded by statement.execute() returning true), so guard with Objects.requireNonNull() rather than silencing the finding.

arnaud-lacurie

Inline notes for the jspecify + NullAway findings mentioned in the PR description.

arnaud-lacurie · 2026-08-28T14:42:15Z


        @Override
        public RelationalArray build() {
+            if (metadata == null) {


Latent bug caught by NullAway: metadata was never initialized if addStruct() was never called, so build() could previously hand back a RelationalArrayFacade with a null delegateMetadata — which would NPE later, confusingly, inside computeType(). Now fails fast here instead.

arnaud-lacurie · 2026-08-28T14:42:15Z

    @Nullable
    public static SQLException map(StatusRuntimeException statusRuntimeException) {
        Status status = StatusProto.fromThrowable(statusRuntimeException);
+        if (status == null) {


StatusProto.fromThrowable(...) is @Nullable per NullAway's built-in library model for this method; the previous code dereferenced it unguarded a few lines down in the two-arg map(Status) overload.

arnaud-lacurie · 2026-08-28T14:42:15Z

 import java.util.UUID;
 import java.util.function.BiFunction;

+import static com.apple.foundationdb.relational.api.Continuation.Reason;


Static import needed here, not just style: jspecify requires Outer.@Nullable Inner syntax to annotate a nullable nested/qualified type (@Nullable Continuation.Reason is a javac error — "scoping construct cannot be annotated"), but the bundled PMD version (6.44.0) can't parse that syntax and silently skips the whole file instead of failing the build. Importing Reason directly sidesteps both.

arnaud-lacurie · 2026-08-28T14:42:15Z

    }

-    private <R> R get(int oneBasedIndex, Function<Column, R> s) {
+    private <R extends @Nullable Object> R get(int oneBasedIndex, Function<Column, R> s) {


Plain <R> implies a non-null upper bound under JSpecify, so this generic helper (reused by both nullable- and non-nullable-returning callers) needs the bound spelled out explicitly to allow nullable instantiation.

arnaud-lacurie · 2026-08-28T14:42:15Z

            return null;
-        });
+        };
+        return this.<@Nullable String>get(oneBasedColumn, f);


Needed an explicit type witness here — nullness inference for the type parameter didn't propagate automatically from the lambda's target type through this generic call.

arnaud-lacurie · 2026-08-28T14:42:15Z

                    if (relationalStatement.execute(sql)) {
                        try (RelationalResultSet rs = relationalStatement.getResultSet()) {
-                            resultSet = TypeConversion.toProtobuf(rs);
+                            resultSet = Objects.requireNonNull(TypeConversion.toProtobuf(rs));


Cross-module ripple: making TypeConversion.toProtobuf(RelationalResultSet) explicitly @Nullable let SpotBugs (which reads nullability annotations across module boundaries, unlike NullAway's own module-scoped enforcement) catch this previously-unguarded dereference in this unmigrated module. rs can't actually be null here in practice (guarded by execute() returning true), so this documents the invariant rather than papering over it.

robert-brunel · 2026-08-28T15:35:02Z

+    @Nullable
    public UUID getUUID(final String fieldName) throws SQLException {


Can’t return types like this be written more compactly as follows:

public @Nullable UUID getUUID(…)

I guess this is a taste thing, but IMO, that looks worse to me. It's also going to be a larger diff from our current codebase, where we put those on separate lines.

Hmm but @Nullable UUID more neatly conveys how JSpecify works, i.e., that it makes the nullability a proper part of the type system.

And if we look at instance variables, it’s just so much nicer to have everything on one line instead of two:

@Nullable Type1 var1; @Nullable Type2 var2;

versus

@Nullable Type1 var1; @Nullable Type2 var2;

It’s also what they do in the documentation:
https://jspecify.dev/docs/user-guide/

There are also cases where the @Nullable must go inline, e.g., from the documentation:

public static <T> List<@Nullable T> nullOutMatches(…) {

robert-brunel · 2026-08-28T15:35:33Z

+    @Nullable
    private MockResultSetRow currentRow;


For fields too it should be possible to condense this to private @Nullable MockResultSetRow currentRow;

Similar to #4532 (comment)

This is a good example of how awkward it looks if the first variable fits on one line but the second one is artificially split in two! Much nicer like this, imo:

private final RelationalResultSetMetaData metadata; private @Nullable MockResultSetRow currentRow;

arnaud-lacurie added 2 commits August 28, 2026 16:19

arnaud-lacurie added the build improvement Improvement to the build system label Aug 28, 2026

arnaud-lacurie changed the title ~~apple/arnaud lacurie/jspecify nullaway/grpc~~ Adopt jspecify + NullAway null-checking on fdb-relational-grpc Aug 28, 2026

arnaud-lacurie mentioned this pull request Aug 28, 2026

Adopt jspecify + NullAway null-checking on fdb-relational-jdbc #4533

Draft

arnaud-lacurie commented Aug 28, 2026

View reviewed changes

robert-brunel reviewed Aug 28, 2026

View reviewed changes

		@Nullable
		public UUID getUUID(final String fieldName) throws SQLException {

Conversation

arnaud-lacurie commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arnaud-lacurie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arnaud-lacurie commented Aug 28, 2026 •

edited

Loading