diff --git a/build.gradle b/build.gradle index f4fe2a3bb..82870685e 100644 --- a/build.gradle +++ b/build.gradle @@ -190,13 +190,18 @@ test { } checkstyle { + toolVersion = "10.4" ignoreFailures = false configDirectory.set(metricsConfigDir.dir("checkstyle")) } pmd { - ignoreFailures = true - ruleSetFiles = files("$rootDir/config/metrics/pmd/pmd.xml") + toolVersion = "6.39.0" + ignoreFailures = false + consoleOutput = true + threads = 4 + ruleSetFiles "${rootDir}/config/pmd.xml" + ruleSets = [] } spotbugs { @@ -240,12 +245,14 @@ task extractCodeMetricsConfig(type: Copy) { into metricsConfigDir } +processResources.dependsOn(createVersionInfoFile, extractCodeMetricsConfig) + clean { delete createVersionInfoFile.outputs.files delete extractCodeMetricsConfig.destinationDir } -processResources.dependsOn(createVersionInfoFile, extractCodeMetricsConfig) +processResources.dependsOn(createVersionInfoFile) jar { //TODO we only use this name because the `.exe` start scripts require the JAR to be named 'TerasologyLauncher.jar' diff --git a/config/pmd.xml b/config/pmd.xml new file mode 100644 index 000000000..8e0591ddd --- /dev/null +++ b/config/pmd.xml @@ -0,0 +1,32 @@ + + + + + + Custom rules for the Terasology Launcher project. + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/src/main/java/org/terasology/launcher/TerasologyLauncher.java b/src/main/java/org/terasology/launcher/TerasologyLauncher.java index a91db2151..8040cc431 100644 --- a/src/main/java/org/terasology/launcher/TerasologyLauncher.java +++ b/src/main/java/org/terasology/launcher/TerasologyLauncher.java @@ -98,7 +98,7 @@ public void start(final Stage initialStage) { }); launcherInitTask.setOnFailed(event -> { - logger.error("The TerasologyLauncher could not be started!", event.getSource().getException()); + logger.error("The TerasologyLauncher could not be started!", event.getSource().getException()); //NOPMD System.exit(1); }); @@ -179,9 +179,12 @@ private void showSplashStage(final Stage initialStage, final Taskjava command manual */ -class GameStarter implements Callable { +final class GameStarter implements Callable { private static final Logger logger = LoggerFactory.getLogger(GameStarter.class); final ProcessBuilder processBuilder; diff --git a/src/main/java/org/terasology/launcher/log/TempLogFilePropertyDefiner.java b/src/main/java/org/terasology/launcher/log/TempLogFilePropertyDefiner.java index aa300c552..04ca0e169 100644 --- a/src/main/java/org/terasology/launcher/log/TempLogFilePropertyDefiner.java +++ b/src/main/java/org/terasology/launcher/log/TempLogFilePropertyDefiner.java @@ -31,7 +31,7 @@ public TempLogFilePropertyDefiner() { throw new IllegalStateException("This class must not be instantiated twice"); } - instance = this; + instance = this; //NOPMD(AssignmentToNonFinalStatic) } public static TempLogFilePropertyDefiner getInstance() { @@ -55,6 +55,7 @@ public String getPrefix() { /** * Set the prefix string for generating the file's name. + * * @param prefix the prefix string to be used in generating the file's name; may be null */ public void setPrefix(String prefix) { @@ -67,6 +68,7 @@ public String getSuffix() { /** * Set the suffix string for generating the file's name. + * * @param suffix the suffix string to be used in generating the file's name; may be null, in which case ".tmp" is used */ public void setSuffix(String suffix) { @@ -75,6 +77,7 @@ public void setSuffix(String suffix) { /** * Returns the temporary log file. + * * @return the log file */ public Path getLogFile() { diff --git a/src/main/java/org/terasology/launcher/repositories/Jenkins.java b/src/main/java/org/terasology/launcher/repositories/Jenkins.java index f8a746004..6a2fb2ecc 100644 --- a/src/main/java/org/terasology/launcher/repositories/Jenkins.java +++ b/src/main/java/org/terasology/launcher/repositories/Jenkins.java @@ -5,12 +5,10 @@ /** * Data model for parsing build information from Jenkins. + * + * Instances of this class will be created by JSON parsers (e.g., GSON) and are usually not instantiated by hand. */ public final class Jenkins { - - private Jenkins() { - } - public static class ApiResult { public Build[] builds; } diff --git a/src/main/java/org/terasology/launcher/repositories/JenkinsClient.java b/src/main/java/org/terasology/launcher/repositories/JenkinsClient.java index 600a9cad4..8c972c07e 100644 --- a/src/main/java/org/terasology/launcher/repositories/JenkinsClient.java +++ b/src/main/java/org/terasology/launcher/repositories/JenkinsClient.java @@ -29,10 +29,10 @@ class JenkinsClient { private static final String ARTIFACT = "artifact/"; - private final Gson gson; - final OkHttpClient client; + private final Gson gson; + JenkinsClient(OkHttpClient httpClient, Gson gson) { this.gson = gson; @@ -83,6 +83,7 @@ Jenkins.ApiResult request(URL url) throws InterruptedException { // c) the 'Expires' header is removed from response for requests with PropertiesRequest @Nullable + @SuppressWarnings("PMD.ReturnEmptyCollectionRatherThanNull") Properties requestProperties(final URL artifactUrl) { Preconditions.checkNotNull(artifactUrl); diff --git a/src/main/java/org/terasology/launcher/repositories/JenkinsRepositoryAdapter.java b/src/main/java/org/terasology/launcher/repositories/JenkinsRepositoryAdapter.java index 9273a8cc3..4c0068016 100644 --- a/src/main/java/org/terasology/launcher/repositories/JenkinsRepositoryAdapter.java +++ b/src/main/java/org/terasology/launcher/repositories/JenkinsRepositoryAdapter.java @@ -138,7 +138,7 @@ private String computeChangelogFrom(Jenkins.ChangeSet changeSet) { private static URL unsafeToUrl(String url) { try { return new URL(url); - } catch (MalformedURLException e) { + } catch (MalformedURLException e) { //NOPMD //TODO: at least log something here? } return null; diff --git a/src/main/java/org/terasology/launcher/settings/LauncherSettings.java b/src/main/java/org/terasology/launcher/settings/LauncherSettings.java index 6b7d2f10b..b82e40a39 100644 --- a/src/main/java/org/terasology/launcher/settings/LauncherSettings.java +++ b/src/main/java/org/terasology/launcher/settings/LauncherSettings.java @@ -7,7 +7,6 @@ import com.google.gson.Gson; import com.google.gson.JsonParseException; import javafx.beans.property.Property; -import javafx.beans.property.ReadOnlyProperty; import javafx.beans.property.SimpleBooleanProperty; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -30,10 +29,9 @@ /** * User settings for the launcher, backed by Java {@link Properties}. */ +@SuppressWarnings("checkstyle:DeclarationOrder") public class LauncherSettings { - private static final Logger logger = LoggerFactory.getLogger(LauncherSettings.class); - public static final String USER_JAVA_PARAMETERS_DEFAULT = "-XX:MaxGCPauseMillis=20"; public static final String USER_GAME_PARAMETERS_DEFAULT = ""; @@ -54,6 +52,11 @@ public class LauncherSettings { public static final String PROPERTY_LAST_INSTALLED_GAME_JOB = "lastInstalledGameJob"; public static final String PROPERTY_LAST_INSTALLED_GAME_VERSION = "lastInstalledGameVersion"; + private static final Logger logger = LoggerFactory.getLogger(LauncherSettings.class); + + private static final String WARN_MSG_INVALID_VALUE = "Invalid value '{}' for the parameter '{}'!"; + private static final Level LOG_LEVEL_DEFAULT = Level.INFO; + static final JavaHeapSize MAX_HEAP_SIZE_DEFAULT = JavaHeapSize.NOT_USED; static final JavaHeapSize INITIAL_HEAP_SIZE_DEFAULT = JavaHeapSize.NOT_USED; static final boolean CLOSE_LAUNCHER_AFTER_GAME_START_DEFAULT = true; @@ -64,9 +67,6 @@ public class LauncherSettings { static final String LAUNCHER_SETTINGS_FILE_NAME = "TerasologyLauncherSettings.properties"; - private static final String WARN_MSG_INVALID_VALUE = "Invalid value '{}' for the parameter '{}'!"; - private static final Level LOG_LEVEL_DEFAULT = Level.INFO; - private final Properties properties; private final Property showPreReleases = new SimpleBooleanProperty(SHOW_PRE_RELEASES_DEFAULT); @@ -105,7 +105,9 @@ public synchronized void init() { void initLocale() { final String localeStr = properties.getProperty(PROPERTY_LOCALE); if (localeStr != null) { - Locale l = I18N.getSupportedLocales().stream().filter(x -> x.toLanguageTag().equals(localeStr)).findFirst().orElse(I18N.getDefaultLocale()); + Locale l = I18N.getSupportedLocales().stream() + .filter(x -> x.toLanguageTag().equals(localeStr)) + .findFirst().orElse(I18N.getDefaultLocale()); I18N.setLocale(l); if (!I18N.getCurrentLocale().toString().equals(localeStr)) { @@ -246,14 +248,6 @@ void initLastInstalledGameVersion() { } } - // --------------------------------------------------------------------- // - // PROPERTIES - // --------------------------------------------------------------------- // - - public ReadOnlyProperty showPreReleases() { - return showPreReleases; - } - // --------------------------------------------------------------------- // // GETTERS // --------------------------------------------------------------------- // diff --git a/src/main/java/org/terasology/launcher/ui/ApplicationController.java b/src/main/java/org/terasology/launcher/ui/ApplicationController.java index e02c63ef6..df32c953a 100644 --- a/src/main/java/org/terasology/launcher/ui/ApplicationController.java +++ b/src/main/java/org/terasology/launcher/ui/ApplicationController.java @@ -244,8 +244,10 @@ private void initComboBoxes() { }, config, showPreReleases); gameReleaseComboBox.itemsProperty().bind(releases); - gameReleaseComboBox.buttonCellProperty().bind(Bindings.createObjectBinding(() -> new GameReleaseCell(installedGames, true), installedGames)); - gameReleaseComboBox.cellFactoryProperty().bind(Bindings.createObjectBinding(() -> list -> new GameReleaseCell(installedGames), installedGames)); + gameReleaseComboBox.buttonCellProperty() + .bind(Bindings.createObjectBinding(() -> new GameReleaseCell(installedGames, true), installedGames)); + gameReleaseComboBox.cellFactoryProperty() + .bind(Bindings.createObjectBinding(() -> list -> new GameReleaseCell(installedGames), installedGames)); selectedRelease.bind(gameReleaseComboBox.getSelectionModel().selectedItemProperty()); //TODO: instead of imperatively updating the changelog view its value should be bound via property, too diff --git a/src/main/java/org/terasology/launcher/ui/FxTimer.java b/src/main/java/org/terasology/launcher/ui/FxTimer.java index 1cfa626d9..bf6e499b0 100644 --- a/src/main/java/org/terasology/launcher/ui/FxTimer.java +++ b/src/main/java/org/terasology/launcher/ui/FxTimer.java @@ -29,7 +29,7 @@ private FxTimer(java.time.Duration actionTime, java.time.Duration period, Runnab this.action = action; timeline.getKeyFrames().add(new KeyFrame(this.actionTime)); // used as placeholder - if (period != actionTime) { + if (!period.equals(actionTime)) { timeline.getKeyFrames().add(new KeyFrame(Duration.millis(period.toMillis()))); } diff --git a/src/main/java/org/terasology/launcher/ui/SettingsController.java b/src/main/java/org/terasology/launcher/ui/SettingsController.java index b0caeea60..d08ce1263 100644 --- a/src/main/java/org/terasology/launcher/ui/SettingsController.java +++ b/src/main/java/org/terasology/launcher/ui/SettingsController.java @@ -31,7 +31,6 @@ import java.util.Arrays; import java.util.List; import java.util.Locale; -import java.util.MissingResourceException; import java.util.stream.Collectors; public class SettingsController { @@ -386,18 +385,16 @@ protected void updateItem(Locale item, boolean empty) { String countryCode = item.toLanguageTag(); String id = "flag_" + countryCode; - try { - // Get the appropriate flag icon via BundleUtils - Image icon = I18N.getFxImage(id); - + // Get the appropriate flag icon via BundleUtils + Image icon = I18N.getFxImage(id); + if (icon != null) { ImageView iconImageView = new ImageView(icon); iconImageView.setFitHeight(11); iconImageView.setPreserveRatio(true); this.setGraphic(iconImageView); - } catch (MissingResourceException e) { - logger.warn("ImageBundle key {} not found", id); - } catch (NullPointerException e) { - logger.warn("Flag icon in ImageBundle key {} missing or corrupt", id); + } else { + logger.warn("Flag icon for key '{}' could not be loaded.", id); + } } } diff --git a/src/main/java/org/terasology/launcher/util/DownloadUtils.java b/src/main/java/org/terasology/launcher/util/DownloadUtils.java index e8d81025f..0d711ebc1 100644 --- a/src/main/java/org/terasology/launcher/util/DownloadUtils.java +++ b/src/main/java/org/terasology/launcher/util/DownloadUtils.java @@ -99,7 +99,7 @@ private static void downloadToFile(ProgressListener listener, long contentLength long writtenBytes = 0; int n; if (!listener.isCancelled()) { - while ((n = in.read(buffer)) != -1) { + while ((n = in.read(buffer)) != -1) { //NOPMD(AssignmentInOperand) if (listener.isCancelled()) { break; } @@ -110,7 +110,7 @@ private static void downloadToFile(ProgressListener listener, long contentLength int percentage = (int) (sizeFactor * writtenBytes); if (percentage < 1) { percentage = 1; - } else if (percentage >= 100) { + } else if (percentage >= 100) { //NOPMD(AvoidLiteralsInIfCondition) percentage = 99; } listener.update(percentage); diff --git a/src/main/java/org/terasology/launcher/util/I18N.java b/src/main/java/org/terasology/launcher/util/I18N.java index 9eae84cd5..e852305ef 100644 --- a/src/main/java/org/terasology/launcher/util/I18N.java +++ b/src/main/java/org/terasology/launcher/util/I18N.java @@ -25,6 +25,7 @@ import java.util.MissingResourceException; import java.util.ResourceBundle; +@SuppressWarnings({"PMD.FieldNamingConventions", "checkstyle:ConstantName"}) public final class I18N { private static final Logger logger = LoggerFactory.getLogger(I18N.class); @@ -39,7 +40,7 @@ public final class I18N { * The currently selected locale. */ private static final Locale systemLocale; - private static final ObjectProperty locale; + private static final ObjectProperty localeProperty; private static final List supportedLocales; /** @@ -77,8 +78,8 @@ public final class I18N { systemLocale = getDefaultLocale(); - locale = new SimpleObjectProperty<>(getDefaultLocale()); - locale.addListener((observable, oldValue, newValue) -> Locale.setDefault(newValue)); + localeProperty = new SimpleObjectProperty<>(getDefaultLocale()); + localeProperty.addListener((observable, oldValue, newValue) -> Locale.setDefault(newValue)); } private I18N() { @@ -89,7 +90,7 @@ public static Locale getDefaultLocale() { } public static Locale getCurrentLocale() { - return locale.get(); + return localeProperty.get(); } public static void setLocale(Locale locale) { @@ -100,14 +101,13 @@ public static void setLocale(Locale locale) { } public static ObjectProperty localeProperty() { - return locale; + return localeProperty; } public static List getSupportedLocales() { return supportedLocales; } - public static String getLabel(String key) { return getLabel(getCurrentLocale(), key); } @@ -129,7 +129,7 @@ public static String getLabel(Locale locale, String key) { } public static Binding labelBinding(String key) { - return Bindings.createStringBinding(() -> getLabel(locale.getValue(), key), locale); + return Bindings.createStringBinding(() -> getLabel(localeProperty.getValue(), key), localeProperty); } public static String getMessage(String key, Object... arguments) { @@ -162,12 +162,15 @@ public static URI getURI(String key) { * Loads a JavaFX {@code Image} from the image path specified by the key in the image bundle file. * * @param key the key as specified in the image bundle file - * @return the JavaFX image - * @throws MissingResourceException if no resource for the specified key can be found + * @return the JavaFX image, or null if the image cannot be found or loaded */ public static Image getFxImage(String key) throws MissingResourceException { final String imagePath = ResourceBundle.getBundle(IMAGE_BUNDLE, getCurrentLocale()).getString(key); - return new Image(I18N.class.getResource(imagePath).toExternalForm()); + URL resource = I18N.class.getResource(imagePath); + if (resource != null) { + return new Image(resource.toExternalForm()); + } + return null; } public static FXMLLoader getFXMLLoader(String key) { diff --git a/src/test/java/org/terasology/launcher/game/TestRunGameTask.java b/src/test/java/org/terasology/launcher/game/TestRunGameTask.java index c3cacf6c3..441009d56 100644 --- a/src/test/java/org/terasology/launcher/game/TestRunGameTask.java +++ b/src/test/java/org/terasology/launcher/game/TestRunGameTask.java @@ -87,15 +87,16 @@ public void testGameOutput() throws InterruptedException, RunGameTask.RunGameErr var hasGameOutputFormat = LogMatchers.hasFormatWithPattern("^Game output.*"); - LogAssert detailedExpectation = TestLoggers.sys().expect( + // try-with-resources to auto-close LogAssert + try (LogAssert detailedExpectation = TestLoggers.sys().expect( RunGameTask.class.getName(), Level.INFO, allOf(hasGameOutputFormat, LogMatchers.hasArguments(gameOutputLines[0])), allOf(hasGameOutputFormat, LogMatchers.hasArguments(gameOutputLines[1])) - ); - - new NonTimingGameTask(null).monitorProcess(gameProcess); + )) { + new NonTimingGameTask(null).monitorProcess(gameProcess); - detailedExpectation.assertObservation(); + detailedExpectation.assertObservation(); + } } @SlowTest @@ -108,8 +109,8 @@ public void testGameExitSuccessful() throws InterruptedException, ExecutionExcep var hasExitMessage = TestLoggers.sys().expect( RunGameTask.class.getName(), Level.DEBUG, allOf( - LogMatchers.hasFormatWithPattern("Game closed with the exit value.*"), - LogMatchers.hasArguments(EXIT_CODE_OK) + LogMatchers.hasFormatWithPattern("Game closed with the exit value.*"), + LogMatchers.hasArguments(EXIT_CODE_OK) ) ); @@ -128,8 +129,8 @@ public void testGameExitError() throws InterruptedException { var hasExitMessage = TestLoggers.sys().expect( RunGameTask.class.getName(), Level.DEBUG, allOf( - LogMatchers.hasFormatWithPattern("Game closed with the exit value.*"), - LogMatchers.hasArguments(EXIT_CODE_ERROR) + LogMatchers.hasFormatWithPattern("Game closed with the exit value.*"), + LogMatchers.hasArguments(EXIT_CODE_ERROR) ) ); @@ -322,7 +323,7 @@ public static Supplier renderColumns(Iterable actualIterable, Ite /** * An Iterator that runs the given callback every iteration. * - * @param list to be iterated over + * @param list to be iterated over * @param onNext to be called each iteration */ public static Iterator spyingIterator(List list, Runnable onNext) { @@ -332,7 +333,9 @@ public static Iterator spyingIterator(List list, Runnable onNext) { }).iterator(); } - /** Things that happen in RunGameTask that we want to make assertions about. */ + /** + * Things that happen in RunGameTask that we want to make assertions about. + */ enum Happenings { PROCESS_OUTPUT_LINE, TASK_VALUE_SET, diff --git a/src/test/java/org/terasology/launcher/repositories/JenkinsClientTest.java b/src/test/java/org/terasology/launcher/repositories/JenkinsClientTest.java index da91fde8d..4dfb5ee0d 100644 --- a/src/test/java/org/terasology/launcher/repositories/JenkinsClientTest.java +++ b/src/test/java/org/terasology/launcher/repositories/JenkinsClientTest.java @@ -34,6 +34,7 @@ */ @DisplayName("JenkinsClient") @ExtendWith(MockitoExtension.class) +@SuppressWarnings({"PMD.CloseResource", "PMD.AvoidDuplicateLiterals"}) class JenkinsClientTest { @Test diff --git a/src/test/java/org/terasology/launcher/repositories/JenkinsPayload.java b/src/test/java/org/terasology/launcher/repositories/JenkinsPayload.java index bcac74a50..60573e49c 100644 --- a/src/test/java/org/terasology/launcher/repositories/JenkinsPayload.java +++ b/src/test/java/org/terasology/launcher/repositories/JenkinsPayload.java @@ -5,6 +5,7 @@ import java.util.List; +@SuppressWarnings("PMD.AvoidDuplicateLiterals") final class JenkinsPayload { private JenkinsPayload() {