Skip to content

Commit

Permalink
Refine checkstyle rules (#6283)
Browse files Browse the repository at this point in the history
- Add (and apply) checkstyle rules for
  - correct spacing (especalliy // space-after-comment-marker)
  - } else if { (<-- in one line)
  - no two empty lines
  - @Deprecation JavaDoc
  - some coding practices
- Fix HTML syntax of JavaDoc comments
- Remove CSS class comment syntax in IconTheme class
- Remove deprecated method BibEntry#getCiteKey()
  • Loading branch information
koppor authored Apr 14, 2020
1 parent fb2a6d7 commit 940ef9d
Show file tree
Hide file tree
Showing 272 changed files with 991 additions and 921 deletions.
63 changes: 59 additions & 4 deletions config/checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
"http://www.checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
<property name="charset" value="UTF-8"/>

<module name="Header">
<property name="header" value=""/>
</module>
Expand All @@ -21,6 +23,9 @@
<property name="fileNamePattern" value="module\-info\.java$"/>
</module>

<module name="FileTabCharacter"/>
<module name="NewlineAtEndOfFile"/>

<module name="TreeWalker">
<!-- Checks for Javadoc comments: https://checkstyle.org/config_javadoc.html -->
<module name="InvalidJavadocPosition"/>
Expand All @@ -42,16 +47,27 @@
<property name="sortStaticImportsAlphabetically" value="true"/>
</module>

<!-- Checks for common coding problems: https://checkstyle.org/config_coding.html -->
<module name="DeclarationOrder"/>

<!-- Checks for whitespace: https://checkstyle.org/config_whitespace.html -->

<module name="EmptyForInitializerPad"/>
<module name="EmptyLineSeparator">
<!-- check all except variable declarations -->
<property name="tokens"
value="IMPORT, CLASS_DEF, INTERFACE_DEF, ENUM_DEF, STATIC_INIT, INSTANCE_INIT, METHOD_DEF"/>
value="IMPORT, STATIC_IMPORT, CLASS_DEF, INTERFACE_DEF, ENUM_DEF, STATIC_INIT, INSTANCE_INIT, METHOD_DEF, CTOR_DEF"/>
<property name="allowMultipleEmptyLines" value="false"/>
<property name="allowMultipleEmptyLinesInsideClassMembers" value="false"/>
</module>
<module name="GenericWhitespace"/>
<module name="MethodParamPad"/>
<module name="NoLineWrap"/>
<module name="NoWhitespaceAfter"/>
<module name="NoWhitespaceBefore"/>
<module name="ParenPad"/>
<module name="SeparatorWrap">
<property name="tokens" value="COMMA, SEMI, ELLIPSIS, ARRAY_DECLARATOR, RBRACK, METHOD_REF"/>
</module>
<module name="SingleSpaceSeparator"/>
<module name="WhitespaceAfter"/>
<module name="WhitespaceAround">
<!-- RCULRY causes issues if classes are nested within arrays, therefore not activated -->
<property name="tokens"
Expand All @@ -70,5 +86,44 @@

<!-- Checks for blocks: https://checkstyle.org/config_blocks.html -->
<module name="NeedBraces"/>

<module name="EmptyBlock">
<property name="option" value="text"/>
</module>

<!-- Disallows empty catch blocks (not even having a comment): https://checkstyle.sourceforge.io/config_blocks.html#EmptyCatchBlock -->
<module name="EmptyCatchBlock"/>

<!--
following rule enforces that there are no one line statements such as
public String getTabName() { return Localization.lang("XMP metadata"); }
Since it is too much effort to reformat all code, it is currently not enabled -->
<!-- <module name="LeftCurly"/> -->

<module name="RightCurly"/>
<!-- coding - https://checkstyle.sourceforge.io/config_coding.html -->

<module name="AvoidDoubleBraceInitialization"/>

<module name="CovariantEquals"/>

<!-- Checks for common coding problems: https://checkstyle.org/config_coding.html -->

<module name="DeclarationOrder"/>
<module name="EmptyStatement"/>

<module name="EqualsHashCode"/>

<!-- force a space after // for comments -->
<module name="TodoComment">
<property name="id" value="commentStartWithSpace"/>
<property name="format" value="^([^\s\/*])"/>
<message key="todo.match" value="Comment text should start with space."/>
</module>

<module name="MissingDeprecated"/>

</module>
</module>
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/JabRefExecutorService.java
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public void submit(TimerTask timerTask, long millisecondsDelay) {
public void shutdownEverything() {
// those threads will be allowed to finish
this.executorService.shutdown();
//those threads will be interrupted in their current task
// those threads will be interrupted in their current task
this.lowPriorityExecutorService.shutdownNow();
// kill the remote thread
stopRemoteThread();
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/JabRefGUI.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private void openWindow(Stage mainStage) {
if (Globals.prefs.getBoolean(JabRefPreferences.WINDOW_MAXIMISED)) {
mainStage.setMaximized(true);
} else if (Screen.getScreens().size() == 1 && isWindowPositionOutOfBounds()) {
//corrects the Window, if its outside of the mainscreen
// corrects the Window, if its outside of the mainscreen
LOGGER.debug("The Jabref Window is outside the Main Monitor\n");
mainStage.setX(0);
mainStage.setY(0);
Expand Down Expand Up @@ -94,8 +94,8 @@ private void openWindow(Stage mainStage) {

mainStage.setOnCloseRequest(event -> {
if (!correctedWindowPos) {
//saves the window position only if its not corrected -> the window will rest at the old Position,
//if the external Screen is connected again.
// saves the window position only if its not corrected -> the window will rest at the old Position,
// if the external Screen is connected again.
saveWindowState(mainStage);
}
boolean reallyQuit = mainFrame.quit();
Expand Down
12 changes: 6 additions & 6 deletions src/main/java/org/jabref/cli/ArgumentProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ private List<ParserResult> processArguments() {

private boolean exportMatches(List<ParserResult> loaded) {
String[] data = cli.getExportMatches().split(",");
String searchTerm = data[0].replace("\\$", " "); //enables blanks within the search term:
//$ stands for a blank
String searchTerm = data[0].replace("\\$", " "); // enables blanks within the search term:
// $ stands for a blank
ParserResult pr = loaded.get(loaded.size() - 1);
BibDatabaseContext databaseContext = pr.getDatabaseContext();
BibDatabase dataBase = pr.getDatabase();
Expand All @@ -252,17 +252,17 @@ private boolean exportMatches(List<ParserResult> loaded) {
searchPreferences.isRegularExpression());
List<BibEntry> matches = new DatabaseSearcher(query, dataBase).getMatches();

//export matches
// export matches
if (!matches.isEmpty()) {
String formatName;

//read in the export format, take default format if no format entered
// read in the export format, take default format if no format entered
switch (data.length) {
case 3:
formatName = data[2];
break;
case 2:
//default exporter: HTML table (with Abstract & BibTeX)
// default exporter: HTML table (with Abstract & BibTeX)
formatName = "tablerefsabsbib";
break;
default:
Expand All @@ -272,7 +272,7 @@ private boolean exportMatches(List<ParserResult> loaded) {
return false;
}

//export new database
// export new database
Optional<Exporter> exporter = Globals.exportFactory.getExporterByName(formatName);
if (!exporter.isPresent()) {
System.err.println(Localization.lang("Unknown export format") + ": " + formatName);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/AbstractViewModel.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
package org.jabref.gui;

public class AbstractViewModel {
//empty
// empty
}
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/EntryTypeView.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public EntryTypeView(BasePanel basePanel, DialogService dialogService, JabRefPre
ControlHelper.setAction(generateButton, this.getDialogPane(), event -> viewModel.runFetcherWorker());

setResultConverter(button -> {
//The buttonType will always be cancel, even if we pressed one of the entry type buttons
// The buttonType will always be cancel, even if we pressed one of the entry type buttons
return type;
});

Expand Down Expand Up @@ -112,8 +112,8 @@ public void initialize() {

new ViewModelListCellFactory<IdBasedFetcher>().withText(item -> item.getName()).install(idBasedFetchers);

//we set the managed property so that they will only be rendered when they are visble so that the Nodes only take the space when visible
//avoids removing and adding from the scence graph
// we set the managed property so that they will only be rendered when they are visble so that the Nodes only take the space when visible
// avoids removing and adding from the scence graph
bibTexTitlePane.managedProperty().bind(bibTexTitlePane.visibleProperty());
ieeeTranTitlePane.managedProperty().bind(ieeeTranTitlePane.visibleProperty());
biblatexTitlePane.managedProperty().bind(biblatexTitlePane.visibleProperty());
Expand Down
24 changes: 12 additions & 12 deletions src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ public void setWindowTitle() {

// no database open
if (panel == null) {
//setTitle(FRAME_TITLE);
// setTitle(FRAME_TITLE);
return;
}

Expand All @@ -307,9 +307,9 @@ public void setWindowTitle() {
.getDatabasePath()
.map(Path::toString)
.orElse(Localization.lang("untitled"));
//setTitle(FRAME_TITLE + " - " + databaseFile + changeFlag + modeInfo);
// setTitle(FRAME_TITLE + " - " + databaseFile + changeFlag + modeInfo);
} else if (panel.getBibDatabaseContext().getLocation() == DatabaseLocation.SHARED) {
//setTitle(FRAME_TITLE + " - " + panel.getBibDatabaseContext().getDBMSSynchronizer().getDBName() + " ["
// setTitle(FRAME_TITLE + " - " + panel.getBibDatabaseContext().getDBMSSynchronizer().getDBName() + " ["
// + Localization.lang("shared") + "]" + modeInfo);
}
}
Expand Down Expand Up @@ -343,7 +343,7 @@ public JabRefPreferences prefs() {
* set to true
*/
private void tearDownJabRef(List<String> filenames) {
//prefs.putBoolean(JabRefPreferences.WINDOW_MAXIMISED, getExtendedState() == Frame.MAXIMIZED_BOTH);
// prefs.putBoolean(JabRefPreferences.WINDOW_MAXIMISED, getExtendedState() == Frame.MAXIMIZED_BOTH);

if (prefs.getBoolean(JabRefPreferences.OPEN_LAST_EDITED)) {
// Here we store the names of all current files. If
Expand Down Expand Up @@ -554,10 +554,10 @@ public void init() {

initDragAndDrop();

//setBounds(GraphicsEnvironment.getLocalGraphicsEnvironment().getMaximumWindowBounds());
//WindowLocation pw = new WindowLocation(this, JabRefPreferences.POS_X, JabRefPreferences.POS_Y, JabRefPreferences.SIZE_X,
// setBounds(GraphicsEnvironment.getLocalGraphicsEnvironment().getMaximumWindowBounds());
// WindowLocation pw = new WindowLocation(this, JabRefPreferences.POS_X, JabRefPreferences.POS_Y, JabRefPreferences.SIZE_X,
// JabRefPreferences.SIZE_Y);
//pw.displayWindowAtStoredLocation();
// pw.displayWindowAtStoredLocation();

// Bind global state
stateManager.activeDatabaseProperty().bind(
Expand Down Expand Up @@ -595,9 +595,9 @@ public void init() {
stateManager.activeSearchQueryProperty().set(newBasePanel.getCurrentSearchQuery());

// groupSidePane.getToggleCommand().setSelected(sidePaneManager.isComponentVisible(GroupSidePane.class));
//previewToggle.setSelected(Globals.prefs.getPreviewPreferences().isPreviewPanelEnabled());
//generalFetcher.getToggleCommand().setSelected(sidePaneManager.isComponentVisible(WebSearchPane.class));
//openOfficePanel.getToggleCommand().setSelected(sidePaneManager.isComponentVisible(OpenOfficeSidePanel.class));
// previewToggle.setSelected(Globals.prefs.getPreviewPreferences().isPreviewPanelEnabled());
// generalFetcher.getToggleCommand().setSelected(sidePaneManager.isComponentVisible(WebSearchPane.class));
// openOfficePanel.getToggleCommand().setSelected(sidePaneManager.isComponentVisible(OpenOfficeSidePanel.class));

setWindowTitle();
// Update search autocompleter with information for the correct database:
Expand Down Expand Up @@ -743,7 +743,7 @@ private MenuBar createMenu() {
);
}

//@formatter:off
// @formatter:off
library.getItems().addAll(
factory.createMenuItem(StandardActions.NEW_ENTRY, new NewEntryAction(this, dialogService, Globals.prefs, stateManager)),
factory.createMenuItem(StandardActions.NEW_ENTRY_FROM_PLAIN_TEXT, new ExtractBibtexAction(stateManager)),
Expand Down Expand Up @@ -883,7 +883,7 @@ private MenuBar createMenu() {
factory.createMenuItem(StandardActions.ABOUT, new AboutAction())
);

//@formatter:on
// @formatter:on
MenuBar menu = new MenuBar();
menu.getStyleClass().add("mainMenu");
menu.getMenus().addAll(
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/actions/JabRefAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ private String getActionName(Action action, Command command) {
return action.getText();
} else {
String commandName = command.getClass().getSimpleName();
if ( commandName.contains("EditAction")
if (commandName.contains("EditAction")
|| commandName.contains("CopyMoreAction")
|| commandName.contains("CopyCitationAction")
|| commandName.contains("PreviewSwitchAction")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,7 @@ public SuggestionProviderString(Callback<T, String> stringConverter) {

// In case no stringConverter was provided, use the default strategy
if (this.stringConverter == null) {
this.stringConverter = obj -> {
return obj != null ? obj.toString() : ""; //$NON-NLS-1$
};
this.stringConverter = obj -> obj != null ? obj.toString() : "";
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public ExtractBibtexDialog() {
@FXML
private void initialize() {
BibDatabaseContext database = stateManager.getActiveDatabase().orElseThrow(() -> new NullPointerException("Database null"));
this.viewModel = new BibtexExtractorViewModel(database, dialogService, JabRefPreferences.getInstance(), fileUpdateMonitor, taskExecutor,undoManager,stateManager);
this.viewModel = new BibtexExtractorViewModel(database, dialogService, JabRefPreferences.getInstance(), fileUpdateMonitor, taskExecutor, undoManager, stateManager);
input.textProperty().bindBidirectional(viewModel.inputTextProperty());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ private void buildGUI() {
Label keyPattern = new Label(Localization.lang("Key pattern"));
gridPane.add(label, ++columnIndex, rowIndex);
gridPane.add(keyPattern, ++columnIndex, rowIndex);
++columnIndex; //3
columnIndex++;
}

rowIndex++;
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/collab/ChangeDisplayDialog.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,19 @@ public ChangeDisplayDialog(BibDatabaseContext database, List<DatabaseChangeViewM
setResultConverter(button -> {
if (button == dismissChanges) {
return false;

} else {
// Perform all accepted changes
NamedCompound ce = new NamedCompound(Localization.lang("Merged external changes"));
for (DatabaseChangeViewModel change : changes) {
if (change instanceof EntryChangeViewModel) {
change.makeChange(database, ce); //We don't have a checkbox for accept and always get the correct merged entry, the accept property in this special case only controls the radio buttons selection
// We don't have a checkbox for accept and always get the correct merged entry, the accept property in this special case only controls the radio buttons selection
change.makeChange(database, ce);
} else if (change.isAccepted()) {
change.makeChange(database, ce);
}
}
ce.end();
//TODO: panel.getUndoManager().addEdit(ce);
// TODO: panel.getUndoManager().addEdit(ce);

return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,21 @@ private void initialize() {
this.itemsProperty().bindBidirectional(viewModel.patternListProperty());
}

public void setValues() { viewModel.setValues(); }
public void setValues() {
viewModel.setValues();
}

public void resetAll() { viewModel.resetAll(); }
public void resetAll() {
viewModel.resetAll();
}

public ListProperty<BibtexKeyPatternPanelItemModel> patternListProperty() { return viewModel.patternListProperty(); }
public ListProperty<BibtexKeyPatternPanelItemModel> patternListProperty() {
return viewModel.patternListProperty();
}

public ObjectProperty<BibtexKeyPatternPanelItemModel> defaultKeyPatternProperty() { return viewModel.defaultKeyPatternProperty(); }
public ObjectProperty<BibtexKeyPatternPanelItemModel> defaultKeyPatternProperty() {
return viewModel.defaultKeyPatternProperty();
}

private void jumpToSearchKey(KeyEvent keypressed) {
if (keypressed.getCharacter() == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,13 @@ public BibtexKeyPatternPanelItemModel(EntryType entryType, String pattern) {
this.pattern.setValue(pattern);
}

public EntryType getEntryType() { return entryType.getValue(); }
public EntryType getEntryType() {
return entryType.getValue();
}

public ObjectProperty<EntryType> entryType() { return entryType; }
public ObjectProperty<EntryType> entryType() {
return entryType;
}

public void setPattern(String pattern) {
this.pattern.setValue(pattern);
Expand All @@ -32,8 +36,12 @@ public String getPattern() {
return pattern.getValue();
}

public StringProperty pattern() { return pattern; }
public StringProperty pattern() {
return pattern;
}

@Override
public String toString() { return "[" + entryType.getValue().getName() + "," + pattern.getValue() + "]"; }
public String toString() {
return "[" + entryType.getValue().getName() + "," + pattern.getValue() + "]";
}
}
Loading

0 comments on commit 940ef9d

Please sign in to comment.