From fcc12c7a935f17ce19ca9d5c4b8d3e87526e44ba Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:26:17 -0700 Subject: [PATCH] Add JSpecify null-safety enforcement with Error Prone + NullAway - Wire net.ltgt.errorprone 5.1.0 + net.ltgt.nullaway 3.1.0 with Spring's recommended config: only NullAway enabled, OnlyNullMarked, JSpecifyMode, RequireExplicitNullMarking, Spring @Contract support - Add org.jspecify:jspecify (Boot BOM-managed) and @NullMarked package-info for every package - Mark genuinely-nullable entity fields @Nullable to match the DB schema - Fix latent NPE in PatchService.finishWorkOnPatch; use requireNonNull where invariants are provable - Add serialVersionUID to PatchId --- build.gradle.kts | 23 +++++++++++ .../patchroulette/config/package-info.java | 4 ++ .../controller/ApiController.java | 40 ++++++++++++------- .../controller/package-info.java | 4 ++ .../io/papermc/patchroulette/model/Patch.java | 24 ++++++----- .../papermc/patchroulette/model/PatchId.java | 4 ++ .../patchroulette/model/package-info.java | 4 ++ .../papermc/patchroulette/package-info.java | 4 ++ .../repository/package-info.java | 4 ++ .../patchroulette/service/PatchService.java | 5 ++- .../patchroulette/service/package-info.java | 4 ++ .../patchroulette/util/package-info.java | 4 ++ 12 files changed, 99 insertions(+), 25 deletions(-) create mode 100644 src/main/java/io/papermc/patchroulette/config/package-info.java create mode 100644 src/main/java/io/papermc/patchroulette/controller/package-info.java create mode 100644 src/main/java/io/papermc/patchroulette/model/package-info.java create mode 100644 src/main/java/io/papermc/patchroulette/package-info.java create mode 100644 src/main/java/io/papermc/patchroulette/repository/package-info.java create mode 100644 src/main/java/io/papermc/patchroulette/service/package-info.java create mode 100644 src/main/java/io/papermc/patchroulette/util/package-info.java diff --git a/build.gradle.kts b/build.gradle.kts index 749de1d..30548a7 100644 --- a/build.gradle.kts +++ b/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().configureEach { + options.errorprone { + disableAllChecks = true // Only NullAway is enabled + error("RequireExplicitNullMarking") + nullaway { + error() + } + option("NullAway:CustomContractAnnotations", "org.springframework.lang.Contract") + } } buildscript { diff --git a/src/main/java/io/papermc/patchroulette/config/package-info.java b/src/main/java/io/papermc/patchroulette/config/package-info.java new file mode 100644 index 0000000..172cc24 --- /dev/null +++ b/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; diff --git a/src/main/java/io/papermc/patchroulette/controller/ApiController.java b/src/main/java/io/papermc/patchroulette/controller/ApiController.java index 9e84407..c275773 100644 --- a/src/main/java/io/papermc/patchroulette/controller/ApiController.java +++ b/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> 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(@RequestParam final String minecraftVersion) final Map users = new HashMap<>(); // Track intervals for each user - Map> userIntervals = new HashMap<>(); + final Map> 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(@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); diff --git a/src/main/java/io/papermc/patchroulette/controller/package-info.java b/src/main/java/io/papermc/patchroulette/controller/package-info.java new file mode 100644 index 0000000..2161327 --- /dev/null +++ b/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; diff --git a/src/main/java/io/papermc/patchroulette/model/Patch.java b/src/main/java/io/papermc/patchroulette/model/Patch.java index 1c25b0b..d492518 100644 --- a/src/main/java/io/papermc/patchroulette/model/Patch.java +++ b/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); } } } diff --git a/src/main/java/io/papermc/patchroulette/model/PatchId.java b/src/main/java/io/papermc/patchroulette/model/PatchId.java index 166118b..ac09807 100644 --- a/src/main/java/io/papermc/patchroulette/model/PatchId.java +++ b/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() { diff --git a/src/main/java/io/papermc/patchroulette/model/package-info.java b/src/main/java/io/papermc/patchroulette/model/package-info.java new file mode 100644 index 0000000..f016def --- /dev/null +++ b/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; diff --git a/src/main/java/io/papermc/patchroulette/package-info.java b/src/main/java/io/papermc/patchroulette/package-info.java new file mode 100644 index 0000000..30e3ede --- /dev/null +++ b/src/main/java/io/papermc/patchroulette/package-info.java @@ -0,0 +1,4 @@ +@NullMarked +package io.papermc.patchroulette; + +import org.jspecify.annotations.NullMarked; diff --git a/src/main/java/io/papermc/patchroulette/repository/package-info.java b/src/main/java/io/papermc/patchroulette/repository/package-info.java new file mode 100644 index 0000000..4b63f0c --- /dev/null +++ b/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; diff --git a/src/main/java/io/papermc/patchroulette/service/PatchService.java b/src/main/java/io/papermc/patchroulette/service/PatchService.java index c2022fe..5dd52b5 100644 --- a/src/main/java/io/papermc/patchroulette/service/PatchService.java +++ b/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); diff --git a/src/main/java/io/papermc/patchroulette/service/package-info.java b/src/main/java/io/papermc/patchroulette/service/package-info.java new file mode 100644 index 0000000..fac0f9f --- /dev/null +++ b/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; diff --git a/src/main/java/io/papermc/patchroulette/util/package-info.java b/src/main/java/io/papermc/patchroulette/util/package-info.java new file mode 100644 index 0000000..5b26cf2 --- /dev/null +++ b/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;