Add and implement JSpecify ADR (#13957)

* Add ADR

* Implement ADR

* Undo unwanted changes

* Remove newline

* Remove newline

* Remove newline

* Update docs/decisions/0051-jspecify-nullable-annotations

Co-authored-by: Subhramit Basu <[email protected]>

* Update docs/decisions/0051-jspecify-nullable-annotations

Co-authored-by: Subhramit Basu <[email protected]>

* Update docs/decisions/0051-jspecify-nullable-annotations

Co-authored-by: Subhramit Basu <[email protected]>

* Update docs/decisions/0051-jspecify-nullable-annotations

Co-authored-by: Subhramit Basu <[email protected]>

* Update docs/decisions/0051-jspecify-nullable-annotations

Co-authored-by: Subhramit Basu <[email protected]>

* Update docs/decisions/0051-jspecify-nullable-annotations

Co-authored-by: Subhramit Basu <[email protected]>

* Update docs/decisions/0051-jspecify-nullable-annotations

Co-authored-by: Subhramit Basu <[email protected]>

* Update docs/decisions/0051-jspecify-nullable-annotations

Co-authored-by: Subhramit Basu <[email protected]>

* Fix refactoring artifacts

* Checkstyle

* Checkstyle

* Fix and add tests

* Fix unused imports

* Remove null tests

* Implement more annotations

* Implement more annotations for consistency

* Remove more null tests

* Fix import and some static analysis null issues

* Remove more null tests

---------

Co-authored-by: Carl Christian Snethlage <[email protected]>
Co-authored-by: Subhramit Basu <[email protected]>

Author: calixtus

.idea/checkstyle-idea.xml

@@ -9,6 +9,7 @@
 //DEPS org.openjfx:javafx-base:24.0.1
 //DEPS org.slf4j:slf4j-api:2.0.13
 //DEPS org.slf4j:slf4j-simple:2.0.13
+//DEPS org.jspecify:jspecify:1.0.0
 
 //SOURCES ../../../../jablib/src/main/java/org/jabref/logic/journals/Abbreviation.java
 //SOURCES ../../../../jablib/src/main/java/org/jabref/logic/journals/AbbreviationFormat.java

build-support/src/main/java/JournalListMvGenerator.java

@@ -7,6 +7,7 @@
 //DEPS info.debatty:java-string-similarity:2.0.0
 //DEPS org.jooq:jool:0.9.14
 //DEPS org.openjfx:javafx-base:24.0.1
+//DEPS org.jspecify:jspecify:1.0.0
 //DEPS org.slf4j:slf4j-api:2.0.13
 //DEPS org.slf4j:slf4j-simple:2.0.13
 

build-support/src/main/java/LtwaListMvGenerator.java

@@ -68,8 +68,8 @@ public String getHeading() {
 * Create constructor which initializes the fields to their default values. Write tests to ensure that everything works as expected!
 
 ```java
-public MyDialogViewModel(Dependency dependency) {
-    this.dependency = Objects.requireNonNull(dependency);
+public MyDialogViewModel(@NonNull Dependency dependency) {
+    this.dependency = dependency;
     heading.set("Hello " + dependency.getUserName());
 }
 ```

docs/code-howtos/javafx.md

@@ -0,0 +1,79 @@
+---
+title: Adopt JSpecify nullness annotations for compile-time null safety
+nav_order: 51
+parent: Decision Records
+status: pending
+---
+
+# Adopt JSpecify nullness annotations for compile-time null safety
+
+## Context and Problem Statement
+
+Our Java codebase contains inconsistent handling of nullability.
+We want to detect null-safety issues earlier at compile time and improve API clarity.
+
+## Decision Drivers
+
+* Reduce `NullPointerException`s in production by shifting detection to compile time.
+* Provide clearer API contracts.
+* Keep runtime overhead low.
+* Ensure incremental and low-risk adoption.
+
+## Considered Options
+
+* Do nothing and keep status quo.
+* Use `Objects.requireNonNull` and defensive runtime checks.
+* Adopt JSpecify annotations.
+* Use `assert x == null`.
+
+## Decision Outcome
+
+Chosen option: "Adopt JSpecify annotations" because comes out best (see below).
+
+## Consequences
+
+* Earlier detection of potential NPEs.
+* Clearer API documentation.
+* Requires training and CI integration.
+* Some friction with unannotated third-party libraries.
+
+## Pros and Cons of the Options
+
+### Do nothing and keep status quo
+
+* Good, because no immediate implementation work.
+* Bad, because NPEs remain runtime-only problems.
+* Bad, because a mix of different approaches leads to bad code.
+* Bad, because it increases technical debt.
+* Bad, because assumes that non-annotated symbols allow null.
+
+### Use `Objects::requireNonNull`
+
+* Good, because JDK native.
+* Good, because no external dependencies.
+* Good, because it makes NPEs more visible on runtime.
+* Bad, because it adds runtime overhead.
+* Bad, because it does not provide compile-time contracts.
+* Bad, because it is not self-documenting for API contract.
+
+### Adopt JSpecify annotations
+
+* Good, because it offers compile-time null safety detection.
+* Good, because it works well with common IDEs.
+* Good, because of standardized annotations.
+* Good, because incremental adoption possible.
+* Good, because static analysis supported.
+* Good, because compatibility with Kotlin.
+* Good, because it is the consensus among major organizations (Google, Microsoft, Jetbrains etc).
+* Bad, because it requires annotation effort.
+* Bad, because it requires developer training.
+
+### Use `assert x == null`
+
+* Good, because Java language native.
+* Good, because no external dependencies.
+* Good, because easily readable.
+* Good, because it makes NPEs more visible on runtime.
+* Bad, because runs by default only in debug mode with option "-ea".
+* Bad, because it does not provide compile-time null safety.
+* Bad, because it is not self-documenting for API contract.

docs/decisions/0051-jspecify-nullable-annotations

@@ -2,7 +2,6 @@
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Function;
 
@@ -45,6 +44,7 @@
 import com.tobiasdiez.easybind.EasyBind;
 import com.tobiasdiez.easybind.EasyBinding;
 import com.tobiasdiez.easybind.PreboundBinding;
+import org.jspecify.annotations.NonNull;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -144,8 +144,7 @@ public void setSelectedEntries(List<BibEntry> newSelectedEntries) {
     }
 
     @Override
-    public void setSelectedGroups(BibDatabaseContext context, List<GroupTreeNode> newSelectedGroups) {
-        Objects.requireNonNull(newSelectedGroups);
+    public void setSelectedGroups(BibDatabaseContext context, @NonNull List<GroupTreeNode> newSelectedGroups) {
         selectedGroups.computeIfAbsent(context.getUid(), k -> FXCollections.observableArrayList()).setAll(newSelectedGroups);
     }
 

jabgui/src/main/java/org/jabref/gui/JabRefGuiStateManager.java

@@ -3,7 +3,6 @@
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.List;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.Random;
 import java.util.stream.Collectors;
@@ -175,10 +174,10 @@ public class LibraryTab extends Tab implements CommandSelectionTab {
      */
     private LibraryTab(@NonNull BibDatabaseContext bibDatabaseContext,
                        @NonNull LibraryTabContainer tabContainer,
-                       DialogService dialogService,
+                       @NonNull DialogService dialogService,
                        AiService aiService,
-                       GuiPreferences preferences,
-                       StateManager stateManager,
+                       @NonNull GuiPreferences preferences,
+                       @NonNull StateManager stateManager,
                        FileUpdateMonitor fileUpdateMonitor,
                        BibEntryTypesManager entryTypesManager,
                        CountingUndoManager undoManager,
@@ -189,8 +188,8 @@ private LibraryTab(@NonNull BibDatabaseContext bibDatabaseContext,
         this.tabContainer = tabContainer;
         this.undoManager = undoManager;
         this.dialogService = dialogService;
-        this.preferences = Objects.requireNonNull(preferences);
-        this.stateManager = Objects.requireNonNull(stateManager);
+        this.preferences = preferences;
+        this.stateManager = stateManager;
         assert bibDatabaseContext.getDatabasePath().isEmpty() || fileUpdateMonitor != null;
         this.fileUpdateMonitor = fileUpdateMonitor;
         this.entryTypesManager = entryTypesManager;

jabgui/src/main/java/org/jabref/gui/LibraryTab.java

@@ -6,7 +6,6 @@
 
 import org.jabref.model.database.BibDatabaseContext;
 
-import org.jspecify.annotations.NonNull;
 import org.jspecify.annotations.NullMarked;
 import org.jspecify.annotations.Nullable;
 
@@ -31,7 +30,7 @@ public interface LibraryTabContainer {
      */
     boolean closeTab(@Nullable LibraryTab tab);
 
-    boolean closeTabs(@NonNull List<LibraryTab> tabs);
+    boolean closeTabs(List<LibraryTab> tabs);
 
     /**
      * Refreshes the ui after changes to the preferences

jabgui/src/main/java/org/jabref/gui/LibraryTabContainer.java

@@ -1,13 +1,14 @@
 package org.jabref.gui.actions;
 
-import java.util.Objects;
 import java.util.Optional;
 
 import org.jabref.gui.icon.IconTheme;
 import org.jabref.gui.icon.JabRefIcon;
 import org.jabref.gui.keyboard.KeyBinding;
 import org.jabref.logic.l10n.Localization;
 
+import org.jspecify.annotations.NonNull;
+
 public enum StandardActions implements Action {
 
     COPY_TO(Localization.lang("Copy to")),
@@ -312,8 +313,8 @@ public String getDescription() {
         return description;
     }
 
-    public Action withText(String text) {
-        this.text = Objects.requireNonNull(text);
+    public Action withText(@NonNull String text) {
+        this.text = text;
         return this;
     }
 }

jabgui/src/main/java/org/jabref/gui/actions/StandardActions.java

@@ -1,11 +1,12 @@
 package org.jabref.gui.autocompleter;
 
-import java.util.Objects;
 import java.util.stream.Stream;
 
 import org.jabref.model.database.BibDatabase;
 import org.jabref.model.entry.field.Field;
 
+import org.jspecify.annotations.NonNull;
+
 /**
  * Stores the full content of one field.
  */
@@ -14,8 +15,8 @@ class FieldValueSuggestionProvider extends StringSuggestionProvider {
     private final Field field;
     private final BibDatabase database;
 
-    FieldValueSuggestionProvider(Field field, BibDatabase database) {
-        this.field = Objects.requireNonNull(field);
+    FieldValueSuggestionProvider(@NonNull Field field, @NonNull BibDatabase database) {
+        this.field = field;
         this.database = database;
     }