Fix #1340 UseStringBufferForStringAppends False Positive with ternary

operator (used both in condition and options)
This commit is contained in:
albfernandez
2016-02-27 13:03:41 +01:00
committed by Andreas Dangel
parent 1c08f94691
commit 3c0516b86f
2 changed files with 30 additions and 2 deletions

View File

@ -6,6 +6,8 @@ package net.sourceforge.pmd.lang.java.rule.optimizations;
import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; import net.sourceforge.pmd.lang.java.ast.ASTArgumentList;
import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator; import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator;
import net.sourceforge.pmd.lang.java.ast.ASTConditionalExpression;
import net.sourceforge.pmd.lang.java.ast.ASTEqualityExpression;
import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
@ -37,6 +39,16 @@ public class UseStringBufferForStringAppendsRule extends AbstractJavaRule {
// used in method call // used in method call
continue; continue;
} }
ASTEqualityExpression equality = name.getFirstParentOfType(ASTEqualityExpression.class);
if (equality != null && equality.getFirstParentOfType(ASTStatementExpression.class) == statement) {
// used in condition
continue;
}
ASTConditionalExpression conditional = name.getFirstParentOfType(ASTConditionalExpression.class);
if (conditional != null && name.jjtGetParent().jjtGetParent().jjtGetParent() == conditional && conditional.getFirstParentOfType(ASTStatementExpression.class) == statement) {
// is used in ternary as only option (not appendend to other string)
continue;
}
if (statement.jjtGetNumChildren() > 0 && statement.jjtGetChild(0) instanceof ASTPrimaryExpression) { if (statement.jjtGetNumChildren() > 0 && statement.jjtGetChild(0) instanceof ASTPrimaryExpression) {
ASTName astName = statement.jjtGetChild(0).getFirstDescendantOfType(ASTName.class); ASTName astName = statement.jjtGetChild(0).getFirstDescendantOfType(ASTName.class);
if(astName != null){ if(astName != null){
@ -47,6 +59,9 @@ public class UseStringBufferForStringAppendsRule extends AbstractJavaRule {
} }
} else if(astName.getImage().equals(name.getImage())){ } else if(astName.getImage().equals(name.getImage())){
ASTAssignmentOperator assignmentOperator = statement.getFirstDescendantOfType(ASTAssignmentOperator.class); ASTAssignmentOperator assignmentOperator = statement.getFirstDescendantOfType(ASTAssignmentOperator.class);
if (astName.jjtGetParent().jjtGetParent().jjtGetParent().getClass() == ASTEqualityExpression.class) {
continue;
}
if (assignmentOperator != null && !assignmentOperator.isCompound()) { if (assignmentOperator != null && !assignmentOperator.isCompound()) {
addViolation(data, astName); addViolation(data, astName);
} }

View File

@ -135,12 +135,25 @@ public class Foo {
]]></code> ]]></code>
</test-code> </test-code>
<test-code> <test-code>
<description>#1340 UseStringBufferForStringAppends False Positive with ternary operator</description> <description>#1340 UseStringBufferForStringAppends False Positive with ternary operator (used in condition) </description>
<expected-problems>0</expected-problems> <expected-problems>0</expected-problems>
<code><![CDATA[ <code><![CDATA[
public class UseStringBuffer { public class UseStringBuffer {
public void foo() { public void foo() {
String country = (country == null || "".equals(country)) String value = "";
country = (value != null) ? "1" : "0";
}
}
]]></code>
</test-code>
<test-code>
<description>#1340 UseStringBufferForStringAppends False Positive with ternary operator (used both in condition and options)</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class UseStringBuffer {
public void foo() {
String country = "";
country = (country == null || "".equals(country))
? ((String) getCountry()) ? ((String) getCountry())
: country; : country;
} }