Add JSpecify null-safety enforcement with Error Prone + NullAway

build.gradle.kts

@@ -1,4 +1,6 @@
 import java.time.Instant
+import net.ltgt.gradle.errorprone.errorprone
+import net.ltgt.gradle.nullaway.nullaway
 
 plugins {
     id("net.kyori.indra") version "4.0.0"
@@ -7,6 +9,8 @@ plugins {
     id("io.spring.dependency-management") version "1.1.7"
     id("com.google.cloud.tools.jib") version "3.5.4"
     id("dev.lukebemish.immaculate") version "0.2.5"
+    id("net.ltgt.errorprone") version "5.1.0"
+    id("net.ltgt.nullaway") version "3.1.0"
 }
 
 indra {
@@ -33,9 +37,28 @@ dependencies {
     implementation("org.springframework.boot:spring-boot-starter-flyway")
     implementation("org.flywaydb:flyway-database-postgresql")
     implementation("org.springframework.boot:spring-boot-starter-security")
+    compileOnly("org.jspecify:jspecify")
     runtimeOnly("com.h2database:h2") // for local
     runtimeOnly("org.postgresql:postgresql") // for prod
     testImplementation("org.springframework.boot:spring-boot-starter-test")
+    errorprone("com.google.errorprone:error_prone_core:2.50.0")
+    errorprone("com.uber.nullaway:nullaway:0.13.8")
+}
+
+nullaway {
+    onlyNullMarked = true
+    jspecifyMode = true
+}
+
+tasks.withType<JavaCompile>().configureEach {
+    options.errorprone {
+        disableAllChecks = true // Only NullAway is enabled
+        error("RequireExplicitNullMarking")
+        nullaway {
+            error()
+        }
+        option("NullAway:CustomContractAnnotations", "org.springframework.lang.Contract")
+    }
 }
 
 buildscript {

src/main/java/io/papermc/patchroulette/config/package-info.java

@@ -0,0 +1,4 @@
+@NullMarked
+package io.papermc.patchroulette.config;
+
+import org.jspecify.annotations.NullMarked;

src/main/java/io/papermc/patchroulette/controller/ApiController.java

@@ -13,6 +13,8 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import org.jspecify.annotations.Nullable;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
@@ -47,7 +49,11 @@ public ResponseEntity<List<String>> getAvailablePatches(
   }
 
   public record PatchDetails(
-      String path, String status, String responsibleUser, Instant lastUpdated, Duration duration) {}
+      String path,
+      String status,
+      @Nullable String responsibleUser,
+      @Nullable Instant lastUpdated,
+      @Nullable Duration duration) {}
 
   @PreAuthorize("hasRole('PATCH')")
   @GetMapping(value = "/get-all-patches", produces = "application/json")
@@ -177,36 +183,39 @@ public ResponseEntity<Stats> stats(@RequestParam final String minecraftVersion)
     final Map<String, UserStats> users = new HashMap<>();
 
     // Track intervals for each user
-    Map<String, List<TimeUtil.TimeInterval>> userIntervals = new HashMap<>();
+    final Map<String, List<TimeUtil.TimeInterval>> userIntervals = new HashMap<>();
 
     for (Patch patch : allPatches) {
-      switch (patch.getStatus()) {
+      final Status status = patch.getStatus();
+      switch (status) {
         case AVAILABLE -> available++;
         case WIP -> wip++;
         case DONE -> done++;
       }
 
-      if (patch.getResponsibleUser() != null) {
-        users.compute(patch.getResponsibleUser(), (user, userStats) -> {
+      final String responsibleUser = patch.getResponsibleUser();
+      if (responsibleUser != null) {
+        users.compute(responsibleUser, (user, userStats) -> {
           if (userStats == null) {
-            userStats = new UserStats(patch.getResponsibleUser(), 0, 0, Duration.ZERO);
+            userStats = new UserStats(responsibleUser, 0, 0, Duration.ZERO);
           }
-          if (patch.getStatus() == Status.WIP) {
+          if (status == Status.WIP) {
             userStats.wip++;
-          } else if (patch.getStatus() == Status.DONE) {
+          } else if (status == Status.DONE) {
             userStats.done++;
           }
           return userStats;
         });
 
         // Track the time interval for this patch if it has duration
-        if (patch.getDuration() != null && patch.getLastUpdated() != null) {
-          Instant endTime = patch.getLastUpdated();
-          Instant startTime = endTime.minus(patch.getDuration());
+        final Duration duration = patch.getDuration();
+        final Instant lastUpdated = patch.getLastUpdated();
+        if (duration != null && lastUpdated != null) {
+          final Instant startTime = lastUpdated.minus(duration);
 
           userIntervals
-              .computeIfAbsent(patch.getResponsibleUser(), k -> new ArrayList<>())
-              .add(new TimeUtil.TimeInterval(startTime, endTime));
+              .computeIfAbsent(responsibleUser, k -> new ArrayList<>())
+              .add(new TimeUtil.TimeInterval(startTime, lastUpdated));
         }
       }
     }
@@ -226,8 +235,9 @@ public ResponseEntity<Stats> stats(@RequestParam final String minecraftVersion)
       // Calculate total duration from merged intervals
       Duration userDuration = TimeUtil.calculateDuration(mergedIntervals);
 
-      // Update user stats
-      users.get(user).timeSpent = userDuration;
+      // Present for every userIntervals key: both maps are populated together above.
+      final UserStats userStats = Objects.requireNonNull(users.get(user));
+      userStats.timeSpent = userDuration;
 
       // Add to total time
       totalTimeSpent = totalTimeSpent.plus(userDuration);

src/main/java/io/papermc/patchroulette/controller/package-info.java

@@ -0,0 +1,4 @@
+@NullMarked
+package io.papermc.patchroulette.controller;
+
+import org.jspecify.annotations.NullMarked;

src/main/java/io/papermc/patchroulette/model/Patch.java

@@ -8,6 +8,7 @@
 import jakarta.persistence.IdClass;
 import java.time.Duration;
 import java.time.Instant;
+import org.jspecify.annotations.Nullable;
 
 @Entity
 @IdClass(PatchId.class)
@@ -23,8 +24,13 @@ public class Patch {
   @Enumerated(EnumType.ORDINAL)
   private Status status;
 
+  @Nullable
   private String responsibleUser;
+
+  @Nullable
   private Instant lastUpdated;
+
+  @Nullable
   private Duration duration;
 
   public Patch() {}
@@ -53,37 +59,37 @@ public void setStatus(final Status status) {
     this.status = status;
   }
 
-  public String getResponsibleUser() {
+  public @Nullable String getResponsibleUser() {
     return this.responsibleUser;
   }
 
-  public void setResponsibleUser(final String responsibleUser) {
+  public void setResponsibleUser(final @Nullable String responsibleUser) {
     this.responsibleUser = responsibleUser;
   }
 
-  public Instant getLastUpdated() {
+  public @Nullable Instant getLastUpdated() {
     return lastUpdated;
   }
 
-  public void setLastUpdated(Instant lastUpdated) {
+  public void setLastUpdated(final @Nullable Instant lastUpdated) {
     this.lastUpdated = lastUpdated;
   }
 
-  public Duration getDuration() {
+  public @Nullable Duration getDuration() {
     return duration;
   }
 
-  public void setDuration(Duration duration) {
+  public void setDuration(final @Nullable Duration duration) {
     this.duration = duration;
   }
 
   public void updateDuration() {
     if (this.lastUpdated != null) {
-      final Duration duration = Duration.between(this.lastUpdated, Instant.now());
+      final Duration elapsed = Duration.between(this.lastUpdated, Instant.now());
       if (this.duration == null) {
-        this.duration = duration;
+        this.duration = elapsed;
       } else {
-        this.duration = this.duration.plus(duration);
+        this.duration = this.duration.plus(elapsed);
       }
     }
   }

src/main/java/io/papermc/patchroulette/model/PatchId.java

@@ -6,9 +6,13 @@
 import java.util.Objects;
 
 public class PatchId implements Serializable {
+  private static final long serialVersionUID = 1L;
+
   private String minecraftVersion;
   private String path;
 
+  // Required by JPA; fields are always set via the constructor or by Hibernate reflection.
+  @SuppressWarnings("NullAway")
   public PatchId() {}
 
   public String getMinecraftVersion() {

src/main/java/io/papermc/patchroulette/model/package-info.java

@@ -0,0 +1,4 @@
+@NullMarked
+package io.papermc.patchroulette.model;
+
+import org.jspecify.annotations.NullMarked;

src/main/java/io/papermc/patchroulette/package-info.java

@@ -0,0 +1,4 @@
+@NullMarked
+package io.papermc.patchroulette;
+
+import org.jspecify.annotations.NullMarked;

src/main/java/io/papermc/patchroulette/repository/package-info.java

@@ -0,0 +1,4 @@
+@NullMarked
+package io.papermc.patchroulette.repository;
+
+import org.jspecify.annotations.NullMarked;

src/main/java/io/papermc/patchroulette/service/PatchService.java

@@ -7,6 +7,7 @@
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
@@ -87,7 +88,9 @@ public void finishWorkOnPatch(final PatchId patchId, final String user) {
     if (patch.getStatus() != Status.WIP) {
       throw new IllegalStateException("Patch " + patchId + " is not WIP");
     }
-    if (!patch.getResponsibleUser().equals(user)) {
+    final String responsibleUser = Objects.requireNonNull(
+        patch.getResponsibleUser(), "Patch " + patchId + " has no responsible user");
+    if (!responsibleUser.equals(user)) {
       throw new IllegalStateException("User " + user + " is not responsible for patch " + patchId);
     }
     patch.setStatus(Status.DONE);

src/main/java/io/papermc/patchroulette/service/package-info.java

@@ -0,0 +1,4 @@
+@NullMarked
+package io.papermc.patchroulette.service;
+
+import org.jspecify.annotations.NullMarked;

src/main/java/io/papermc/patchroulette/util/package-info.java

@@ -0,0 +1,4 @@
+@NullMarked
+package io.papermc.patchroulette.util;
+
+import org.jspecify.annotations.NullMarked;