qa: logger qa, use parameterized logger (#5170)

to find occasions, used 'git grep -i logger | grep "+"'. a better way to do this might be openrewrite: https://docs.openrewrite.org/recipes/java/logging/parameterizedlogging

Co-authored-by: jdrueckert <jd.rueckert@googlemail.com>

engine/src/main/java/org/terasology/engine/config/flexible/AutoConfigTypeHandler.java

@@ -89,7 +89,7 @@ public Optional<AutoConfig> deserialize(PersistedData data) {
             }
             return Optional.of(config);
         } catch (InstantiationException | InvocationTargetException | NoSuchMethodException | IllegalAccessException e) {
-            logger.error("Cannot create type [" + typeInfo + "] for deserialization", e);
+            logger.error("Cannot create type [{}] for deserialization", typeInfo, e);
         }
         return Optional.empty();
     }

engine/src/main/java/org/terasology/engine/config/flexible/internal/SettingImpl.java

@@ -138,8 +138,8 @@ public boolean set(T newValue) {
         Preconditions.checkNotNull(newValue, "The value of a setting cannot be null.");
 
         if (override.get().isPresent()) {
-            LOGGER.warn("An attempt was made to overwrite the value specified in the System property." +
-                    " This will give nothing while the System Property value is supplied");
+            LOGGER.warn("An attempt was made to overwrite the value specified in the System property."
+                    + " This will give nothing while the System Property value is supplied");
             return false;
         }
 

engine/src/main/java/org/terasology/engine/core/ComponentSystemManager.java

@@ -177,7 +177,7 @@ public void register(ComponentSystem object) {
         context.get(EntityManager.class).getEventSystem().registerEventHandler(object);
 
         if (initialised) {
-            logger.warn("System " + object.getClass().getName() + " registered post-init.");
+            logger.warn("System {} registered post-init.", object.getClass().getName());
             initialiseSystem(object);
         }
     }

engine/src/main/java/org/terasology/engine/core/module/SandboxFileManager.java

@@ -70,7 +70,7 @@ public void readFile(String filename, Consumer<InputStream> consumer) {
                 // consumer to read the file, if it exists
                 consumer.accept(moduleInputStream);
             } catch (IOException e) {
-                logger.error("Could not read the file: " + filename, e);
+                logger.error("Could not read the file: {}", filename, e);
             }
 
             return null;
@@ -109,7 +109,7 @@ public void writeFile(String filename, Consumer<OutputStream> consumer) {
                 // consumer to write the file
                 consumer.accept(moduleInputStream);
             } catch (IOException e) {
-                logger.error("Could not write the file: " + filename, e);
+                logger.error("Could not write the file: {}", filename, e);
             }
 
             return null;

engine/src/main/java/org/terasology/engine/logic/behavior/actions/conditions/ConditionAction.java

@@ -55,14 +55,12 @@ public BehaviorState modify(Actor actor, BehaviorState result) {
             }
             return BehaviorState.SUCCESS;
         } catch (ClassNotFoundException e) {
-            logger.error("Class not found. " +
-                    "Does the Component specified exist?", e);
+            logger.error("Class not found. Does the Component specified exist?", e);
         } catch (NoSuchFieldException e) {
-            logger.error("Field not found. " +
-                    "Does the field specified in 'values' (publicly) exist in the Component specified in 'componentPresent'?", e);
+            logger.error("Field not found. "
+                    + "Does the field specified in 'values' (publicly) exist in the Component specified in 'componentPresent'?", e);
         } catch (IllegalAccessException e) {
-            logger.error("Illegal access. " +
-                    "Do we have access to the Component in question?", e);
+            logger.error("Illegal access. Do we have access to the Component in question?", e);
         }
         return BehaviorState.FAILURE;
     }

engine/src/main/java/org/terasology/engine/rendering/dag/AbstractNode.java

@@ -197,17 +197,17 @@ public boolean addOutputBufferPairConnection(int id, BufferPair bufferPair) {
 
             // set data for all connected connections
             if (!localBufferPairConnection.getConnectedConnections().isEmpty()) {
-                logger.debug("Propagating bufferPair data to all connected connections of " + localBufferPairConnection + ": ");
+                logger.debug("Propagating bufferPair data to all connected connections of {}: ", localBufferPairConnection);
                 localBufferPairConnection.getConnectedConnections().forEach((k, v) -> {
-                    logger.debug("setting data for: " + v.toString() + " ,");
+                    logger.debug("setting data for: {} ,", v);
                     v.setData(bufferPair);
                 });
                 logger.debug("data propagated.\n");
             }
 
             if (localBufferPairConnection.getData() != null) {
-                logger.warn("Adding output buffer pair to slot id " + id
-                        + " of " + this.nodeUri + "node overwrites data of existing connection: " + localBufferPairConnection.toString());
+                logger.warn("Adding output buffer pair to slot id {} of {} node overwrites data of existing connection: {}",
+                        id, this.nodeUri, localBufferPairConnection);
             }
             localBufferPairConnection.setData(bufferPair);
             success = true;
@@ -234,17 +234,17 @@ public boolean addOutputBufferPairConnection(int id, BufferPairConnection from)
 
             // set data for all connected connections
             if (!localBufferPairConnection.getConnectedConnections().isEmpty()) {
-                logger.info("Propagating data from " + from.toString() + " to all connected connections of " + localBufferPairConnection + ": ");
+                logger.info("Propagating data from {} to all connected connections of {}: ", from.toString(), localBufferPairConnection);
                 localBufferPairConnection.getConnectedConnections().forEach((k, v) -> {
-                    logger.info("setting data for: " + v.toString() + " ,");
+                    logger.info("setting data for: {} ,", v);
                     v.setData(from.getData());
                 });
                 logger.info("data propagated.\n");
             }
 
             if (localBufferPairConnection.getData() != null) {
-                logger.warn("Adding output buffer pair connection " + from.toString() + "\n to slot id " + id
-                        + " of " + this.nodeUri + "node overwrites data of existing connection: " + localBufferPairConnection.toString());
+                logger.warn("Adding output buffer pair connection to slot id {} of {} node overwrites data of existing connection: {}",
+                        id, this.nodeUri, localBufferPairConnection);
             }
             localBufferPairConnection.setData(from.getData());
 
@@ -305,16 +305,16 @@ protected boolean addOutputFboConnection(int id, FBO fboData) {
             FboConnection fboConnection = (FboConnection) outputConnections.get(connectionUri);
 
             if (fboConnection.getData() != null) {
-                logger.warn("Adding output fbo data to slot id " + id
-                        + " of " + this.nodeUri + "node overwrites data of existing connection: " + fboConnection.toString());
+                logger.warn("Adding output fbo data to slot id {} of {} node overwrites data of existing connection: {}",
+                        id, this.nodeUri, fboConnection);
             }
             fboConnection.setData(fboData);
 
             // set data for all connected connections
             if (!fboConnection.getConnectedConnections().isEmpty()) {
-                logger.info("Propagating fbo data to all connected connections of " + fboConnection + ": ");
+                logger.info("Propagating fbo data to all connected connections of {}: ", fboConnection);
                 fboConnection.getConnectedConnections().forEach((k, v) -> {
-                    logger.info("setting data for: " + v.toString() + " ,");
+                    logger.info("setting data for: {} ,", v);
                     v.setData(fboData);
                 });
                 logger.info("data propagated.\n");
@@ -445,9 +445,7 @@ public int getNumberOfOutputConnections() {
     public DependencyConnection getInputConnection(String name) {
         DependencyConnection connection = inputConnections.get(name);
         if (connection == null) {
-            String errorMessage = String.format("Getting input connection named %s returned null." +
-                    " No such input connection in %s", name, this.toString());
-            logger.error(errorMessage);
+            logger.error("Getting input connection named {} returned null. No such input connection in {}", name, this);
             // throw new NullPointerException(errorMessage);
         }
         return connection;
@@ -457,9 +455,7 @@ public DependencyConnection getInputConnection(String name) {
     public DependencyConnection getOutputConnection(String name) {
         DependencyConnection connection = outputConnections.get(name);
         if (connection == null) {
-            String errorMessage = String.format("Getting output connection named %s returned null." +
-                                                " No such output connection in %s", name, this.toString());
-            logger.error(errorMessage);
+            logger.error("Getting output connection named {} returned null. No such output connection in {}.", name, this);
             // throw new NullPointerException(errorMessage);
         }
         return connection;
@@ -515,7 +511,7 @@ protected FBO requiresFbo(FboConfig fboConfig, BaseFboManager fboManager) {
         if (!fboUsages.containsKey(fboName)) {
             fboUsages.put(fboName, fboManager);
         } else {
-            logger.warn("FBO " + fboName + " is already requested.");
+            logger.warn("FBO {} is already requested.", fboName);
             fbo = fboManager.get(fboName);
             this.addInputFboConnection(inputConnections.size() + 1, fbo);
             return fbo;

engine/src/main/java/org/terasology/engine/rendering/dag/RenderGraph.java

@@ -52,8 +52,8 @@ public void addNode(Node node) {
         }
         if (akaNodeMap.containsKey(nodeAka)) {
             Node aNode = akaNodeMap.get(nodeAka);
-            logger.info("Node " + nodeUri + " also known as" + nodeAka + " already matches existing node with uri "
-                    + aNode.getUri() + " - attempting replacing...");
+            logger.info("Node {} also known as {} already matches existing node with uri {} - attempting replacing...",
+                    nodeUri, nodeAka, aNode.getUri());
             replaceNode(aNode, node);
         } else {
             nodeMap.put(nodeUri, node);
@@ -149,7 +149,7 @@ private void connect(Node... nodeList) {
                 if (!graph.hasEdgeConnecting(fromNode, toNode)) {
                     graph.putEdge(fromNode, toNode);
                 } else {
-                    logger.warn("Trying to connect two already connected nodes, " + fromNode.getUri() + " and " + toNode.getUri());
+                    logger.warn("Trying to connect two already connected nodes, {} and {}", fromNode.getUri(), toNode.getUri());
                 }
             }
 
@@ -169,7 +169,7 @@ public void disconnect(Node fromNode, Node toNode) {
         Preconditions.checkNotNull(toNode, "toNode cannot be null!");
 
         if (!graph.hasEdgeConnecting(fromNode, toNode)) {
-            logger.warn("Trying to disconnect two nodes that aren't connected, " + fromNode.getUri() + " and " + toNode.getUri());
+            logger.warn("Trying to disconnect two nodes that aren't connected,  {} and {}", fromNode.getUri(), toNode.getUri());
         }
 
         graph.removeEdge(fromNode, toNode);
@@ -264,7 +264,7 @@ private void connectFbo(Node toNode, int inputFboId, DependencyConnection fromCo
         // TODO this will have to be caught by a try-catch or redone if we were going use gui to tamper with dag
         // Is not yet connected?
         if (!fromConnection.getConnectedConnections().isEmpty()) {
-            logger.warn("Warning, " + fromConnection + "connection is already read somewhere else.");
+            logger.warn("Warning, {} connection is already read somewhere else.", fromConnection);
         }
         // If adding new input goes smoothly
         // TODO These checks might be redundant
@@ -275,8 +275,8 @@ private void connectFbo(Node toNode, int inputFboId, DependencyConnection fromCo
 
         } else { // if adding new input failed, it already existed - check for connections
             //TODO update
-            logger.info(toNode.getUri() + ".connectFbo(" + inputFboId + ", " + fromConnection.getName() + "):" +
-                    " Connection already existed. Testing for its connections..");
+            logger.info("{}.connectFbo({}, {}): Connection already existed. Testing for its connections..",
+                    toNode.getUri(), inputFboId, fromConnection.getName());
             DependencyConnection localConnection = toNode.getInputFboConnection(inputFboId);
             // DependencyConnection localConnectionConnectedTo = localConnection.getConnectedConnections();
             // if our input is connected
@@ -302,7 +302,7 @@ public void connectFbo(Node fromNode, int outputId, Node toNode, int inputId) {
 //        if (!areConnected(fromNode, toNode)) {
 //            connect(fromNode, toNode);
 //        }
-        logger.debug("Connected " + fromNode.getOutputFboConnection(outputId) + " to " + toNode + ".");
+        logger.debug("Connected {} to {}.", fromNode.getOutputFboConnection(outputId),  toNode);
     }
 
     public void reconnectFbo(Node fromNode, int outputId, Node toNode, int inputId) {
@@ -328,7 +328,7 @@ public void reconnectFbo(Node fromNode, int outputId, Node toNode, int inputId)
     private void connectBufferPair(Node toNode, int inputConnectionId, DependencyConnection fromConnection) {
         // Is not yet connected?
         if (!fromConnection.getConnectedConnections().isEmpty()) {
-            logger.info("Warning, " + fromConnection + "connection is already read somewhere else.");
+            logger.info("Warning, {} connection is already read somewhere else.", fromConnection);
         }
         // If adding new input goes smoothly
         // TODO These checks might be redundant
@@ -339,8 +339,8 @@ private void connectBufferPair(Node toNode, int inputConnectionId, DependencyCon
 
         } else { // if adding new input failed, it already existed - check for connections
             //TODO update
-            logger.info(toNode.getUri() + ".connectFbo(" + inputConnectionId + ", " + fromConnection.getName() + "):" +
-                    " Connection already existed. Testing for its connections..");
+            logger.info("{}.connectFbo({}, {}): Connection already existed. Testing for its connections..",
+                    toNode.getUri(), inputConnectionId, fromConnection.getName());
             DependencyConnection localConnection = toNode.getInputBufferPairConnection(inputConnectionId);
             // DependencyConnection localConnectionConnectedTo = localConnection.getConnectedConnections();
             // if our input is connected
@@ -401,7 +401,7 @@ public void connectBufferPair(Node fromNode, int outputId, Node toNode, int inpu
 //        if (!areConnected(fromNode, toNode)) {
 //            connect(fromNode, toNode);
 //        }
-        logger.debug("Connected " + fromNode.getOutputBufferPairConnection(outputId) + " to " + toNode + ".");
+        logger.debug("Connected {} to {}.", fromNode.getOutputBufferPairConnection(outputId), toNode);
     }
 
     /**
@@ -533,13 +533,13 @@ public void reconnectInputToOutput(String fromNodeUri, int outputId, Node toNode
      */
     private void reconnectInputToOutput(Node toNode, int inputId, DependencyConnection fromConnection,
                                         ConnectionType connectionType, boolean disconnectPrevious) {
-        logger.debug("Attempting reconnection of " + toNode.getUri() + " to " + fromConnection.getParentNode() + "'s output.");
+        logger.debug("Attempting reconnection of {} to {}'s output.", toNode.getUri(), fromConnection.getParentNode());
         Node fromNode;
 
         fromNode = findNode(fromConnection.getParentNode());
         if (!fromConnection.getConnectedConnections().isEmpty()) {
-            logger.warn("WARNING: destination connection (" + fromConnection + ") is already connected to ("
-                    + fromConnection.getConnectedConnections());
+            logger.warn("WARNING: destination connection ({}) is already connected to ({})",
+                    fromConnection, fromConnection.getConnectedConnections());
             // TODO update the hashmap to string to be pretty
             // throw new RuntimeException("Could not reconnect, destination connection (" + fromConnection + ") is already connected to ("
             // + fromConnection.getConnectedConnections() + "). Remove connection first.");
@@ -591,7 +591,7 @@ private void reconnectInputToOutput(Node toNode, int inputId, DependencyConnecti
                 // either do this after everything is set up, or in renderGraph.addNode
                 // and when calling these trough api, call resetDesiredStateChanges();
             } else {
-                logger.info(toNode + "'s connection " + connectionToReconnect + " was not connected. Attempting new connection...");
+                logger.info("{}'s connection {} was not connected. Attempting new connection...", toNode, connectionToReconnect);
                 this.connectFbo(toNode, inputId, fromConnection);
             }
         } else { //                               TODO make it connectionToReconnect
@@ -607,7 +607,7 @@ private void reconnectInputToOutput(Node toNode, int inputId, DependencyConnecti
                     connectionName = "[unsupported connection type]";
             }
 
-            logger.info("No such input connection named " + connectionName + ". Attempting new connection...");
+            logger.info("No such input connection named {}. Attempting new connection...", connectionName);
 
             switch (connectionType) {
                 case FBO:
@@ -638,15 +638,15 @@ public void connectFbo(String fromNodeUri, int outputId, Node toNode, int inputI
 
     @Deprecated
     public void disconnectOutputFboConnection(Node node, int connectionId) {
-        logger.debug("Attempting disconnection of " + node + "'s output fbo number " + connectionId + "..");
+        logger.debug("Attempting disconnection of {}'s output fbo number {}..", node, connectionId);
 
         if (node != null) {
             DependencyConnection outputConnection = node.getOutputFboConnection(connectionId);
             if (outputConnection != null) {
                 outputConnection.disconnect();
                 logger.debug("..disconnecting complete.");
             } else {
-                logger.warn("Could not find output Fbo connection number " + connectionId + "within " + node + ".");
+                logger.warn("Could not find output Fbo connection number {} within {}.", connectionId, node);
             }
         } else {
             throw new RuntimeException("Could not find node named " + node + " within renderGraph.");
@@ -661,7 +661,7 @@ public void disconnectOutputFboConnection(String nodeUri, int connectionId) {
     }
 
     public void disconnectInputFbo(String nodeUri, int connectionId) {
-        logger.debug("Attempting disconnection of " + nodeUri + "'s input fbo number " + connectionId);
+        logger.debug("Attempting disconnection of {}'s input fbo number {}.", nodeUri, connectionId);
         Node node = findNode(new SimpleUri(nodeUri));
         if (node != null) {
             ((AbstractNode) node).disconnectInputFbo(connectionId);
@@ -730,7 +730,7 @@ public void reconnectAllConnectedInputsTo(DependencyConnection connectionToRepla
         } else if (newOutputConnection instanceof BufferPairConnection) {
             connectionType = ConnectionType.BUFFER_PAIR;
         } else {
-            logger.error("Unknown connection type: " + newOutputConnection + " .\n");
+            logger.error("Unknown connection type: {} .\n", newOutputConnection);
             throw new RuntimeException("Unknown connection type: " + newOutputConnection + " .\n");
         }