From 2d57e6755a0da228d5e29213f61ac54acb2ee6cf Mon Sep 17 00:00:00 2001 From: John Armgardt Date: Fri, 12 Nov 2021 16:46:36 -0600 Subject: [PATCH 1/4] Fixed issue --- .../performance/UselessStringValueOfRule.java | 2 +- .../performance/xml/UselessStringValueOf.xml | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UselessStringValueOfRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UselessStringValueOfRule.java index 389d7b96b8..ab03d5dfca 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UselessStringValueOfRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UselessStringValueOfRule.java @@ -54,7 +54,7 @@ public class UselessStringValueOfRule extends AbstractJavaRule { && "+".equals(gp.getImage())) { boolean ok = false; if (gp.getChild(0) == parent) { - ok = !isPrimitive(gp.getChild(1)); + return super.visit(node, data); } else { for (int i = 0; !ok && gp.getChild(i) != parent; i++) { ok = !isPrimitive(gp.getChild(i)); diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/UselessStringValueOf.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/UselessStringValueOf.xml index 538b0cb9e0..5c01efe4b2 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/UselessStringValueOf.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/UselessStringValueOf.xml @@ -109,6 +109,27 @@ public class Test { private void print(String s) { System.out.println(s); } +} + ]]> + + + + #3492 False positive for UselessStringValueOf, when there is no initial String to append to + 0 + From 6fb8fb57e7add5053f84acb76d772dcf545bb9cf Mon Sep 17 00:00:00 2001 From: John Armgardt Date: Thu, 2 Dec 2021 15:52:30 -0600 Subject: [PATCH 2/4] Checked for method primitive return type --- .../java/rule/performance/UselessStringValueOfRule.java | 8 ++++++-- .../pmd/lang/java/symboltable/MethodNameDeclaration.java | 5 +++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UselessStringValueOfRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UselessStringValueOfRule.java index ab03d5dfca..a4ed812e59 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UselessStringValueOfRule.java +++ b/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; @@ -54,7 +55,7 @@ public class UselessStringValueOfRule extends AbstractJavaRule { && "+".equals(gp.getImage())) { boolean ok = false; if (gp.getChild(0) == parent) { - return super.visit(node, data); + ok = !isPrimitive(gp.getChild(1)); } else { for (int i = 0; !ok && gp.getChild(i) != parent; i++) { ok = !isPrimitive(gp.getChild(i)); @@ -71,7 +72,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); @@ -80,6 +81,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(); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodNameDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodNameDeclaration.java index 024e1f0728..ff13ba6b23 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodNameDeclaration.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodNameDeclaration.java @@ -32,6 +32,11 @@ public class MethodNameDeclaration extends AbstractNameDeclaration { return p.isVarargs(); } + public boolean isPrimitiveReturnType() { + return getMethodNameDeclaratorNode().getParent().getResultType().getChild(0) + .getChild(0) instanceof ASTPrimitiveType; + } + public ASTMethodDeclarator getMethodNameDeclaratorNode() { return (ASTMethodDeclarator) node; } From e801bb65252d0c289e8337e29da18be6181e22cb Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 3 Dec 2021 15:59:21 +0100 Subject: [PATCH 3/4] [java] MethodNameDeclaration.isPrimitiveReturnType() - check for void --- .../pmd/lang/java/symboltable/MethodNameDeclaration.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodNameDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodNameDeclaration.java index ff13ba6b23..340a0b41c6 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodNameDeclaration.java +++ b/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 { @@ -33,8 +34,8 @@ public class MethodNameDeclaration extends AbstractNameDeclaration { } public boolean isPrimitiveReturnType() { - return getMethodNameDeclaratorNode().getParent().getResultType().getChild(0) - .getChild(0) instanceof ASTPrimitiveType; + ASTResultType resultType = getMethodNameDeclaratorNode().getParent().getResultType(); + return !resultType.isVoid() && resultType.getChild(0).getChild(0) instanceof ASTPrimitiveType; } public ASTMethodDeclarator getMethodNameDeclaratorNode() { From 72a037d27f1ab64a2850bdae5329e213fc13c0a6 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 3 Dec 2021 16:04:26 +0100 Subject: [PATCH 4/4] [doc] Update release notes (#3492, #3631) --- docs/pages/release_notes.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b8f8783555..e0153f2e11 100644 --- a/docs/pages/release_notes.md +++ b/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 %}