Fix: suppress WARN for empty or blank column name in MainTableColumnModel#parse()

[Afra0704]

Related issues and pull requests

Closes #15571

PR Description

When MainTableColumnModel.parse() was called with an empty or blank string (from stored preferences), it triggered a WARN: Column type '' is unknown. message repeatedly. This fix adds a guard clause in parse() to return a default NORMALFIELD column model early when the input is empty or blank, preventing the unnecessary warning. isBlank() is used instead of isEmpty() to also handle whitespace-only strings, which are equally invalid as column names.

Steps to test

1. Run the existing JUnit tests in MainTableColumnModelTest
2. Verify that emptyStringShouldReturnNormalFieldWithEmptyQualifier and blankStringShouldReturnNormalFieldWithEmptyQualifier both pass
3. No WARN: Column type '' is unknown. should appear in the console

Checklist

• 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 added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
• I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
• [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Suppress warning for empty or blank column names in MainTableColumnModel

🐞 Bug fix

Walkthroughs

Description

• Add null/blank check in MainTableColumnModel.parse() to prevent warnings
• Return default NORMALFIELD type for empty or whitespace-only column names
• Add unit tests for empty and blank string inputs
• Update CHANGELOG documenting the fix

Diagram

flowchart LR
  A["parse() receives empty/blank string"] -->|null or isBlank check| B["Return NORMALFIELD with empty qualifier"]
  C["parse() receives valid string"] -->|normal flow| D["Split and process column name"]
  B --> E["No WARN message logged"]
  D --> F["Type determined from string"]

Loading

File Changes

1. jabgui/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java 🐞 Bug fix +4/-0

Add null/blank check in parse method

• Add guard clause to check if rawColumnName is null or blank
• Return early with default NORMALFIELD type and empty qualifier
• Prevents unnecessary warning message for invalid column names

jabgui/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java

---

2. jabgui/src/test/java/org/jabref/gui/maintable/MainTableColumnModelTest.java 🧪 Tests +16/-0

Add unit tests for empty/blank inputs

• Add test for empty string input to verify NORMALFIELD type is returned
• Add test for whitespace-only string input with same expectations
• Both tests verify empty qualifier is set correctly

jabgui/src/test/java/org/jabref/gui/maintable/MainTableColumnModelTest.java

---

3. CHANGELOG.md 📝 Documentation +1/-0

Document fix in changelog

• Add entry documenting the fix for issue #15571
• Note that warning is no longer printed for empty column names

CHANGELOG.md

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. parse() accepts null unannotated ☑ 📘 Rule violation ≡ Correctness

Description

MainTableColumnModel is @NullMarked, but parse now explicitly handles null without marking
the parameter as nullable, making the API nullability contract unclear/inconsistent. This can
mislead callers and static analysis and hides null-safety issues behind a silent default.

Code

jabgui/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java[R234-236]

+        if (rawColumnName == null || rawColumnName.isBlank()) {
+            return new MainTableColumnModel(Type.NORMALFIELD, "");
+        }

Evidence

The class is annotated with @NullMarked (non-null by default), but the newly added guard clause
treats rawColumnName as potentially null without adding a JSpecify nullability annotation on the
parameter, violating the requirement to use explicit nullability via JSpecify for public APIs.

AGENTS.md
jabgui/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java[31-31]
jabgui/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java[233-236]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`MainTableColumnModel` is `@NullMarked`, but `parse(String rawColumnName)` now checks for `null` (`rawColumnName == null || rawColumnName.isBlank()`), which implicitly widens the API to accept null while keeping a non-null signature.
## Issue Context
Under JSpecify `@NullMarked`, parameters are non-null by default. If `null` is a supported input, the parameter must be explicitly annotated `@Nullable`; otherwise the null-handling should be removed.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java[233-236]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Null column list entry ☑ 🐞 Bug ☼ Reliability

Description

parse() returns NORMALFIELD with an empty qualifier for blank input, but
MainTableColumnFactory.createColumn() returns null for NORMALFIELD when the qualifier is blank and
createColumns() adds that null into the returned list. This can later cause NullPointerExceptions
when MainTable streams over its columns and dereferences them (e.g., while applying saved sort
order).

Code

jabgui/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java[R234-236]

+        if (rawColumnName == null || rawColumnName.isBlank()) {
+            return new MainTableColumnModel(Type.NORMALFIELD, "");
+        }

Evidence

The new guard clause makes blank inputs produce a model with blank qualifier. The column factory
explicitly refuses to create a column for NORMALFIELD with blank qualifier (returns null), and
createColumns() adds the result without filtering. MainTable later streams over the table columns
and calls methods on each column, which will throw if any element is null.

jabgui/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java[233-254]
jabgui/src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java[87-146]
jabgui/src/main/java/org/jabref/gui/maintable/MainTable.java[187-195]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When `parse()` returns a `NORMALFIELD` column model with a blank qualifier, `MainTableColumnFactory.createColumn()` returns `null`, and `createColumns()` currently adds that `null` to the list. A `null` column can later trigger NPEs when code iterates/dereferences table columns.
### Issue Context
This PR intentionally suppresses warnings for blank column names, so invalid persisted values may still occur; the UI code should be robust against such invalid entries.
### Fix Focus Areas
- Ensure `MainTableColumnFactory.createColumns()` never returns a list containing `null` (e.g., filter out nulls, optionally log once when skipping).
- Consider additionally skipping/cleaning blank column names when loading preferences.
- jabgui/src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java[140-146]
- jabgui/src/main/java/org/jabref/gui/preferences/JabRefGuiPreferences.java[1041-1074]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[Afra0704]

Thanks for the review! I've addressed both issues:

1. Removed the null check to stay consistent with @NullMarked
2. Added filter(Objects::nonNull) in MainTableColumnFactory.createColumns() to prevent null columns from being added to the list

[Maran23]

This filter call is new. Why was it added?

[Afra0704]

createColumn() returns null for NORMALFIELD with blank qualifier.
Without this filter, the null would be added to the column list and
cause a NullPointerException later.

[Maran23]

Okay but what I'm wondering: why was this not an issue before?

[Afra0704]

This was originally pointed out by Qodo AI, which suggested that createColumn() may return null for this case and that filtering it out would be a reasonable defensive measure.

That said, I'm not entirely sure why this did not cause problems before. My guess is that the previous behavior did not exercise this path in practice, or that the resulting null entry was not dereferenced later.

So the filter may not be strictly necessary to fix the original WARN itself, but I kept it as a defensive fix to avoid returning null entries from createColumns(). If you'd prefer to keep this PR focused only on the WARN, I can remove it.

[Maran23]

Okay, so null is not returned right now, this is just a defensive measure?

Probably fine, but I will let the others decide. But that is good to know, because at first I thought we now throw a NPE but did not before, which sounds like a regression and is not expected behavior.

The Behavior should be the same as before, just without the warning. Thats why I asked.

[calixtus]

If I remember correctly, in default circumstances columnPreferences.getColumns always delivered a list of valid values or an empty list. So here Objects::nonNull is not allowed, we opted for compile time null safety with JSpecify annotations and nullaway in ADRs 52 and 53. Solution is not to filter, but to annotate createColumn accordingly.

[Afra0704]

Thank you for the guidance! I have removed the filter and annotated createColumn()
with @Nullable instead, following JabRef's JSpecify approach.

[calixtus]

Adapt to ADR 52 https://devdocs.jabref.org/decisions/0052-jspecify-nullable-annotations.html

[calixtus]

No this is wrong. This is guaranteed to be nonnull. Always an ArrayList.

[calixtus]

createColumn (without the s) must be annotated and if neccessary modified. Maybe a good place to do some Data Oriented Programming?

[Afra0704]

Sorry for the mistake! I have moved the @Nullable annotation to createColumn()
instead of createColumns().

[calixtus]

createColumn (without the s) must be annotated and if neccessary modified. Maybe a good place to do some Data Oriented Programming?

Maybe this would escalate this too much.

Nevertheless, by marking it Nullable this will result in a compiler warning for now, which is ok, until we eventually fix all nullaway warnings. But this issue is tracked now.

[github-actions]

Your pull request conflicts with the target branch.

Please merge with your code. For a step-by-step guide to resolve merge conflicts, see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line.