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.

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
2 changes: 1 addition & 1 deletion jabgui/src/main/java/org/jabref/Launcher.java
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
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
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
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package org.jabref.gui.commonfxcontrols;

import java.util.List;
import java.util.stream.Collectors;

import javax.swing.undo.UndoManager;

Expand Down
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
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
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
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
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
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
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
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