Add and implement JSpecify ADR by calixtus · Pull Request #13957 · JabRef/jabref

calixtus · 2025-09-22T22:48:53Z

Selfexplaining Title

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
[.] I manually tested my changes in running JabRef (always required)
[.] I added JUnit tests for changes (if applicable)
[.] I added screenshots in the PR description (if change is visible to the user)
[.] I described the change in CHANGELOG.md in a way that is understandable for the average user (if change is visible to the user)
[.] I checked the user documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request updating file(s) in https://github.com/JabRef/user-documentation/tree/main/en.

# Conflicts: # jabls/src/main/java/module-info.java

subhramit

Very minor refinements on ADR

Co-authored-by: Subhramit Basu <subhramit.bb@live.in>

subhramit

ADR ✅

JabRef/jabref#13957 jabls/src/main/java/module-info.java

trag-bot · 2025-09-24T12:19:27Z

@trag-bot didn't find any issues in the code! ✅✨

subhramit · 2025-09-25T07:27:01Z

    @Override
-    public void setSelectedGroups(BibDatabaseContext context, List<GroupTreeNode> newSelectedGroups) {
-        Objects.requireNonNull(newSelectedGroups);
+    public void setSelectedGroups(BibDatabaseContext context, @NonNull List<GroupTreeNode> newSelectedGroups) {


is context nullable?

Can always be changed. My method was only: monkey see, monkey do.
Change was made bc of change in interface. Can be discussed in static analysis followup pr.

In case all nullable variables are marked @Nullable, one can mark the whole class as @NullMarked and can remove all the @NonNull annotations.

subhramit · 2025-09-25T07:27:41Z

+    PersonNameSuggestionProvider(@NonNull Field field, BibDatabase database) {
+        this(List.of(field), database);
    }

-    public PersonNameSuggestionProvider(Collection<Field> fields, BibDatabase database) {
+    public PersonNameSuggestionProvider(@NonNull Collection<Field> fields, BibDatabase database) {


similar qn, could database be null

subhramit · 2025-09-25T07:28:03Z


-    public WordSuggestionProvider(Field field, BibDatabase database) {
-        this.field = Objects.requireNonNull(field);
+    public WordSuggestionProvider(@NonNull Field field, BibDatabase database) {


here as well

(i am asking because I see @NonNull BibDatabaseContext databaseContext in some classes below)

subhramit

I checked the CitationStyle classes, looks good.
Some questions though^

* 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 <subhramit.bb@live.in> * Update docs/decisions/0051-jspecify-nullable-annotations Co-authored-by: Subhramit Basu <subhramit.bb@live.in> * Update docs/decisions/0051-jspecify-nullable-annotations Co-authored-by: Subhramit Basu <subhramit.bb@live.in> * Update docs/decisions/0051-jspecify-nullable-annotations Co-authored-by: Subhramit Basu <subhramit.bb@live.in> * Update docs/decisions/0051-jspecify-nullable-annotations Co-authored-by: Subhramit Basu <subhramit.bb@live.in> * Update docs/decisions/0051-jspecify-nullable-annotations Co-authored-by: Subhramit Basu <subhramit.bb@live.in> * Update docs/decisions/0051-jspecify-nullable-annotations Co-authored-by: Subhramit Basu <subhramit.bb@live.in> * Update docs/decisions/0051-jspecify-nullable-annotations Co-authored-by: Subhramit Basu <subhramit.bb@live.in> * 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 <calixtus@users.noreply.github.com> Co-authored-by: Subhramit Basu <subhramit.bb@live.in>

* Add cleanup dialog tabs with individual tab preferences * Fixed indentation and added commenting * Fix Trag-bot review issues - Removed trivial comments - Renamed PDF-related variables - Updated methods to return Optional - Used Optional property for FieldFormatterCleanups * Fix Trag-bot review issues - Removed trivial comments - Fixed CHANGELOG.md * Fix Trag-bot review issues - Removed trivial comments - Improved Optional checks - Perform null check for other parameters * Avoid nested Optionals, returning Optional<CleanupPreferences> directly * Refactor CleanupPreferences by keeping one assertion per test * Converted tests to assertEquals * Maintain consistent naming conventions * Returns CleanupPreferences directly since value is always present * Initial review refactor draft - Create new ViewModel, pull logic from Action and SingleAction into ViewModel - Move Apply button to each tab - Remove categories from ENUM and keep enums of all jobs in each respective tab to be used for cleanup * fix import error! * Reformat codebase (more carefully) (#13885) * Fix non record comments by carl # Conflicts: # jabgui/src/main/java/org/jabref/gui/edit/automaticfiededitor/MoveFieldValueAction.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/cell/sidebuttons/ToggleMergeUnmergeButton.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/CommentMerger.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/FieldMerger.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/FileMerger.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/GroupMerger.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/KeywordMerger.java # jablib/src/main/java/org/jabref/logic/layout/format/HTMLChars.java # jablib/src/main/java/org/jabref/model/entry/identifier/ArXivIdentifier.java # jablib/src/main/java/org/jabref/model/entry/identifier/EprintIdentifier.java * Add file exceptions * Remove shebang line * Remove shebang line * Remove shebang line * Expand variables & rename class --------- Co-authored-by: Oliver Kopp <kopp.dev@gmail.com> * fix import error & merge * Apply OpenRewrite Cleanup * Refactor Cleanup Tabs - Moved cleanup panel logic into CleanupDialogViewModel for better separation of UI and logic - Changed tabSupplier and taskExecutor from Optional to nullable parameters - Moved updateWith logic into the record for cleaner preference updates. - General design improvements: more maintainable. * Fix getDisplayName method * Fix formatting * Trag-bot review and fix en properties * fix indentation plssss * format properly and change to observablelist * fix formatting entriestoprocess (please) * Updated names and changed optional dependencies back to nullable * Refactored panels to use separate ViewModels - Introduced ViewModels to encapsulate state and logic for panels. - Replaced direct UI manipulation with bidirectional bindings. - Ensures cleaner UI logic, easier maintenance * Moved ALL_JOBS to respective ViewModels, small naming changes * Replaced requireNotNull to @NotNull following #13957 * Address review feedback in CleanupDialogViewModel - Remove redundant comments following self-explanatory code - Add modifiedEntriesCount > 0 condition - Use "entry(s)" localization form for clean up message --------- Co-authored-by: Subhramit Basu <subhramit.bb@live.in> Co-authored-by: Oliver Kopp <kopp.dev@gmail.com> Co-authored-by: Christoph <siedlerkiller@gmail.com>

* Add cleanup dialog tabs with individual tab preferences * Fixed indentation and added commenting * Fix Trag-bot review issues - Removed trivial comments - Renamed PDF-related variables - Updated methods to return Optional - Used Optional property for FieldFormatterCleanups * Fix Trag-bot review issues - Removed trivial comments - Fixed CHANGELOG.md * Fix Trag-bot review issues - Removed trivial comments - Improved Optional checks - Perform null check for other parameters * Avoid nested Optionals, returning Optional<CleanupPreferences> directly * Refactor CleanupPreferences by keeping one assertion per test * Converted tests to assertEquals * Maintain consistent naming conventions * Returns CleanupPreferences directly since value is always present * Initial review refactor draft - Create new ViewModel, pull logic from Action and SingleAction into ViewModel - Move Apply button to each tab - Remove categories from ENUM and keep enums of all jobs in each respective tab to be used for cleanup * fix import error! * Reformat codebase (more carefully) (JabRef#13885) * Fix non record comments by carl # Conflicts: # jabgui/src/main/java/org/jabref/gui/edit/automaticfiededitor/MoveFieldValueAction.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/cell/sidebuttons/ToggleMergeUnmergeButton.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/CommentMerger.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/FieldMerger.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/FileMerger.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/GroupMerger.java # jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/KeywordMerger.java # jablib/src/main/java/org/jabref/logic/layout/format/HTMLChars.java # jablib/src/main/java/org/jabref/model/entry/identifier/ArXivIdentifier.java # jablib/src/main/java/org/jabref/model/entry/identifier/EprintIdentifier.java * Add file exceptions * Remove shebang line * Remove shebang line * Remove shebang line * Expand variables & rename class --------- Co-authored-by: Oliver Kopp <kopp.dev@gmail.com> * fix import error & merge * Apply OpenRewrite Cleanup * Refactor Cleanup Tabs - Moved cleanup panel logic into CleanupDialogViewModel for better separation of UI and logic - Changed tabSupplier and taskExecutor from Optional to nullable parameters - Moved updateWith logic into the record for cleaner preference updates. - General design improvements: more maintainable. * Fix getDisplayName method * Fix formatting * Trag-bot review and fix en properties * fix indentation plssss * format properly and change to observablelist * fix formatting entriestoprocess (please) * Updated names and changed optional dependencies back to nullable * Refactored panels to use separate ViewModels - Introduced ViewModels to encapsulate state and logic for panels. - Replaced direct UI manipulation with bidirectional bindings. - Ensures cleaner UI logic, easier maintenance * Moved ALL_JOBS to respective ViewModels, small naming changes * Replaced requireNotNull to @NotNull following JabRef#13957 * Address review feedback in CleanupDialogViewModel - Remove redundant comments following self-explanatory code - Add modifiedEntriesCount > 0 condition - Use "entry(s)" localization form for clean up message --------- Co-authored-by: Subhramit Basu <subhramit.bb@live.in> Co-authored-by: Oliver Kopp <kopp.dev@gmail.com> Co-authored-by: Christoph <siedlerkiller@gmail.com>

JabRef/jabref#13957 jabls/src/main/java/module-info.java

calixtus added 2 commits September 22, 2025 20:59

Add ADR

ba62974

Implement ADR

d3a41fc

calixtus added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers dev: code-quality Issues related to code or architecture decisions dev: adr labels Sep 22, 2025

calixtus and others added 5 commits September 23, 2025 00:50

Undo unwanted changes

fc16fa0

Remove newline

51070ba

Remove newline

b2374b0

Remove newline

a9aab40

Merge remote-tracking branch 'upstream/main' into jspecify-adr

3c4e2e4

# Conflicts: # jabls/src/main/java/module-info.java

subhramit reviewed Sep 22, 2025

View reviewed changes

calixtus and others added 8 commits September 23, 2025 07:20

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

3294da5

Co-authored-by: Subhramit Basu <subhramit.bb@live.in>

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

6ed3665

Co-authored-by: Subhramit Basu <subhramit.bb@live.in>

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

ca8fa71

Co-authored-by: Subhramit Basu <subhramit.bb@live.in>

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

ecd53cd

Co-authored-by: Subhramit Basu <subhramit.bb@live.in>

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

e3c724b

Co-authored-by: Subhramit Basu <subhramit.bb@live.in>

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

5c13a2d

Co-authored-by: Subhramit Basu <subhramit.bb@live.in>

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

c1ce718

Co-authored-by: Subhramit Basu <subhramit.bb@live.in>

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

9844883

Co-authored-by: Subhramit Basu <subhramit.bb@live.in>

calixtus requested a review from subhramit September 23, 2025 05:22

subhramit previously approved these changes Sep 23, 2025

View reviewed changes

calixtus added 2 commits September 23, 2025 11:52

Fix refactoring artifacts

051e4f4

Merge remote-tracking branch 'upstream/jspecify-adr' into jspecify-adr

3dcf18d

calixtus dismissed subhramit’s stale review via 3dcf18d September 23, 2025 09:52

calixtus added 2 commits September 23, 2025 11:54

Merge branch 'main' into jspecify-adr

905a762

Checkstyle

ca02db9