Allan Caplan's implementation of InefficientStringBufferAppend, thanks Allan!

git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@4046 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Tom Copeland
2005-12-08 22:43:59 +00:00
parent de503c7d68
commit 4cdc84d6b1
6 changed files with 136 additions and 9 deletions

View File

@ -2,6 +2,7 @@
New rules:
Migration ruleset: IntegerInstantiation
JUnit ruleset: UseAssertNullInsteadOfAssertTrue
Strings ruleset: InefficientStringBufferAppend
Fixed bug 1277373 - InefficientStringBuffering now catches more cases.
Fixed bug 1371757 - Misleading example in AvoidSynchronizedAtMethodLevel
Fixed bug 1371980 - InefficientStringBuffering no longer flags StringBuffer methods other than append().

View File

@ -0,0 +1,65 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package test.net.sourceforge.pmd.rules.strings;
import net.sourceforge.pmd.PMD;
import net.sourceforge.pmd.Rule;
import test.net.sourceforge.pmd.testframework.SimpleAggregatorTst;
import test.net.sourceforge.pmd.testframework.TestDescriptor;
public class InefficientStringBufferAppendTest extends SimpleAggregatorTst {
private Rule rule;
public void setUp() throws Exception {
rule = findRule("strings", "InefficientStringBufferAppend");
}
public void testAll() {
runTests(new TestDescriptor[] {
new TestDescriptor(TEST1, "appending single character string, should fail", 1, rule),
new TestDescriptor(TEST2, "appending single char, should be ok", 0, rule),
new TestDescriptor(TEST3, "this is probably wrong, but shouldn't fail", 0, rule),
new TestDescriptor(TEST4, "concatenates a three character int", 0, rule),
new TestDescriptor(TEST5, "concatenates a string explicitly set to 1 character, not explicitly checking right now", 0, rule),
});
}
private static final String TEST1 =
"public class Foo {" + PMD.EOL +
" public void bar(StringBuffer sb) {" + PMD.EOL +
" sb.append(\"a\");" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST2 =
"public class Foo {" + PMD.EOL +
" public void bar(StringBuffer sb) {" + PMD.EOL +
" sb.append('a');" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST3 =
"public class Foo {" + PMD.EOL +
" public void bar(StringBuffer sb) {" + PMD.EOL +
" sb.append(\"a\" + \"foo\");" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST4 =
"public class Foo {" + PMD.EOL +
" public void bar(StringBuffer sb) {" + PMD.EOL +
" sb.append(123);" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST5 =
"public class Foo {" + PMD.EOL +
" public void bar(StringBuffer sb) {" + PMD.EOL +
" String str = \"a\";" + PMD.EOL +
" sb.append(str);" + PMD.EOL +
" }" + PMD.EOL +
"}";
}

View File

@ -141,6 +141,29 @@ Using equalsIgnoreCase() is faster than using toUpperCase/toLowerCase().equals()
</rule>
<rule name="InefficientStringBufferAppend"
message="Avoid appending characters as strings in StringBuffer.append"
class="net.sourceforge.pmd.rules.strings.InefficientStringBufferAppend">
<description>
Avoid concatenating characters as strings in StringBuffer.append
</description>
<priority>2</priority>
<example>
<![CDATA[
public class Foo {
void bar() {
StringBuffer sb=new StringBuffer();
// Avoid this
sb.append("a");
// use instead something like this
StringBuffer sb=new StringBuffer();
sb.append('a');
}
}
]]>
</example>
</rule>
</ruleset>

View File

@ -0,0 +1,37 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.rules.strings;
import net.sourceforge.pmd.AbstractRule;
import net.sourceforge.pmd.ast.ASTBlockStatement;
import net.sourceforge.pmd.ast.ASTLiteral;
/*
* This rule finds the following:
* <pre>
* StringBuffer.append("c"); // appends a single character
* </pre>
* It is preferable to use
* StringBuffer.append('c'); // appends a single character
* Implementation of PMD RFE 1373863
*/
public class InefficientStringBufferAppend extends AbstractRule {
public Object visit(ASTLiteral node, Object data) {
ASTBlockStatement bs = (ASTBlockStatement) node.getFirstParentOfType(ASTBlockStatement.class);
if (bs == null) {
return data;
}
String str = node.getImage();
if (str != null && str.length() == 3 && str.charAt(0) == '\"'
&& str.charAt(2) == '"') {
if (!InefficientStringBuffering.isInStringBufferAppend(node, 10)) {
return data;
}
addViolation(data, node);
}
return data;
}
}

View File

@ -3,6 +3,9 @@
*/
package net.sourceforge.pmd.rules.strings;
import java.util.Iterator;
import java.util.List;
import net.sourceforge.pmd.AbstractRule;
import net.sourceforge.pmd.ast.ASTAdditiveExpression;
import net.sourceforge.pmd.ast.ASTAllocationExpression;
@ -11,11 +14,9 @@ import net.sourceforge.pmd.ast.ASTClassOrInterfaceType;
import net.sourceforge.pmd.ast.ASTLiteral;
import net.sourceforge.pmd.ast.ASTName;
import net.sourceforge.pmd.ast.Node;
import net.sourceforge.pmd.ast.SimpleNode;
import net.sourceforge.pmd.symboltable.VariableNameDeclaration;
import java.util.Iterator;
import java.util.List;
/*
* How this rule works:
* find additive expressions: +
@ -56,14 +57,14 @@ public class InefficientStringBuffering extends AbstractRule {
if (isAllocatedStringBuffer(node)) {
addViolation(data, node);
}
} else if (isInStringBufferAppend(node)) {
} else if (isInStringBufferAppend(node, 8)) {
addViolation(data, node);
}
return data;
}
private boolean isInStringBufferAppend(ASTAdditiveExpression node) {
if (!eighthParentIsBlockStatement(node)) {
protected static boolean isInStringBufferAppend(SimpleNode node, int length) {
if (!xParentIsBlockStatement(node, length)) {
return false;
}
ASTBlockStatement s = (ASTBlockStatement) node.getFirstParentOfType(ASTBlockStatement.class);
@ -84,9 +85,9 @@ public class InefficientStringBuffering extends AbstractRule {
return vnd.getTypeImage().equals("StringBuffer");
}
private boolean eighthParentIsBlockStatement(ASTAdditiveExpression node) {
protected static boolean xParentIsBlockStatement(SimpleNode node, int length) {
Node curr = node;
for (int i=0; i<8; i++) {
for (int i=0; i<length; i++) {
if (node.jjtGetParent() == null) {
return false;
}

View File

@ -45,6 +45,7 @@
</subsection>
<subsection name="Contributors">
<ul>
<li>Allan Caplan - wrote InefficientStringBufferAppend, wrote IntegerInstantiation, wrote UseStringBufferLength, wrote AvoidEnumAsIdentifier, bug report 1313216 for designer flaw, patch 1306999 to enhance CloseResource to check for more types, suggested including suppressed rule violations in the report</li>
<li>Wouter Zelle - Renderer improvement suggestions, wrote NonThreadSafeSingleton rule, wrote DefaultPackage rule, worked thru ASTMethodDeclaration.isSyntacticallyX design, reported docs bug 1292689 for UnnecessaryLocalBeforeReturn, reported leftover ExceptionTypeChecking source file, rewrote UselessOverridingMethod in Java, UselessOverridingMethod rule, ProperLogger rule, nifty code to allow variables in XPath rules, externalInfoUrl data for all rules in basic and unusedcode rulesets, some very nifty XSLT, improvements to UseCorrectExceptionLogging, designed and implemented the "externalInfoUrl" feature in the rule definitions, fixed a devious bug in RuleSetFactory, AvoidPrintStackTrace, initial implementation of SimplifyConditional</li>
<li>Bhatia Saurabh - reported a bug in UseStringBufferLength</li>
<li>Hendrik Maryns - reported bug 1375290 for SuppressWarnings facility</li>
@ -52,7 +53,6 @@
<li>Elliotte Rusty Harold - noted missed case for UnusedFormalParameter, documentation suggestions, reported mistake in UnnecessaryLocalBeforeReturn message, bug report 1371757 for misleading AvoidSynchronizedAtMethodLevel example, bug report 1293277 for duplicated rule messages, bug report for ConstructorCallsOverridableMethod, suggestion for improving command line interface, mispeling report, suggestion for improving Designer startup script, "how to make a ruleset" documentation suggestions, noticed outdated Xerces jars, script renaming suggestions, UseLocaleWithCaseConversions rule suggestion</li>
<li>Sean Montgomery - bug report 1371980 for InefficientStringBuffering</li>
<li>Didier Duquennoy - bug report for InefficientStringBuffering, bug report for ImmutableField, suggestions for improving Benchmark utility, bug report for InefficientStringBuffering, bug report for AvoidConcateningNonLiteralsInStringBuffer, reported a missed hit for EqualsNull, bug report for MissingStaticMethodInNonInstantiatableClass, pmd-netbeans feedback</li>
<li>Allan Caplan - wrote IntegerInstantiation, wrote UseStringBufferLength, wrote AvoidEnumAsIdentifier, bug report 1313216 for designer flaw, patch 1306999 to enhance CloseResource to check for more types, suggested including suppressed rule violations in the report</li>
<li>Jean-Marc Vanel - suggested enhancements to the PMD scoreboard</li>
<li>Fabio Insaccanebbia - AvoidDecimalLiteralsInBigDecimalConstructor, ClassCastExceptionWithToArray</li>
<li>Johan Stuyts - nice patch for UncommentedEmptyConstructor and UncommentedEmptyMethod, patch to allow empty catch blocks with comments in them, patch to clean up build environment</li>