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

refactor: address IntelliJ QAPlug plugin findings #5149

Merged
merged 35 commits into from
Nov 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
aa6ca30
qa: make final fields static
jdrueckert Oct 22, 2023
1a53322
qa: remove unnecessary local before return
jdrueckert Oct 22, 2023
db4f626
qa: avoid instanceof checks in catch clause
jdrueckert Oct 22, 2023
e3d80fb
qa: method/constructor should not throw java.lang.Exception
jdrueckert Oct 22, 2023
f22c1fa
qa: instanceof check covers null check
jdrueckert Oct 22, 2023
9d7204b
qa: avoid toString on String objects
jdrueckert Oct 22, 2023
993b370
qa: remove overrides that only call their super
jdrueckert Oct 22, 2023
315ef35
qa: remove unused private methods
jdrueckert Oct 22, 2023
3c5d507
qa: if statement nesting not necessary
jdrueckert Oct 22, 2023
df3539a
qa: remove unused local variable
jdrueckert Oct 22, 2023
b0ec929
qa: close resources after use
jdrueckert Oct 22, 2023
06c5fcf
qa: use equals() to compare object references
jdrueckert Oct 22, 2023
3428369
Merge branch 'develop' into chore/qaplug-findings
jdrueckert Oct 27, 2023
a3f8fbc
fix: reintroduce override, needed to elevate access modifier
jdrueckert Oct 27, 2023
760a174
fix: exception type in override
jdrueckert Oct 27, 2023
f067ba2
chore: replace invalid javadoc character with unicode representation
jdrueckert Oct 27, 2023
077d488
javadoc: fix broken references
jdrueckert Oct 28, 2023
6a816ed
javadoc: fix disallowed self-closing and empty elements
jdrueckert Oct 28, 2023
a27bd8e
javadoc: fix invalid uses of @param and @return
jdrueckert Oct 28, 2023
c93ad63
javadoc: fix bad use of symbols
jdrueckert Oct 28, 2023
ad27ad0
javadoc: fix disallowed list item tag without surrounding list
jdrueckert Oct 28, 2023
cce1a8b
javadoc: fix erroneous tags
jdrueckert Oct 28, 2023
45e6571
javadoc: fix more references
jdrueckert Oct 28, 2023
35c2bc3
javadoc: fix emphasize tags
jdrueckert Oct 28, 2023
60a41a5
chore: fix missing javadoc errors
jdrueckert Oct 28, 2023
3d13e1f
javadoc: fix @see
jdrueckert Oct 29, 2023
848947e
checkstyle: remove illegal throws
jdrueckert Oct 30, 2023
f585450
checkstyle: remove unused imports
jdrueckert Oct 30, 2023
01e8edd
refactor: replace explicit close with try-with-resources
jdrueckert Oct 30, 2023
b841271
revert: removal of unused param from AbstractFamily constructor
jdrueckert Oct 30, 2023
2aad50f
revert: close zip Filesystem
jdrueckert Oct 30, 2023
49725fa
revert: removing required zip.close
jdrueckert Oct 30, 2023
bf26781
Merge branch 'develop' into chore/qaplug-findings
jdrueckert Oct 31, 2023
971c853
Merge branch 'develop' into chore/qaplug-findings
jdrueckert Nov 6, 2023
08bd7ea
Merge branch 'develop' into chore/qaplug-findings
jdrueckert Nov 11, 2023
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 @@ -37,16 +37,14 @@ class Environment {
Environment(Name... moduleNames) {
try {
reset(Sets.newHashSet(moduleNames));
} catch (RuntimeException e) {
throw e;
} catch (Exception e) {
if (e instanceof RuntimeException) {
throw (RuntimeException) e;
} else {
throw new RuntimeException(e);
}
throw new RuntimeException(e);
}
}

protected void reset(Set<Name> moduleNames) throws Exception {
protected void reset(Set<Name> moduleNames) throws IOException {
this.context = new ContextImpl();
RecordAndReplayCurrentStatus recordAndReplayCurrentStatus = new RecordAndReplayCurrentStatus();
context.put(RecordAndReplayCurrentStatus.class, recordAndReplayCurrentStatus);
Expand Down Expand Up @@ -99,7 +97,7 @@ protected void setupPathManager() throws IOException {
PathManager.getInstance();
}

protected void setupModuleManager(Set<Name> moduleNames) throws Exception {
protected void setupModuleManager(Set<Name> moduleNames) {
// empty
}

Expand Down Expand Up @@ -160,9 +158,9 @@ protected void setupCelestialSystem() {
/**
* Cleans up all resources (similar to AutoCloseable)
*
* @throws Exception if something goes wrong
* @throws RuntimeException if something goes wrong
*/
public void close() throws Exception {
public void close() {
CoreRegistry.setContext(null);
context = null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ protected void setupConfig() {
}

@Override
protected void setupModuleManager(Set<Name> moduleNames) throws Exception {
protected void setupModuleManager(Set<Name> moduleNames) throws RuntimeException {
TypeRegistry.WHITELISTED_CLASSES = ExternalApiWhitelist.CLASSES.stream().map(Class::getName).collect(Collectors.toSet());

ModuleManager moduleManager = ModuleManagerFactory.create();
Expand Down Expand Up @@ -320,7 +320,7 @@ protected void loadPrefabs() {
}

@Override
public void close() throws Exception {
public void close() throws RuntimeException {
// it would be nice, if elements in the context implemented (Auto)Closeable

// The StorageManager creates a thread pool (through TaskMaster)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.terasology.reflection.ModuleTypeRegistry;
import org.terasology.reflection.TypeRegistry;

import java.io.IOException;
import java.nio.file.Path;
import java.util.stream.Collectors;

Expand All @@ -21,7 +22,7 @@ public abstract class ModuleEnvironmentTest {
protected ModuleTypeRegistry typeRegistry;

@BeforeEach
public void before(@TempDir Path tempHome) throws Exception {
public void before(@TempDir Path tempHome) throws IOException {
PathManager.getInstance().useOverrideHomePath(tempHome);

moduleManager = ModuleManagerFactory.create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public final class ModuleManagerFactory {
private ModuleManagerFactory() {
}

public static ModuleManager create() throws Exception {
public static ModuleManager create() {
// Loading screens, among other things, break when NUI classes are not added to engine.
ModuleManager moduleManager = new ModuleManager("", ImmutableList.of(UIWidget.class));
Module unittestModule = moduleManager.registerPackageModule("org.terasology.unittest");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ public long skip(long n) throws IOException {
/**
* Initalizes the vorbis stream. Reads the stream until info and comment are read.
*/
private void initVorbis() throws Exception {
private void initVorbis() throws IOException {
// Now we can read pages
syncState.init();

Expand All @@ -283,7 +283,7 @@ private void initVorbis() throws Exception {
return; //break;
}
// error case. Must not be Vorbis data
throw new Exception("Input does not appear to be an Ogg bitstream.");
throw new IOException("Input does not appear to be an Ogg bitstream.");
}

// Get the serial number and set up the rest of decode.
Expand All @@ -302,15 +302,15 @@ private void initVorbis() throws Exception {
comment.init();
if (streamState.pagein(page) < 0) {
// error; stream version mismatch perhaps
throw new Exception("Error reading first page of Ogg bitstream data.");
throw new IOException("Error reading first page of Ogg bitstream data.");
}
if (streamState.packetout(packet) != 1) {
// no page? must not be vorbis
throw new Exception("Error reading initial header packet.");
throw new IOException("Error reading initial header packet.");
}
if (info.synthesis_headerin(comment, packet) < 0) {
// error case; not a vorbis header
throw new Exception("This Ogg bitstream does not contain Vorbis audio data.");
throw new IOException("This Ogg bitstream does not contain Vorbis audio data.");
}

// At this point, we're sure we're Vorbis. We've set up the logical
Expand Down Expand Up @@ -346,7 +346,7 @@ private void initVorbis() throws Exception {
if (result == -1) {
// Uh oh; data at some point was corrupted or missing!
// We can't tolerate that in a header. Die.
throw new Exception("Corrupt secondary header. Exiting.");
throw new IOException("Corrupt secondary header. Exiting.");
}
info.synthesis_headerin(comment, packet);
i++;
Expand All @@ -364,7 +364,7 @@ private void initVorbis() throws Exception {
bytes = 0;
}
if (bytes == 0 && i < 2) {
throw new Exception("End of file before finding all Vorbis headers!");
throw new IOException("End of file before finding all Vorbis headers!");
}
syncState.wrote(bytes);
}
Expand All @@ -386,7 +386,7 @@ private void initVorbis() throws Exception {
*/
private int decodePacket() {
// check the endianes of the computer.
final boolean bigEndian = ByteOrder.nativeOrder() == ByteOrder.BIG_ENDIAN;
final boolean bigEndian = ByteOrder.nativeOrder().equals(ByteOrder.BIG_ENDIAN);

if (block.synthesis(packet) == 0) {
dspState.synthesis_blockin(block);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,8 @@ public BaseSoundPool() {

public SOURCE getLockedSource() {
for (SOURCE source : soundSources.keySet()) {
if (!isActive(source)) {
if (lock(source)) {
return source;
}
if (!isActive(source) && lock(source)) {
return source;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ private float findClosestIndex(Color color) {

private Color findClosestColor(float findex) {
int index = DoubleMath.roundToInt(findex * (double) (colors.size() - 1), RoundingMode.HALF_UP);
Color color = colors.get(index);
return color;
return colors.get(index);
}
}
10 changes: 5 additions & 5 deletions engine/src/main/java/org/terasology/engine/core/GameThread.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ private GameThread() {
* @return Whether the currentThread is the gameThread.
*/
public static boolean isCurrentThread() {
return Thread.currentThread() == gameThread;
return Thread.currentThread().equals(gameThread);
}

/**
Expand All @@ -41,7 +41,7 @@ public static boolean isCurrentThread() {
* @param process
*/
public static void asynch(Runnable process) {
if (Thread.currentThread() != gameThread) {
if (!Thread.currentThread().equals(gameThread)) {
pendingRunnables.push(process);
} else {
process.run();
Expand All @@ -56,7 +56,7 @@ public static void asynch(Runnable process) {
* @param process
*/
public static void synch(Runnable process) throws InterruptedException {
if (Thread.currentThread() != gameThread) {
if (!Thread.currentThread().equals(gameThread)) {
BlockingProcess blockingProcess = new BlockingProcess(process);
pendingRunnables.push(blockingProcess);
blockingProcess.waitForCompletion();
Expand All @@ -69,7 +69,7 @@ public static void synch(Runnable process) throws InterruptedException {
* Runs all pending processes submitted from other threads
*/
public static void processWaitingProcesses() {
if (Thread.currentThread() == gameThread) {
if (Thread.currentThread().equals(gameThread)) {
List<Runnable> processes = Lists.newArrayList();
pendingRunnables.drainTo(processes);
processes.forEach(Runnable::run);
Expand All @@ -80,7 +80,7 @@ public static void processWaitingProcesses() {
* Removes all pending processes without running them
*/
public static void clearWaitingProcesses() {
if (gameThread == Thread.currentThread()) {
if (gameThread.equals(Thread.currentThread())) {
pendingRunnables.clear();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public AbstractEventSystemDecorator(EventSystem eventSystem) {
}

public boolean currentThreadIsMain() {
return mainThread == Thread.currentThread();
return mainThread.equals(Thread.currentThread());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

import com.google.common.base.MoreObjects;
import org.terasology.engine.audio.AudioManager;
import org.terasology.engine.config.Config;
import org.terasology.engine.config.TelemetryConfig;
import org.terasology.engine.core.GameEngine;
import org.terasology.engine.core.LoggingContext;
import org.terasology.engine.core.modes.loadProcesses.RegisterInputSystem;
Expand All @@ -17,11 +15,7 @@
import org.terasology.engine.rendering.nui.NUIManager;
import org.terasology.engine.rendering.nui.editor.systems.NUIEditorSystem;
import org.terasology.engine.rendering.nui.editor.systems.NUISkinEditorSystem;
import org.terasology.engine.rendering.nui.layers.mainMenu.LaunchPopup;
import org.terasology.engine.rendering.nui.layers.mainMenu.MessagePopup;
import org.terasology.engine.telemetry.TelemetryScreen;
import org.terasology.engine.telemetry.TelemetryUtils;
import org.terasology.engine.telemetry.logstash.TelemetryLogstashAppender;
import org.terasology.engine.utilities.Assets;

/**
Expand Down Expand Up @@ -97,35 +91,6 @@ public void init(GameEngine gameEngine) {
// pushLaunchPopup();
}

private void pushLaunchPopup() {
Config config = context.get(Config.class);
TelemetryConfig telemetryConfig = config.getTelemetryConfig();
TranslationSystem translationSystem = context.get(TranslationSystem.class);
TelemetryLogstashAppender appender = TelemetryUtils.fetchTelemetryLogstashAppender();
if (!telemetryConfig.isLaunchPopupDisabled()) {
String telemetryTitle = translationSystem.translate("${engine:menu#telemetry-launch-popup-title}");
String telemetryMessage = translationSystem.translate("${engine:menu#telemetry-launch-popup-text}");
LaunchPopup telemetryConfirmPopup = nuiManager.pushScreen(LaunchPopup.ASSET_URI, LaunchPopup.class);
telemetryConfirmPopup.setMessage(telemetryTitle, telemetryMessage);
telemetryConfirmPopup.setYesHandler(() -> {
telemetryConfig.setTelemetryAndErrorReportingEnable(true);

// Enable error reporting
appender.start();
});
telemetryConfirmPopup.setNoHandler(() -> {
telemetryConfig.setTelemetryAndErrorReportingEnable(false);

// Disable error reporting
appender.stop();
});
telemetryConfirmPopup.setOptionButtonText(translationSystem.translate("${engine:menu#telemetry-button}"));
telemetryConfirmPopup.setOptionHandler(() -> {
nuiManager.pushScreen(TelemetryScreen.ASSET_URI, TelemetryScreen.class);
});
}
}

@Override
public void dispose(boolean shuttingDown) {
stopBackgroundMusic();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import java.util.Date;
/**
* A set of module extensions for remote modules.
* NOTE: this is copy&paste from meta-server.
* NOTE: this is copy&amp;paste from meta-server.
*/
public enum RemoteModuleExtension {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package org.terasology.engine.core.subsystem.config;

import org.terasology.engine.config.BindsConfig;
import org.terasology.engine.context.Context;
import org.terasology.engine.core.SimpleUri;
import org.terasology.engine.input.BindAxisEvent;
import org.terasology.engine.input.BindButtonEvent;
Expand Down Expand Up @@ -35,14 +34,14 @@
public interface BindsManager {

/**
* The actual binds config. This reflects the current status from {@link #updateDefaultBinds(Context)}
* The actual binds config. This reflects the current status from {@link BindsManager#updateConfigWithDefaultBinds()}.
* and all further modifications like modifications in the user input settings.
* @return

Check warning on line 39 in engine/src/main/java/org/terasology/engine/core/subsystem/config/BindsManager.java

View check run for this annotation

Terasology Jenkins.io / JavaDoc

JavaDoc @return

NORMAL: no description for @return
*/
BindsConfig getBindsConfig();

/**
* The default bindings. This reflects the current status from {@link #updateDefaultBinds(Context)}.
* The default bindings. This reflects the current status from {@link BindsManager#updateConfigWithDefaultBinds()}.
* @return Returns the default bindings. Changes to this config modify the actual instance
* but become invalid the next time {@link #updateConfigWithDefaultBinds()} is called.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ private BindableAxis registerBindAxis(String id, BindAxisEvent event, BindableBu
}

private BindableAxis registerRealBindAxis(String id, BindAxisEvent event) {
BindableRealAxis axis = new BindableRealAxis(id.toString(), event);
BindableRealAxis axis = new BindableRealAxis(id, event);
axisBinds.add(axis);
axisLookup.put(id, axis);
return axis;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,9 @@ public boolean addPrefab(String prefabName) {
}
}
/**
* Adds all of the components from a prefab to this builder
* Adds all components from a prefab to this builder
*
* @param prefab the prefab to add
* @return whether the prefab was successfully added
*/
public void addPrefab(Prefab prefab) {
if (prefab != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@ public interface EntityPool {
* All entities containing every one of the given components.
* <p>
* Implementation note:
* <a href="http://www.angelikalanger.com/GenericsFAQ/FAQSections/ProgrammingIdioms.html#Topic5"
* title="Designing Generic Methods">Java generic types are a mess</a>, especially where
* varargs are involved. We can't use {@link SafeVarargs @SafeVarargs} on any interface methods
* Java generic types are a mess (see <a href="http://www.angelikalanger.com/GenericsFAQ/FAQSections/ProgrammingIdioms.html#Topic5">
* Designing Generic Methods</a>), especially where varargs are involved.
* We can't use {@link SafeVarargs @SafeVarargs} on any interface methods
* because there is no way to know the <em>implementations</em> of the interface are safe.
* <p>
* In this case, in practice, there are few (if any) callers that pass more than two values.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,8 @@ public boolean isAlwaysRelevant() {

@Override
public void setAlwaysRelevant(boolean alwaysRelevant) {
if (exists()) {
if (alwaysRelevant != isAlwaysRelevant()) {
setScope(alwaysRelevant ? GLOBAL : CHUNK);
}
if (exists() && alwaysRelevant != isAlwaysRelevant()) {
setScope(alwaysRelevant ? GLOBAL : CHUNK);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,13 +605,9 @@

public Optional<EngineEntityPool> getPool(long id) {
Optional<EngineEntityPool> pool = Optional.ofNullable(poolMap.get(id));
if (!pool.isPresent()) {
if (id != NULL_ID) {
// TODO: Entity pools assignment is not needed as of now, can be enabled later on when necessary.
if (!isExistingEntity(id)) {
logger.error("Entity {} doesn't exist", id);
}
}
// TODO: Entity pools assignment is not needed as of now, can be enabled later on when necessary.

Check warning on line 608 in engine/src/main/java/org/terasology/engine/entitySystem/entity/internal/PojoEntityManager.java

View check run for this annotation

Terasology Jenkins.io / Open Tasks Scanner

TODO

NORMAL: Entity pools assignment is not needed as of now, can be enabled later on when necessary.
if (!pool.isPresent() && id != NULL_ID && !isExistingEntity(id)) {
logger.error("Entity {} doesn't exist", id);
}
return pool;
}
Expand Down Expand Up @@ -707,16 +703,6 @@
}
}

/**
* This method gets called when the entity gets reactivated. e.g. after storage an entity needs to be reactivated.
*/
private void notifyReactivation(EntityRef entity, Collection<Component> components) {
for (EntityChangeSubscriber subscriber : subscribers) {
subscriber.onReactivation(entity, components);
}
}


/**
* This method gets called before an entity gets deactivated (e.g. for storage).
*/
Expand Down
Loading
Loading