Merge pull request #3631 from johnra2:3492-usvo-fp-no-initial-string

[java] Fixed False positive for UselessStringValueOf when there is no initial String to append to #3631

* pr-3631:
  [doc] Update release notes (#3492, #3631)
  [java] MethodNameDeclaration.isPrimitiveReturnType() - check for void
  Checked for method primitive return type
  Fixed issue

docs/pages/release_notes.md

@@ -16,9 +16,14 @@ This is a {{ site.pmd.release_type }} release.
 
 ### Fixed Issues
 
+*   java-performance
+    *   [#3492](https://github.com/pmd/pmd/issues/3492): \[java] UselessStringValueOf: False positive when there is no initial String to append to
+
 ### API Changes
 
 ### External Contributions
 
+*   [#3631](https://github.com/pmd/pmd/pull/3631): \[java] Fixed False positive for UselessStringValueOf when there is no initial String to append to - [John Armgardt](https://github.com/johnra2)
+
 {% endtocmaker %}
 

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UselessStringValueOfRule.java

@@ -14,6 +14,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
 import net.sourceforge.pmd.lang.java.ast.ASTReferenceType;
 import net.sourceforge.pmd.lang.java.ast.ASTType;
 import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+import net.sourceforge.pmd.lang.java.symboltable.MethodNameDeclaration;
 import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
 import net.sourceforge.pmd.lang.symboltable.NameDeclaration;
 
@@ -75,7 +76,7 @@ public class UselessStringValueOfRule extends AbstractJavaRule {
 
     private static boolean isPrimitive(Node parent) {
         boolean result = false;
-        if (parent instanceof ASTPrimaryExpression && parent.getNumChildren() == 1) {
+        if (parent instanceof ASTPrimaryExpression && parent.getNumChildren() > 0) {
             Node child = parent.getChild(0);
             if (child instanceof ASTPrimaryPrefix && child.getNumChildren() == 1) {
                 Node gc = child.getChild(0);
@@ -84,6 +85,9 @@ public class UselessStringValueOfRule extends AbstractJavaRule {
                     NameDeclaration nd = name.getNameDeclaration();
                     if (nd instanceof VariableNameDeclaration && ((VariableNameDeclaration) nd).isPrimitiveType()) {
                         result = true;
+                    } else if (nd instanceof MethodNameDeclaration
+                            && ((MethodNameDeclaration) nd).isPrimitiveReturnType()) {
+                        result = true;
                     }
                 } else if (gc instanceof ASTLiteral) {
                     result = !((ASTLiteral) gc).isStringLiteral();

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodNameDeclaration.java

@@ -9,6 +9,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
 import net.sourceforge.pmd.lang.java.ast.ASTFormalParameters;
 import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclarator;
 import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType;
+import net.sourceforge.pmd.lang.java.ast.ASTResultType;
 import net.sourceforge.pmd.lang.symboltable.AbstractNameDeclaration;
 
 public class MethodNameDeclaration extends AbstractNameDeclaration {
@@ -32,6 +33,11 @@ public class MethodNameDeclaration extends AbstractNameDeclaration {
         return p.isVarargs();
     }
 
+    public boolean isPrimitiveReturnType() {
+        ASTResultType resultType = getMethodNameDeclaratorNode().getParent().getResultType();
+        return !resultType.isVoid() && resultType.getChild(0).getChild(0) instanceof ASTPrimitiveType;
+    }
+
     public ASTMethodDeclarator getMethodNameDeclaratorNode() {
         return (ASTMethodDeclarator) node;
     }

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/UselessStringValueOf.xml

@@ -112,6 +112,27 @@ public class Test {
 }
         ]]></code>
     </test-code>
+
+    <test-code>
+        <description>#3492 False positive for UselessStringValueOf, when there is no initial String to append to</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Test {
+    private short getPrefix(long id) {
+        return 0;
+    }
+    private short getTimestamp(long id) {
+        return 0;
+    }
+    private short getCounter(long id) {
+        return 0;
+    }
+    public String toShortString(long id) {
+        return String.valueOf(getPrefix(id)) + getTimestamp(id) + getCounter(id); // (12)
+    }
+}
+        ]]></code>
+    </test-code>
     
     <test-code>
         <description>#3491 False positive for UselessStringValueOf when valueOf(char [], int, int) is used</description>