Skip to content

fix: resolve static analysis issues in code #13270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

fix: resolve static analysis issues in code #13270

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
cfc3361
fix: resolve static analysis issues in code
ungerts Jun 6, 2025
0ff44ac
fix: resolve checkstyle violations and improve code quality
ungerts Jun 6, 2025
b844e0f
fix: resolve issues in LocalizationConsistencyCheck
ungerts Jun 6, 2025
ff17d38
Revert, misc
subhramit Jun 18, 2025
4d80b22
More underscore
subhramit Jun 18, 2025
b6946a1
Fix raw use of list
subhramit Jun 18, 2025
f37e553
Use peek
subhramit Jun 18, 2025
a4e3b92
Merge branch 'main' into feature/static-analysis-fixes
subhramit Jun 18, 2025
521164a
Missing import
subhramit Jun 18, 2025
0ef1159
Lost commit
subhramit Jun 18, 2025
6681865
Overlap error code changes with changes of this branch
subhramit Jun 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jabgui/src/main/java/org/jabref/Launcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public static void initLogging(String[] args) {

// We must configure logging as soon as possible, which is why we cannot wait for the usual
// argument parsing workflow to parse logging options e.g. --debug
boolean isDebugEnabled = Arrays.stream(args).anyMatch(arg -> "--debug".equalsIgnoreCase(arg));
boolean isDebugEnabled = Arrays.stream(args).anyMatch("--debug"::equalsIgnoreCase);

// addLogToDisk
// We cannot use `Injector.instantiateModelOrService(BuildInfo.class).version` here, because this initializes logging
Expand Down
10 changes: 3 additions & 7 deletions jabgui/src/main/java/org/jabref/gui/autocompleter/PersonNameStringConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ public PersonNameStringConverter(AutoCompletePreferences preferences) {
autoCompFF = false;
autoCompLF = true;
break;
default:
case BOTH:
default:
autoCompFF = true;
autoCompLF = true;
break;
Expand All @@ -42,24 +42,20 @@ public PersonNameStringConverter(AutoCompletePreferences preferences) {
public String toString(Author author) {
if (autoCompLF) {
switch (autoCompleteFirstNameMode) {
case ONLY_ABBREVIATED:
case ONLY_ABBREVIATED, BOTH:
return author.getFamilyGiven(true);
case ONLY_FULL:
return author.getFamilyGiven(false);
case BOTH:
return author.getFamilyGiven(true);
default:
break;
}
}
if (autoCompFF) {
switch (autoCompleteFirstNameMode) {
case ONLY_ABBREVIATED:
case ONLY_ABBREVIATED, BOTH:
return author.getGivenFamily(true);
case ONLY_FULL:
return author.getGivenFamily(false);
case BOTH:
return author.getGivenFamily(true);
default:
break;
}
Expand Down
10 changes: 4 additions & 6 deletions jabgui/src/main/java/org/jabref/gui/commonfxcontrols/CitationKeyPatternSuggestionCell.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,14 @@ public CitationKeyPatternSuggestionTextField(List<String> citationKeyPatterns) {
}

private void setListener() {
textProperty().addListener((observable, oldValue, newValue) -> {
textProperty().addListener((_, _, _) -> {
String enteredText = getText();
if (enteredText == null || enteredText.isEmpty()) {
suggestionsList.hide();
} else {
List<String> filteredEntries = citationKeyPatterns.stream()
.filter(e -> e.toLowerCase().contains(enteredText.toLowerCase()))
.collect(Collectors.toList());
.toList();

if (!filteredEntries.isEmpty()) {
populatePopup(filteredEntries);
Expand All @@ -96,9 +96,7 @@ private void setListener() {
}
});

focusedProperty().addListener((observable, oldValue, newValue) -> {
suggestionsList.hide();
});
focusedProperty().addListener((_, _, _) -> suggestionsList.hide());
}

private void populatePopup(List<String> searchResult) {
Expand All @@ -119,7 +117,7 @@ private void populatePopup(List<String> searchResult) {

menuItems.add(item);

item.setOnAction(actionEvent -> {
item.setOnAction(_ -> {
setText(result);
positionCaret(result.length());
suggestionsList.hide();
Expand Down
9 changes: 4 additions & 5 deletions jabgui/src/main/java/org/jabref/gui/consistency/ConsistencyCheckDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,10 @@ public void initialize() {
message.message().getFirst().equals(viewModel.selectedEntryTypeProperty().get())
);

viewModel.selectedEntryTypeProperty().addListener((obs, oldValue, newValue) -> {
filteredData.setPredicate(message ->
message.message().getFirst().equals(newValue)
);
});
viewModel.selectedEntryTypeProperty().addListener((_, _, newValue) ->
filteredData.setPredicate(message ->
message.message().getFirst().equals(newValue)
));

tableView.setItems(filteredData);

Expand Down
24 changes: 11 additions & 13 deletions jabgui/src/main/java/org/jabref/gui/consistency/ConsistencyCheckDialogViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@

public class ConsistencyCheckDialogViewModel extends AbstractViewModel {

private final Logger LOGGER = LoggerFactory.getLogger(ConsistencyCheckDialogViewModel.class);
private static final Logger LOGGER = LoggerFactory.getLogger(ConsistencyCheckDialogViewModel.class);
private static final int EXTRA_COLUMNS_COUNT = 2;

private final BibliographyConsistencyCheck.Result result;
private final DialogService dialogService;
Expand All @@ -54,7 +55,6 @@ public class ConsistencyCheckDialogViewModel extends AbstractViewModel {

private final List<Field> allReportedFields;
private final int columnCount;
private final int EXTRA_COLUMNS_COUNT = 2;
private final ObservableList<ConsistencyMessage> tableData = FXCollections.observableArrayList();
private final StringProperty selectedEntryType = new SimpleStringProperty();

Expand Down Expand Up @@ -85,9 +85,7 @@ public StringProperty selectedEntryTypeProperty() {

public List<String> getEntryTypes() {
List<String> entryTypes = new ArrayList<>();
result.entryTypeToResultMap().forEach((entrySet, entryTypeResult) -> {
entryTypes.add(entrySet.toString());
});
result.entryTypeToResultMap().forEach((entrySet, _) -> entryTypes.add(entrySet.toString()));
return entryTypes;
}

Expand All @@ -96,7 +94,7 @@ public ObservableList<ConsistencyMessage> getTableData() {
}

public Set<String> getColumnNames() {
Set<String> result = new LinkedHashSet<>(columnCount + EXTRA_COLUMNS_COUNT);
Set<String> result = LinkedHashSet.newLinkedHashSet(columnCount + EXTRA_COLUMNS_COUNT);
result.add("Entry Type");
result.add("CitationKey");
allReportedFields.forEach(field-> result.add(field.getDisplayName().trim()));
Expand Down Expand Up @@ -124,12 +122,12 @@ private void writeMapEntry(Map.Entry<EntryType, BibliographyConsistencyCheck.Ent
BibliographyConsistencyCheck.EntryTypeResult entries = mapEntry.getValue();
SequencedCollection<BibEntry> bibEntries = entries.sortedEntries();

bibEntries.forEach(Unchecked.consumer(bibEntry -> {
writeBibEntry(bibEntry, entryType, requiredFields, optionalFields);
}));
bibEntries.forEach(Unchecked.consumer(bibEntry ->
writeBibEntry(bibEntry, entryType, requiredFields, optionalFields)
));
}

private void writeBibEntry(BibEntry bibEntry, String entryType, Set<Field> requiredFields, Set<Field> optionalFields) throws IOException {
private void writeBibEntry(BibEntry bibEntry, String entryType, Set<Field> requiredFields, Set<Field> optionalFields) {
List<String> theRecord = getFindingsAsList(bibEntry, entryType, requiredFields, optionalFields);
List<String> message = new ArrayList<>();
for (String s: theRecord) {
Expand All @@ -143,7 +141,7 @@ private List<String> getFindingsAsList(BibEntry bibEntry, String entryType, Set<
List<String> result = new ArrayList<>(columnCount + EXTRA_COLUMNS_COUNT);
result.add(entryType);
result.add(bibEntry.getCitationKey().orElse(""));
allReportedFields.forEach(field -> {
allReportedFields.forEach(field ->
result.add(bibEntry.getField(field).map(_ -> {
if (requiredFields.contains(field)) {
return ConsistencySymbol.REQUIRED_FIELD_AT_ENTRY_TYPE_CELL_ENTRY.getText();
Expand All @@ -152,8 +150,8 @@ private List<String> getFindingsAsList(BibEntry bibEntry, String entryType, Set<
} else {
return ConsistencySymbol.UNKNOWN_FIELD_AT_ENTRY_TYPE_CELL_ENTRY.getText();
}
}).orElse(ConsistencySymbol.UNSET_FIELD_AT_ENTRY_TYPE_CELL_ENTRY.getText()));
});
}).orElse(ConsistencySymbol.UNSET_FIELD_AT_ENTRY_TYPE_CELL_ENTRY.getText()))
);
return result;
}

Expand Down
1 change: 0 additions & 1 deletion jabgui/src/main/java/org/jabref/gui/edit/EditAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ public void execute() {
}
} else if ((focusOwner instanceof CodeArea) || (focusOwner instanceof WebView)) {
LOGGER.debug("Ignoring request in CodeArea or WebView");
return;
} else {
LOGGER.debug("Else: {}", focusOwner.getClass().getSimpleName());
// Not sure what is selected -> copy/paste/cut selected entries except for Preview and CodeArea
Expand Down
11 changes: 6 additions & 5 deletions jabgui/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,11 @@ private Optional<Path> askForSavePath() {
// Workaround for linux systems not adding file extension
if (!savePath.getFileName().toString().toLowerCase().endsWith(".bib")) {
savePath = Path.of(savePath.toString() + ".bib");
if (!Files.notExists(savePath)) {
if (!dialogService.showConfirmationDialogAndWait(Localization.lang("Overwrite file"), Localization.lang("'%0' exists. Overwrite file?", savePath.getFileName()))) {
if (!Files.notExists(savePath) && !dialogService.showConfirmationDialogAndWait(Localization.lang("Overwrite file"),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double negative 'notExists' makes the code harder to read and understand. Should use 'Files.exists()' instead for better readability and maintainability.

Localization.lang("'%0' exists. Overwrite file?", savePath.getFileName()))) {
return Optional.empty();
}
}

selectedPath = Optional.of(savePath);
}
}
Expand Down Expand Up @@ -246,7 +246,7 @@ private boolean save(Path targetPath, SaveDatabaseMode mode) {
dialogService.notify(Localization.lang("Library saved"));
return success;
} catch (SaveException ex) {
LOGGER.error("A problem occurred when trying to save the file %s".formatted(targetPath), ex);
LOGGER.error("A problem occurred when trying to save the file {}", targetPath, ex);
dialogService.showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file."), ex);
return false;
} finally {
Expand Down Expand Up @@ -307,7 +307,8 @@ private void saveWithDifferentEncoding(Path file, boolean selectedOnly, Charset
.filter(buttonType -> buttonType.equals(tryDifferentEncoding))
.isPresent();
if (saveWithDifferentEncoding) {
Optional<Charset> newEncoding = dialogService.showChoiceDialogAndWait(Localization.lang("Save library"), Localization.lang("Select new encoding"), Localization.lang("Save library"), encoding, Encodings.getCharsets());
Optional<Charset> newEncoding = dialogService.showChoiceDialogAndWait(Localization.lang("Save library"), Localization.lang("Select new encoding"),
Localization.lang("Save library"), encoding, Encodings.getCharsets());
Comment on lines +310 to +311
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convention is to put every argument in a single line.

if (newEncoding.isPresent()) {
// Make sure to remember which encoding we used.
libraryTab.getBibDatabaseContext().getMetaData().setEncoding(newEncoding.get(), ChangePropagation.DO_NOT_POST_EVENT);
Expand Down
1 change: 1 addition & 0 deletions jabgui/src/main/java/org/jabref/gui/externalfiles/ChainedFilters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public class ChainedFilters implements DirectoryStream.Filter<Path> {

private DirectoryStream.Filter<Path>[] filters;

@SafeVarargs
public ChainedFilters(DirectoryStream.Filter<Path>... filters) {
this.filters = filters;
}
Comment on lines 18 to 23
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SafeVarArgs annotation is like SuppressWarnings. But ignoring warnings is imho not the best idea.
Can we refactor this to use proper list and do sthg like this?

        for (DirectoryStream.Filter<Path> filter : filters) {
            this.filters.add(filter);
        }

Can we use List.of on this?
Can the method signature be refactored to use a List?

Expand Down
6 changes: 4 additions & 2 deletions jabgui/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesCrawler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.io.File;
import java.io.IOException;
import java.nio.file.DirectoryStream;
import java.nio.file.DirectoryStream.Filter;
import java.nio.file.Files;
import java.nio.file.Path;
Expand Down Expand Up @@ -91,7 +92,8 @@ FileNodeViewModel searchDirectory(Path directory, UnlinkedPDFFileFilter unlinked
// 2. GitIgnoreFilter
ChainedFilters filters = new ChainedFilters(unlinkedPDFFileFilter, new GitIgnoreFileFilter(directory));
Map<Boolean, List<Path>> directoryAndFilePartition;
try (Stream<Path> filesStream = StreamSupport.stream(Files.newDirectoryStream(directory, filters).spliterator(), false)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it split in two ?

try (DirectoryStream<Path> dirStream = Files.newDirectoryStream(directory, filters);
Stream<Path> filesStream = StreamSupport.stream(dirStream.spliterator(), false)) {
directoryAndFilePartition = filesStream.collect(Collectors.partitioningBy(Files::isDirectory));
} catch (IOException e) {
LOGGER.error("Error while searching files", e);
Expand Down Expand Up @@ -136,7 +138,7 @@ FileNodeViewModel searchDirectory(Path directory, UnlinkedPDFFileFilter unlinked
// create and add FileNodeViewModel to the FileNodeViewModel for the current directory
fileNodeViewModelForCurrentDirectory.getChildren().addAll(resultingFiles.stream()
.map(FileNodeViewModel::new)
.collect(Collectors.toList()));
.toList());

return fileNodeViewModelForCurrentDirectory;
}
Expand Down
2 changes: 1 addition & 1 deletion jabgui/src/main/java/org/jabref/gui/fieldeditors/KeywordsEditor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public KeywordsEditor(Field field,
KeyBindingRepository keyBindingRepository = Injector.instantiateModelOrService(KeyBindingRepository.class);

if (keyBindingRepository.checkKeyCombinationEquality(KeyBinding.PASTE, event)) {
String clipboardText = clipBoardManager.getContents();
String clipboardText = ClipBoardManager.getContents();
if (!clipboardText.isEmpty()) {
KeywordList keywordsList = KeywordList.parse(clipboardText, viewModel.getKeywordSeparator());
keywordsList.stream().forEach(keyword -> keywordTagsField.addTags(keyword));
Expand Down
3 changes: 2 additions & 1 deletion ...main/java/org/jabref/gui/fieldeditors/optioneditors/mapbased/MapBasedEditorViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ protected T getValueFromString(String string) {
try {
return (T) string;
} catch (ClassCastException ex) {
LOGGER.error("Could not cast string to type %1$s. Try overriding the method in a subclass and provide a conversion from string to the concrete type %1$s".formatted(string.getClass()), ex);
LOGGER.error("Could not cast string to type {}. Try overriding the method in a subclass and provide a conversion from string to the concrete type {}",
string.getClass(), string.getClass(), ex);
}
return null;
}
Expand Down
Loading
Loading