Merge branch 'pr-303' into pmd/5.5.x
This commit is contained in:
@ -4,49 +4,63 @@
|
||||
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));
|
||||
* </pre>
|
||||
*
|
||||
*
|
||||
* @author acaplan
|
||||
*/
|
||||
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 && "trim".equals(target) || target.endsWith(".trim")) {
|
||||
addViolation(data, node);
|
||||
}
|
||||
}
|
||||
return data;
|
||||
}
|
||||
|
||||
}
|
||||
|
@ -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>
|
||||
|
@ -21,6 +21,8 @@ The PMD team is pleased to announce PMD 5.5.5.
|
||||
* [#275](https://github.com/pmd/pmd/issues/275): \[java] FinalFieldCouldBeStatic: Constant in @interface incorrectly reported as "could be made static"
|
||||
* [#282](https://github.com/pmd/pmd/issues/282): \[java] UnnecessaryLocalBeforeReturn false positive when cloning Maps
|
||||
* [#291](https://github.com/pmd/pmd/issues/291): \[java] Improve quality of AccessorClassGeneration
|
||||
* java-strings:
|
||||
* [#290](https://github.com/pmd/pmd/issues/290): \[java] InefficientEmptyStringCheck misses String.trim().isEmpty()
|
||||
|
||||
### API Changes
|
||||
|
||||
@ -29,4 +31,4 @@ The PMD team is pleased to announce PMD 5.5.5.
|
||||
* [#280](https://github.com/pmd/pmd/pull/280): \[apex] Support for Aggregate Result in CRUD rules
|
||||
* [#289](https://github.com/pmd/pmd/pull/289): \[apex] Complex SOQL Crud check bug fixes
|
||||
* [#296](https://github.com/pmd/pmd/pull/296): \[apex] Adding String.IsNotBlank to the whitelist to prevent False positives
|
||||
|
||||
* [#303](https://github.com/pmd/pmd/pull/303): \[java] InefficientEmptyStringCheckRule now reports String.trim().isEmpty()
|
||||
|
Reference in New Issue
Block a user