Skip to content
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

qa: address PMD GuardLogStatement warnings #5209

Merged
merged 30 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
3b23f5d
qa engine, audio, fluent logger.
soloturn Dec 2, 2023
e9c4f14
qa engine, config, fluent logger.
soloturn Dec 2, 2023
496fcca
qa engine, core, fluent logger.
soloturn Dec 2, 2023
6611111
qa engine, entitySystem, fluent logger.
soloturn Dec 2, 2023
23e58cc
qa engine, game, fluent logger.
soloturn Dec 2, 2023
c182e76
qa engine, identity, fluent logger.
soloturn Dec 2, 2023
cf84823
qa engine, input, fluent logger.
soloturn Dec 2, 2023
f0b2bab
qa engine, logic, fluent logger.
soloturn Dec 2, 2023
d5a0dba
qa engine, network, fluent logger.
soloturn Dec 2, 2023
3f69b34
qa engine, particles, fluent logger.
soloturn Dec 2, 2023
c1230d3
qa engine, persistence, fluent logger.
soloturn Dec 2, 2023
3cd9936
qa engine, physics, fluent logger.
soloturn Dec 2, 2023
c3c6c4b
qa engine, recording, fluent logger.
soloturn Dec 2, 2023
87fae23
qa engine, registry, fluent logger.
soloturn Dec 2, 2023
f4487b8
qa engine, rendering, fluent logger.
soloturn Dec 2, 2023
1a7fe0b
qa engine, utilities, fluent logger.
soloturn Dec 2, 2023
2a293e5
qa engine, world, fluent logger.
soloturn Dec 2, 2023
be19e62
review feedback pr
soloturn Feb 12, 2024
e9fad3d
feedback fluent-logger pr. except to make log line shorter.
soloturn Feb 12, 2024
cf50c8e
Update engine/src/main/java/org/terasology/engine/rendering/opengl/Op…
soloturn Apr 7, 2024
f8b792c
line length warning removed
soloturn May 13, 2024
12ae29a
refactor: use less verbose varient of fluent API
jdrueckert May 17, 2024
3705eff
fix: //NOPMD doesn't work if log spans multiple lines
jdrueckert May 17, 2024
c48561f
refactor: introduce and reference local variables
jdrueckert May 17, 2024
56f9ed8
refactor: suppress guardlogstatement warnings for methods intended fo…
jdrueckert May 17, 2024
e4cb5fc
revert: unnecessary usage of fluent API
jdrueckert May 17, 2024
2f15a82
qa: address remaining PMD guardlogstatement warnings
jdrueckert May 17, 2024
eb7ed68
Merge branch 'develop' into qa/fluent-logger
jdrueckert May 17, 2024
4ad29ed
fix: log format
jdrueckert May 17, 2024
f4b34ff
Merge remote-tracking branch 'origin/qa/fluent-logger' into qa/fluent…
jdrueckert May 17, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public class OpenALManager implements AudioManager {

private final Map<SoundSource<?>, AudioEndListener> endListeners = Maps.newHashMap();

@SuppressWarnings("PMD.GuardLogStatement")
public OpenALManager(AudioConfig config) throws OpenALException {
logger.info("Initializing OpenAL audio manager");

Expand All @@ -65,7 +66,6 @@ public OpenALManager(AudioConfig config) throws OpenALException {
AL.createCapabilities(alcCapabilities);

logger.info("OpenAL {} initialized!", AL10.alGetString(AL10.AL_VERSION));

logger.info("Using OpenAL: {} by {}", AL10.alGetString(AL10.AL_RENDERER), AL10.alGetString(AL10.AL_VENDOR));
logger.info("Using device: {}", ALC10.alcGetString(device, ALC10.ALC_DEVICE_SPECIFIER));
logger.info("Available AL extensions: {}", AL10.alGetString(AL10.AL_EXTENSIONS));
Expand Down Expand Up @@ -260,7 +260,7 @@ private void notifyEndListeners(boolean interrupted) {
try {
entry.getValue().onAudioEnd(interrupted);
} catch (Exception e) {
logger.error("onAudioEnd() notification failed for {}", entry.getValue(), e);
logger.error("onAudioEnd() notification failed for {}", entry.getValue(), e); //NOPMD
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ protected void doReload(StaticSoundData newData) {
length = (float) size / channels / (bits / 8) / frequency;
});
} catch (InterruptedException e) {
logger.error("Failed to reload {}", getUrn(), e);
logger.error("Failed to reload {}", getUrn(), e); //NOPMD
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ protected void doReload(StreamingSoundData data) {
try {
GameThread.synch(this::initializeBuffers);
} catch (InterruptedException e) {
logger.error("Failed to reload {}", getUrn(), e);
logger.error("Failed to reload {}", getUrn(), e); //NOPMD
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,6 @@ public void setRenderSkeletons(boolean renderSkeletons) {

@Override
public void propertyChange(PropertyChangeEvent evt) {
logger.debug("Set {} property to {}.", evt.getPropertyName().toUpperCase(), evt.getNewValue());
logger.atDebug().log("Set {} property to {}.", evt.getPropertyName().toUpperCase(), evt.getNewValue());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ private <T extends AutoConfig> void loadSettingsFromDisk(Class<T> configClass, T
T loadedConfig = (T) serializer.deserialize(TypeInfo.of(configClass), inputStream).get();
mergeConfig(configClass, loadedConfig, config);
} catch (Exception e) {
logger.error("Error while loading config {} from disk", config.getId(), e);
logger.error("Error while loading config {} from disk", config.getId(), e); //NOPMD
}
}

Expand Down Expand Up @@ -116,7 +116,7 @@ private void saveConfigToDisk(AutoConfig config) {
StandardOpenOption.CREATE)) {
serializer.serialize(config, TypeInfo.of((Class<AutoConfig>) config.getClass()), output);
} catch (IOException e) {
logger.error("Error while saving config {} to disk", config.getId(), e);
logger.error("Error while saving config {} to disk", config.getId(), e); //NOPMD
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ protected PersistedData serializeNonNull(AutoConfig value, PersistedDataSerializ
if (typeHandler.isPresent()) {
fields.put(field.getName(), typeHandler.get().serialize(setting.get(), serializer));
} else {
logger.error("Cannot serialize type [{}]", setting.getValueType());
logger.error("Cannot serialize type [{}]", setting.getValueType()); //NOPMD
}
}
} catch (IllegalAccessException e) {
Expand All @@ -65,7 +65,7 @@ public Optional<AutoConfig> deserialize(PersistedData data) {
for (Map.Entry<String, PersistedData> entry : data.getAsValueMap().entrySet()) {
Field settingField = settingFields.get(entry.getKey());
if (settingField == null) {
logger.warn("Cannot to find setting field with name [{}]", entry.getKey());
logger.warn("Cannot to find setting field with name [{}]", entry.getKey()); //NOPMD
continue;
}
try {
Expand All @@ -77,11 +77,10 @@ public Optional<AutoConfig> deserialize(PersistedData data) {
if (value.isPresent()) {
setting.set(value.get());
} else {
logger.error("Cannot deserialize value [{}] to type [{}]", entry.getValue(),
setting.getValueType());
logger.error("Cannot deserialize value [{}] to type [{}]", entry.getValue(), setting.getValueType()); //NOPMD
}
} else {
logger.error("Cannot deserialize type [{}]", setting.getValueType());
logger.error("Cannot deserialize type [{}]", setting.getValueType()); //NOPMD
}
} catch (IllegalAccessException e) {
// ignore, AutoConfig.getSettingsFieldsIn return public fields.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public boolean isSatisfiedBy(Locale value) {
}

@Override
@SuppressWarnings("PMD.GuardLogStatement")
public void warnUnsatisfiedBy(Locale value) {
logger.warn("Locale {} should be one of {}",
value,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,12 @@ public boolean isSatisfiedBy(T value) {
* {@inheritDoc}
*/
@Override
@SuppressWarnings("PMD.GuardLogStatement")
public void warnUnsatisfiedBy(T value) {
LOGGER.warn("Value {} is not in the range {}{}, {}{}", value, minInclusive ? "[" : "(",
min != null ? min : "UNBOUNDED", max != null ? max : "UNBOUNDED", maxInclusive ? "]" : ")");
LOGGER.warn("Value {} is not in the range {}{}, {}{}", value,
minInclusive ? "[" : "(",
min != null ? min : "UNBOUNDED",
max != null ? max : "UNBOUNDED",
maxInclusive ? "]" : ")");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public boolean isSatisfiedBy(String value) {
}

@Override
@SuppressWarnings("PMD.GuardLogStatement")
public void warnUnsatisfiedBy(String value) {
logger.warn("String [{}] does not match the conditions: {}", value,
predicates.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public void initialise() {
if (widget.isPresent()) {
mainContainer.addWidget(widget.get());
} else {
logger.warn("Cannot create widget for config: {}", config.getId());
logger.warn("Cannot create widget for config: {}", config.getId()); //NOPMD
}
}
WidgetUtil.trySubscribe(this, "close", button -> triggerBackAnimation());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public UIWidget buildWidgetFor(AutoConfig config) {
Optional<UIWidget> settingWidget = settingWidgetFactory.createWidgetFor(setting);

if (!settingWidget.isPresent()) {
logger.error("Couldn't find a widget for the Setting [{}]", setting.getHumanReadableName());
logger.error("Couldn't find a widget for the Setting [{}]", setting.getHumanReadableName()); //NOPMD
continue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void loadSystems(ModuleEnvironment environment, NetworkMode netMode) {
ListMultimap<Name, Class<?>> systemsByModule = ArrayListMultimap.create();
for (Class<?> type : environment.getTypesAnnotatedWith(RegisterSystem.class)) {
if (!ComponentSystem.class.isAssignableFrom(type)) {
logger.error("Cannot load {}, must be a subclass of ComponentSystem", type.getSimpleName());
logger.error("Cannot load {}, must be a subclass of ComponentSystem", type.getSimpleName()); //NOPMD
continue;
}
Name moduleId = environment.getModuleProviding(type);
Expand Down Expand Up @@ -177,7 +177,7 @@ public void register(ComponentSystem object) {
context.get(EntityManager.class).getEventSystem().registerEventHandler(object);

if (initialised) {
logger.warn("System {} registered post-init.", object.getClass().getName());
logger.atWarn().log("System {} registered post-init.", object.getClass().getName());
initialiseSystem(object);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ private static Path findInstallPath() {
URI urlToSource = PathManager.class.getProtectionDomain().getCodeSource().getLocation().toURI();
Path codeLocation = Paths.get(urlToSource);
installationSearchPaths.add(codeLocation);
LOGGER.info("PathManager: Initial code location is " + codeLocation.toAbsolutePath());
LOGGER.atInfo().log("PathManager: Initial code location is " + codeLocation.toAbsolutePath());
} catch (URISyntaxException e) {
LOGGER.error("PathManager: Failed to convert code location to path.", e);
}
Expand All @@ -90,7 +90,7 @@ private static Path findInstallPath() {
// Use the current directory as a fallback.
Path currentDirectory = Paths.get("").toAbsolutePath();
installationSearchPaths.add(currentDirectory);
LOGGER.info("PathManager: Working directory is " + currentDirectory);
LOGGER.info("PathManager: Working directory is {}", currentDirectory);

for (Path startPath : installationSearchPaths) {
Path installationPath = findNativesHome(startPath, 5);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ public void initialize() {
}

double seconds = 0.001 * totalInitTime.elapsed(TimeUnit.MILLISECONDS);
logger.info("Initialization completed in {}sec.", String.format("%.2f", seconds));
logger.info("Initialization completed in {}sec.", String.format("%.2f", seconds)); //NOPMD
}

private void verifyInitialisation() {
Expand All @@ -265,6 +265,7 @@ private void verifyInitialisation() {
/**
* Logs software, environment and hardware information.
*/
@SuppressWarnings("PMD.GuardLogStatement")
private void logEnvironmentInfo() {
logger.info("{}", TerasologyVersion.getInstance());
logger.info("Home path: {}", PathManager.getInstance().getHomePath());
Expand Down Expand Up @@ -543,7 +544,7 @@ public void cleanup() {
try {
subsystem.preShutdown();
} catch (RuntimeException e) {
logger.error("Error preparing to shutdown {} subsystem", subsystem.getName(), e);
logger.error("Error preparing to shutdown {} subsystem", subsystem.getName(), e); //NOPMD
}
}

Expand All @@ -553,7 +554,7 @@ public void cleanup() {
try {
subsystem.shutdown();
} catch (RuntimeException e) {
logger.error("Error shutting down {} subsystem", subsystem.getName(), e);
logger.error("Error shutting down {} subsystem", subsystem.getName(), e); //NOPMD
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ private void popStep() {
current = null;
if (!loadProcesses.isEmpty()) {
current = loadProcesses.remove();
logger.debug("{}", current.getMessage());
logger.debug("{}", current.getMessage()); //NOPMD
current.begin();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@
worldInfo.setSeed(random.nextString(16));
}

logger.info("World seed: \"{}\"", worldInfo.getSeed());
logger.info("World seed: \"{}\"", worldInfo.getSeed()); //NOPMD

// TODO: Separate WorldRenderer from world handling in general

Check warning on line 100 in engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseWorld.java

View check run for this annotation

Terasology Jenkins.io / Open Tasks Scanner

TODO

NORMAL: Separate WorldRenderer from world handling in general
WorldGeneratorManager worldGeneratorManager = context.get(WorldGeneratorManager.class);
WorldGenerator worldGenerator;
try {
Expand All @@ -106,7 +106,7 @@
worldGenerator.setWorldSeed(worldInfo.getSeed());
context.put(WorldGenerator.class, worldGenerator);
} catch (UnresolvedWorldGeneratorException | UnresolvedDependencyException e) {
logger.error("Unable to load world generator {}. Available world generators: {}",
logger.atError().log("Unable to load world generator {}. Available world generators: {}",
worldInfo.getWorldGenerator(), worldGeneratorManager.getWorldGenerators());
context.get(GameEngine.class).changeState(new StateMainMenu("Failed to resolve world generator."));
return true; // We need to return true, otherwise the loading state will just call us again immediately
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public boolean step() {
context.get(GameEngine.class).changeState(mainMenu);
return false;
} else {
logger.info("Activating module: {}:{}", moduleInfo.getName(), moduleInfo.getVersion());
logger.atInfo().log("Activating module: {}:{}", moduleInfo.getName(), moduleInfo.getVersion());
gameManifest.addModule(module.getId(), module.getVersion());
moduleSet.add(module);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public boolean step() {
boolean entityCreated = false;
for (EntityRef entity : em.getAllEntities()) {
entityCreated = true;
logger.error("Entity created before load: {}", entity.toFullDescription());
logger.atError().log("Entity created before load: {}", entity.toFullDescription());
}
if (entityCreated) {
throw new IllegalStateException("Entity creation detected during component system initialisation, game load aborting");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public boolean step() {
ModuleEnvironment env = moduleManager.loadEnvironment(result.getModules(), true);

for (Module moduleInfo : env.getModulesOrderedByDependencies()) {
logger.info("Activating module: {}:{}", moduleInfo.getId(), moduleInfo.getVersion());
logger.atInfo().log("Activating module: {}:{}", moduleInfo.getId(), moduleInfo.getVersion());
}

EnvironmentSwitchHandler environmentSwitchHandler = context.get(EnvironmentSwitchHandler.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class ModuleInstaller implements Callable<List<Module>> {
@Override
public List<Module> call() throws Exception {
Map<URI, Path> filesToDownload = getDownloadUrls(moduleList);
logger.info("Started downloading {} modules", filesToDownload.size());
logger.info("Started downloading {} modules", filesToDownload.size()); //NOPMD
MultiFileDownloader downloader = new MultiFileDownloader(filesToDownload, downloadProgressListener);
List<Path> downloadedModulesPaths = downloader.call();
logger.info("Module download completed, loading the new modules...");
Expand All @@ -48,7 +48,7 @@ public List<Module> call() throws Exception {
Module module = moduleManager.registerArchiveModule(filePath);
newInstalledModules.add(module);
} catch (IOException e) {
logger.warn("Could not load module {}", filePath.getFileName(), e);
logger.warn("Could not load module {}", filePath.getFileName(), e); //NOPMD
}
}
logger.info("Finished loading the downloaded modules");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ public ModuleRegistry call() throws IOException {
String json = gson.toJson(jObject);

ModuleMetadata meta = metaReader.read(new StringReader(json));
logger.debug("Read module {} - {}", meta.getId(), meta.getVersion());
logger.debug("Read module {} - {}", meta.getId(), meta.getVersion()); //NOPMD
modules.add(new Module(meta, new EmptyFileSource(), Collections.emptyList(), new Reflections(),
(c) -> false));
}

int count = modules.size();
logger.info("Retrieved {} {}", count, (count == 1) ? "entry" : "entries");
logger.info("Retrieved {} {}", count, (count == 1) ? "entry" : "entries"); //NOPMD
}
return modules;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,9 @@ private void loadModulesFromClassPath() {
continue;
}
if (registry.add(module)) {
logger.info("Loaded {} from {}", module.getId(), path);
logger.info("Loaded {} from {}", module.getId(), path); //NOPMD
} else {
logger.info("Module {} from {} was a duplicate; not registering this copy.", module.getId(), path);
logger.info("Module {} from {} was a duplicate; not registering this copy.", module.getId(), path); //NOPMD
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void preInitialise(Context rootContext) {
checkServerIdentity();

// TODO: Move to display subsystem
logger.info("Video Settings: {}", config.renderConfigAsJson(config.getRendering()));
logger.info("Video Settings: {}", config.renderConfigAsJson(config.getRendering())); //NOPMD

rootContext.put(Config.class, config);
//add facades
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,11 @@ private void registerAxisBinds(ModuleEnvironment environment) {
BindableButton positiveButton = getBindButton(new SimpleUri(info.positiveButton()));
BindableButton negativeButton = getBindButton(new SimpleUri(info.negativeButton()));
if (positiveButton == null) {
logger.warn("Failed to register axis \"{}\", missing positive button \"{}\"", id, info.positiveButton());
logger.atWarn().log("Failed to register axis \"{}\", missing positive button \"{}\"", id, info.positiveButton());
continue;
}
if (negativeButton == null) {
logger.warn("Failed to register axis \"{}\", missing negative button \"{}\"", id, info.negativeButton());
logger.atWarn().log("Failed to register axis \"{}\", missing negative button \"{}\"", id, info.negativeButton());
continue;
}
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public class GLFWErrorCallback implements GLFWErrorCallbackI {
private static final Logger logger = LoggerFactory.getLogger("GLFW");

@Override
@SuppressWarnings("PMD.GuardLogStatement")
public void invoke(int error, long description) {
logger.error("Received error. Code: {}, Description: {}", error, MemoryUtil.memASCII(description));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ public <T extends Component> T addComponent(long entityId, T component) {
if (!oldComponent.isPresent()) {
notifyComponentAdded(getEntity(entityId), component.getClass());
} else {
logger.error("Adding a component ({}) over an existing component for entity {}", component.getClass(), entityId);
logger.error("Adding a component ({}) over an existing component for entity {}", component.getClass(), entityId); //NOPMD
notifyComponentChanged(getEntity(entityId), component.getClass());
}

Expand Down Expand Up @@ -580,7 +580,7 @@ public void saveComponent(long entityId, Component component) {
.map(pool -> pool.getComponentStore().put(entityId, component));

if (!oldComponent.isPresent()) {
logger.error("Saving a component ({}) that doesn't belong to this entity {}", component.getClass(), entityId);
logger.error("Saving a component ({}) that doesn't belong to this entity {}", component.getClass(), entityId); //NOPMD
}
if (eventSystem != null) {
EntityRef entityRef = getEntity(entityId);
Expand Down
Loading
Loading