From 4cdc84d6b113d84d6f7779acb82ecb03ce0255e6 Mon Sep 17 00:00:00 2001 From: Tom Copeland Date: Thu, 8 Dec 2005 22:43:59 +0000 Subject: [PATCH] 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 --- pmd/etc/changelog.txt | 1 + .../InefficientStringBufferAppendTest.java | 65 +++++++++++++++++++ pmd/rulesets/strings.xml | 23 +++++++ .../InefficientStringBufferAppend.java | 37 +++++++++++ .../strings/InefficientStringBuffering.java | 17 ++--- pmd/xdocs/credits.xml | 2 +- 6 files changed, 136 insertions(+), 9 deletions(-) create mode 100644 pmd/regress/test/net/sourceforge/pmd/rules/strings/InefficientStringBufferAppendTest.java create mode 100644 pmd/src/net/sourceforge/pmd/rules/strings/InefficientStringBufferAppend.java diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index 9b03950a54..e9ae0fea91 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -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(). diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/strings/InefficientStringBufferAppendTest.java b/pmd/regress/test/net/sourceforge/pmd/rules/strings/InefficientStringBufferAppendTest.java new file mode 100644 index 0000000000..ab9a71b24f --- /dev/null +++ b/pmd/regress/test/net/sourceforge/pmd/rules/strings/InefficientStringBufferAppendTest.java @@ -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 + + "}"; +} diff --git a/pmd/rulesets/strings.xml b/pmd/rulesets/strings.xml index 4a8edf5534..e32a64a547 100644 --- a/pmd/rulesets/strings.xml +++ b/pmd/rulesets/strings.xml @@ -141,6 +141,29 @@ Using equalsIgnoreCase() is faster than using toUpperCase/toLowerCase().equals() + + +Avoid concatenating characters as strings in StringBuffer.append + + 2 + + + + diff --git a/pmd/src/net/sourceforge/pmd/rules/strings/InefficientStringBufferAppend.java b/pmd/src/net/sourceforge/pmd/rules/strings/InefficientStringBufferAppend.java new file mode 100644 index 0000000000..02ae87022f --- /dev/null +++ b/pmd/src/net/sourceforge/pmd/rules/strings/InefficientStringBufferAppend.java @@ -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: + * 
+ * StringBuffer.append("c"); // appends a single character
+ *
+ * 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; + } +} diff --git a/pmd/src/net/sourceforge/pmd/rules/strings/InefficientStringBuffering.java b/pmd/src/net/sourceforge/pmd/rules/strings/InefficientStringBuffering.java index 1c962399c2..4e0c24c0bb 100644 --- a/pmd/src/net/sourceforge/pmd/rules/strings/InefficientStringBuffering.java +++ b/pmd/src/net/sourceforge/pmd/rules/strings/InefficientStringBuffering.java @@ -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
    +
  • 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
  • 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
  • Bhatia Saurabh - reported a bug in UseStringBufferLength
  • Hendrik Maryns - reported bug 1375290 for SuppressWarnings facility
    • @@ -52,7 +53,6 @@
  • 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
  • Sean Montgomery - bug report 1371980 for InefficientStringBuffering
  • 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
    • -
  • 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
  • Jean-Marc Vanel - suggested enhancements to the PMD scoreboard
  • Fabio Insaccanebbia - AvoidDecimalLiteralsInBigDecimalConstructor, ClassCastExceptionWithToArray
  • Johan Stuyts - nice patch for UncommentedEmptyConstructor and UncommentedEmptyMethod, patch to allow empty catch blocks with comments in them, patch to clean up build environment