Added violation case for InefficientEmptyStringCheckRule

String.trim().isEmpty() now triggers a violation.

Fixed checkstyle
This commit is contained in:
Clément Fournier
2017-03-22 10:30:50 +01:00
committed by Juan Martín Sotuyo Dodero
parent 59de7a1460
commit 6bf8138aaf
3 changed files with 103 additions and 20 deletions

View File

@ -4,19 +4,21 @@
package net.sourceforge.pmd.lang.java.rule.strings;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
import net.sourceforge.pmd.lang.java.rule.AbstractInefficientZeroCheck;
import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence;
/**
* This rule finds code which inefficiently determines empty strings. This code
* <p/>
*
* <pre>
* if(str.trim().length()==0){....
* </pre>
*
* <p/> is quite inefficient as trim() causes a new String to be created.
* Smarter code to check for an empty string would be: <p/>
* <p>
* is quite inefficient as trim() causes a new String to be created. Smarter
* code to check for an empty string would be:
* </p>
*
* <pre>
* Character.isWhitespace(str.charAt(i));
@ -26,27 +28,39 @@ import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence;
*/
public class InefficientEmptyStringCheckRule extends AbstractInefficientZeroCheck {
/**
* Determine if we're dealing with String.length method
*
* @param occ
* The name occurrence
* @return true if it's String.length, else false
*/
@Override
public boolean isTargetMethod(JavaNameOccurrence occ) {
if (occ.getNameForWhichThisIsAQualifier() != null
&& occ.getNameForWhichThisIsAQualifier().getImage().indexOf("trim") != -1) {
Node pExpression = occ.getLocation().jjtGetParent().jjtGetParent();
if (pExpression.jjtGetNumChildren() >= 3
&& "length".equals(pExpression.jjtGetChild(2).getImage())) {
if (pExpression.jjtGetNumChildren() > 2 && "length".equals(pExpression.jjtGetChild(2).getImage())) {
return true;
}
}
return false;
}
@Override
public boolean appliesToClassName(String name) {
return "String".equals(name);
}
@Override
public Object visit(ASTPrimaryExpression node, Object data) {
if (node.jjtGetNumChildren() > 3) {
// Check last suffix
if (!"isEmpty".equals(node.jjtGetChild(node.jjtGetNumChildren() - 2).getImage())) {
return data;
}
Node prevCall = node.jjtGetChild(node.jjtGetNumChildren() - 4);
String target = prevCall.jjtGetNumChildren() > 0 ? prevCall.jjtGetChild(0).getImage() : prevCall.getImage();
if (target != null && target.indexOf("trim") != -1) {
addViolation(data, node);
}
}
return data;
}
}

View File

@ -0,0 +1,21 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.strings;
import net.sourceforge.pmd.testframework.SimpleAggregatorTst;
/**
* @author Clément Fournier (clement.fournier@insa-rennes.fr)
*
*/
public class InefficientEmptyStringCheckTest extends SimpleAggregatorTst {
private static final String RULESET = "java-strings";
@Override
public void setUp() {
addRule(RULESET, "InefficientEmptyStringCheck");
}
}

View File

@ -132,4 +132,52 @@ public class Foo {
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
String.trim().isEmpty() is called, should have failed
]]></description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
void bar() {
String foo = "foo";
boolean b = foo.trim().isEmpty();
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
String.trim().isEmpty() is called after a chain call, should have failed
]]></description>
<expected-problems>2</expected-problems>
<code><![CDATA[
public class Foo {
void bar() {
String foo = "foo";
boolean b = Arrays.toString(foo.toCharArray()).trim().isEmpty();
int i = 2;
b = String.valueOf(i).trim().isEmpty();
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
String.trim().isEmpty() is called after a chain call, should have failed twice
]]></description>
<expected-problems>2</expected-problems>
<code><![CDATA[
public class Foo {
void bar() {
String foo = "foo";
boolean b = Arrays.toString(foo.toCharArray()).trim().isEmpty();
b = String.valueOf(2).trim().isEmpty();
}
}
]]></code>
</test-code>
</test-data>