Skip to content

Commit

Permalink
qa: address PMD AvoidPrintStackTrace findings
Browse files Browse the repository at this point in the history
  • Loading branch information
@jdrueckert
jdrueckert committed Aug 18, 2024
1 parent 4a2ba11 commit 7c25f44
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 8 deletions.
2 changes: 1 addition & 1 deletion engine/src/main/java/org/terasology/engine/core/LoggingContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public static void initialize(Path logFileFolder) {
try {
deleteLogFiles(logFileFolder, Duration.ofDays(5).getSeconds());
} catch (IOException e) {
e.printStackTrace();
e.printStackTrace(); //NOPMD
}

// Unfortunately, setting context-based variables works only after initialization
Expand Down
2 changes: 1 addition & 1 deletion engine/src/main/java/org/terasology/engine/core/module/ModuleInstaller.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private Map<URI, Path> getDownloadUrls(Iterable<Module> modules) {
try {
uri = RemoteModuleExtension.getDownloadUrl(metadata).toURI();
} catch (URISyntaxException e) {
e.printStackTrace();
logger.error("Couldn't get download URL: ", e);
}
String fileName = String.format("%s-%s.jar", id, version);
Path folder = PathManager.getInstance().getHomeModPath().normalize();
Expand Down
6 changes: 5 additions & 1 deletion ...ne/src/main/java/org/terasology/engine/core/module/rendering/RenderingModuleRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@
// SPDX-License-Identifier: Apache-2.0
package org.terasology.engine.core.module.rendering;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.terasology.engine.context.Context;
import org.terasology.engine.rendering.dag.ModuleRendering;
import org.terasology.engine.rendering.nui.layers.mainMenu.UniverseSetupScreen;

Check warning on line 9 in engine/src/main/java/org/terasology/engine/core/module/rendering/RenderingModuleRegistry.java

View check run for this annotation

Terasology Jenkins.io / CheckStyle

UnusedImportsCheck 

NORMAL:
Unused import - org.terasology.engine.rendering.nui.layers.mainMenu.UniverseSetupScreen.
Raw output 
<p>Since Checkstyle 3.0</p><p> Checks for unused import statements. Checkstyle uses a simple but very reliable algorithm to report on unused import statements. An import statement is considered unused if: </p><ul><li> It is not referenced in the file. The algorithm does not support wild-card imports like <code>import java.io.*;</code>. Most IDE's provide very sophisticated checks for imports that handle wild-card imports. </li><li> It is a duplicate of another import. This is when a class is imported more than once. </li><li> The class imported is from the <code>java.lang</code> package. For example importing <code>java.lang.String</code>. </li><li> The class imported is from the same package. </li><li><b>Optionally:</b> it is referenced in Javadoc comments. This check is on by default, but it is considered bad practice to introduce a compile time dependency for documentation purposes only. As an example, the import <code>java.util.Date</code> would be considered referenced with the Javadoc comment <code>{@link Date}</code>. The alternative to avoid introducing a compile time dependency would be to write the Javadoc comment as <code>{@link java.util.Date}</code>. </li></ul><p> The main limitation of this check is handling the case where an imported type has the same name as a declaration, such as a member variable. </p><p> For example, in the following case the import <code>java.awt.Component</code> will not be flagged as unused: </p><pre><code> import java.awt.Component; class FooBar { private Object Component; // a bad practice in my opinion ... } </code></pre>
import org.terasology.gestalt.module.ModuleEnvironment;
import org.terasology.context.annotation.API;
import org.terasology.gestalt.naming.Name;
Expand All @@ -20,6 +23,7 @@

@API
public class RenderingModuleRegistry {
private static final Logger LOGGER = LoggerFactory.getLogger(RenderingModuleRegistry.class);

private List<ModuleRendering> orderedModuleRenderingInstances = new ArrayList<>();

Expand Down Expand Up @@ -126,7 +130,7 @@ private Set<ModuleRendering> fetchRenderingModules(ModuleEnvironment moduleEnvir
Constructor<?> constructor = renderingClass.getConstructor(Context.class);
moduleRenderingInstance = (ModuleRendering) constructor.newInstance(context);
} catch (InstantiationException | IllegalAccessException | NoSuchMethodException | InvocationTargetException e) {
e.printStackTrace();
LOGGER.error("Couldn't get constructor: ", e);
}
}
if (moduleRenderingInstance != null) {
Expand Down
1 change: 1 addition & 0 deletions engine/src/main/java/org/terasology/engine/logic/behavior/asset/GroupBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* @see Group
*/
@API
@SuppressWarnings("PMD.AvoidPrintStackTrace")
public class GroupBuilder {

private GroupData groupData;
Expand Down
2 changes: 1 addition & 1 deletion engine/src/main/java/org/terasology/engine/logic/debug/MovementDebugCommands.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ public String playerEyeHeight(@Sender EntityRef client, @CommandParam("eye-heigh
}
return "";
} catch (NullPointerException e) {
e.printStackTrace();
logger.error("Couldn't set player eye height: e");
return "";
}
}
Expand Down
2 changes: 1 addition & 1 deletion engine/src/main/java/org/terasology/engine/network/internal/ServerConnectListManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ private void saveLists() {
whitelistWriter.close();
}
} catch (IOException e) {
e.printStackTrace();
logger.error("Couldn't save lists: ", e);
}
}

Expand Down
9 changes: 7 additions & 2 deletions ...rc/main/java/org/terasology/engine/rendering/nui/layers/mainMenu/UniverseSetupScreen.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.terasology.engine.config.Config;
import org.terasology.engine.context.Context;
import org.terasology.engine.core.GameEngine;
Expand Down Expand Up @@ -42,6 +44,7 @@
import org.terasology.engine.world.generator.internal.WorldGeneratorManager;
import org.terasology.engine.world.generator.plugin.TempWorldGeneratorPluginLibrary;
import org.terasology.engine.world.generator.plugin.WorldGeneratorPluginLibrary;
import org.terasology.engine.world.time.WorldTimeImpl;

Check warning on line 47 in engine/src/main/java/org/terasology/engine/rendering/nui/layers/mainMenu/UniverseSetupScreen.java

View check run for this annotation

Terasology Jenkins.io / CheckStyle

UnusedImportsCheck 

NORMAL:
Unused import - org.terasology.engine.world.time.WorldTimeImpl.
Raw output 
<p>Since Checkstyle 3.0</p><p> Checks for unused import statements. Checkstyle uses a simple but very reliable algorithm to report on unused import statements. An import statement is considered unused if: </p><ul><li> It is not referenced in the file. The algorithm does not support wild-card imports like <code>import java.io.*;</code>. Most IDE's provide very sophisticated checks for imports that handle wild-card imports. </li><li> It is a duplicate of another import. This is when a class is imported more than once. </li><li> The class imported is from the <code>java.lang</code> package. For example importing <code>java.lang.String</code>. </li><li> The class imported is from the same package. </li><li><b>Optionally:</b> it is referenced in Javadoc comments. This check is on by default, but it is considered bad practice to introduce a compile time dependency for documentation purposes only. As an example, the import <code>java.util.Date</code> would be considered referenced with the Javadoc comment <code>{@link Date}</code>. The alternative to avoid introducing a compile time dependency would be to write the Javadoc comment as <code>{@link java.util.Date}</code>. </li></ul><p> The main limitation of this check is handling the case where an imported type has the same name as a declaration, such as a member variable. </p><p> For example, in the following case the import <code>java.awt.Component</code> will not be flagged as unused: </p><pre><code> import java.awt.Component; class FooBar { private Object Component; // a bad practice in my opinion ... } </code></pre>
import org.terasology.engine.world.zones.Zone;
import org.terasology.gestalt.assets.AssetType;
import org.terasology.gestalt.assets.ResourceUrn;
Expand Down Expand Up @@ -94,6 +97,8 @@
* for a particular game template.
*/
public class UniverseSetupScreen extends CoreScreenLayer implements UISliderOnChangeTriggeredListener, PropertyChangeListener {
private static final Logger LOGGER = LoggerFactory.getLogger(UniverseSetupScreen.class);

public static final ResourceUrn ASSET_URI = new ResourceUrn("engine:universeSetupScreen");

Check warning on line 102 in engine/src/main/java/org/terasology/engine/rendering/nui/layers/mainMenu/UniverseSetupScreen.java

View check run for this annotation

Terasology Jenkins.io / CheckStyle

DeclarationOrderCheck 

NORMAL:
Variable access definition in wrong order.
Raw output 
<p>Since Checkstyle 3.2</p><p> According to <a href="https://www.oracle.com/technetwork/java/javase/documentation/codeconventions-141855.html#1852"> Code Conventions for the Java Programming Language</a> , the parts of a class or interface declaration should appear in the following order: </p><ol><li> Class (static) variables. First the public class variables, then protected, then package level (no access modifier), and then private. </li><li> Instance variables. First the public class variables, then protected, then package level (no access modifier), and then private. </li><li> Constructors </li><li> Methods </li></ol><p> Purpose of <b>ignore*</b> option is to ignore related violations, however it still impacts on other class members. </p><p> ATTENTION: the check skips class fields which have <a href="https://docs.oracle.com/javase/specs/jls/se8/html/jls-8.html#jls-8.3.3"> forward references</a> from validation due to the fact that we have Checkstyle's limitations to clearly detect user intention of fields location and grouping. For example, <pre><code> public class A { private double x = 1.0; private double y = 2.0; public double slope = x / y; // will be skipped from validation due to forward reference } </code></pre></p>

@In
Expand Down Expand Up @@ -354,10 +359,10 @@ private void addNewWorld(WorldGeneratorInfo worldGeneratorInfo) {
} catch (UnresolvedWorldGeneratorException e) {
getManager().pushScreen(MessagePopup.ASSET_URI, MessagePopup.class).setMessage("Selected world generator cannot be resolved!",
"Please report this issue on Discord/GitHub and select a different world generator!");
e.printStackTrace();
LOGGER.error("Selected world generator cannot be resolved: ", e);
} catch (UnresolvedDependencyException e) {
//TODO: this will likely fail at game creation time later-on due to lack of world generator - don't just ignore this

Check warning on line 364 in engine/src/main/java/org/terasology/engine/rendering/nui/layers/mainMenu/UniverseSetupScreen.java

View check run for this annotation

Terasology Jenkins.io / Open Tasks Scanner

TODO 

NORMAL:
this will likely fail at game creation time later-on due to lack of world generator - don't just ignore this
e.printStackTrace();
LOGGER.error("Selected world generator cannot be resolved: ", e);
}

configureProperties();
Expand Down
2 changes: 1 addition & 1 deletion facades/PC/src/main/java/org/terasology/engine/Terasology.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ private void populateSubsystems(TerasologyEngineBuilder builder) {
builder.add(new HibernationSubsystem());
}

@SuppressWarnings("PMD.SystemPrintln")
@SuppressWarnings({"PMD.SystemPrintln", "PMD.AvoidPrintStackTrace"})
private void reportException(Throwable throwable) {
Path logPath = LoggingContext.getLoggingPath();

Expand Down

0 comments on commit 7c25f44

Please sign in to comment.