Merge pull request #292 from jborgers/kotlin/AvoidStringBuffer

AvoidStringBuffer kotlin rule
jborgers · Mar 1, 2024 · 4a49bc2 · 4a49bc2
2 parents d858c47 + 2d2d0af
commit 4a49bc2
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 5 deletions.
diff --git a/docs/EvaluateRulesForKotlin.md b/docs/EvaluateRulesForKotlin.md
@@ -9,14 +9,14 @@ Evaluation of Java PMD rules for use as Kotlin rules
 | 3.  | PP-1     | AvoidDecimalAndChoiceFormatAsField       | Yes?   | Low        | Yes              | High        | Not found         | NumberFornat/DateFormat not included?                                                                                   |
 | 4.  |          | AvoidDuplicateAssignmentsInCases         | Yes    | Medium     | Yes              | Low/Medium  | Partly found      | Detekt:DuplicateCaseInWhenExpression has overlap but is not the same. Add example, doc, Questionable if occuring often. |
 | 5.  |          | AvoidImplicitlyRecompilingRegex          | Yes    | High       | Yes              | High        | Not found         | Kotlin has own String/regex, also occurs here? support both?                                                            |
-| 6.  | JB-1     | AvoidInMemoryStreamingDefaultConstructor | Yes    | Low        | Yes              | High        | Not found         | Kotlin types? -> No                                                                                                     |
+| 6.  | JB-1-PR  | AvoidInMemoryStreamingDefaultConstructor | Yes    | Low        | Yes              | High        | Not found         | Kotlin types? -> No                                                                                                     |
 | 7.  |          | AvoidMultipleConcatStatements            | Yes    | Medium     | Yes              | High        | Not found         | How concat in Kotlin? Seems like Java                                                                                   | 
 | 8.  | PP-3     | AvoidRecompilingPatterns                 | Yes    | Low/Medium | Yes              | High        | Not found         | Kotlin version?                                                                                                         |
 | 9.  | JB-3     | AvoidRecompilingXPathExpression          | Yes    | Low        | Yes              | Medium/High | Not found         | Good example ThreadLocal in Kotlin?                                                                                     |
 | 10. |          | AvoidRecreatingDateTimeFormatter         | Yes    | Medium     | Yes              | High        | Not found         | -                                                                                                                       |
 | 11. |          | AvoidReflectionInToStringAndHashCode     | Yes    | Low/Medium | Yes              | Low/Medium  | Not found         | -                                                                                                                       |
 | 12. | PP-2     | AvoidSimpleDateFormat                    | Yes    | Low        | Yes              | Medium      | Not found         |                                                                                                                         |
-| 13. | JB-2     | AvoidStringBuffer                        | Yes    | Low        | Yes              | Low/Medium  | Not found         |                                                                                                                         |
+| 13. | JB-2-PR   | AvoidStringBuffer                        | Yes    | Low        | Yes              | Low/Medium  | Not found         |                                                                                                                         |
 | 14. |          | AvoidUnconditionalBuiltLogStrings        | Yes    | High       | Yes              | Medium      | Not found         |                                                                                                                         | 
 | 15. |          | AvoidWideScopeXPathExpression            | Yes    | Low        | Yes              | Medium      | Not found         |                                                                                                                         | 
 | 16. |          | AvoidXPathAPIUsage                       | Yes    | Low        | Yes              | Medium      | Not found         | remove VTD reference?, seems old, better alternatives?                                                                  |

diff --git a/rulesets/kotlin/jpinpoint-kotlin-rules.xml b/rulesets/kotlin/jpinpoint-kotlin-rules.xml
@@ -57,6 +57,40 @@ class AvoidInMemoryStreamingDefaultConstructor {
         </example>
     </rule>
 
+    <rule name="AvoidStringBuffer" class="net.sourceforge.pmd.lang.rule.XPathRule"
+          since="7.0"
+          language="kotlin"
+          message="StringBuffer is used. It introduces locking overhead, use StringBuilder."
+          typeResolution="true"
+          externalInfoUrl="https://github.com/jborgers/PMD-jPinpoint-rules/tree/master/docs/JavaCodePerformance.md#isu01" >
+        <description>Problem: StringBuffer introduces locking overhead because it is thread safe. Its thread-safety is rarely needed.&#13;
+            Solution: Replace StringBuffer by StringBuilder.  (jpinpoint-rules)</description>
+        <priority>3</priority>
+        <properties>
+            <property name="tag" value="jpinpoint-rule" type="String" description="for-sonar"/>
+            <property name="version" value="3.1"/>
+            <property name="xpath">
+                <value><![CDATA[
+//VariableDeclaration/Type//T-Identifier[@Text='StringBuffer'],
+//Declaration//PrimaryExpression//T-Identifier[@Text='StringBuffer']
+]]>
+                </value>
+            </property>
+        </properties>
+        <example>
+            <![CDATA[
+class AvoidStringBuffer {
+    var fieldSb: StringBuffer? = null // bad
+    fun bad() {
+        val sb = StringBuffer() // bad
+    }
+    fun good() {
+        val sb = StringBuilder()
+    }
+]]>
+        </example>
+    </rule>
+
 <!-- END Included file 'common.xml' -->
 <!-- BEGIN Included file 'remoting.xml' -->
     <rule name="AvoidDeprecatedHystrix"

diff --git a/src/main/resources/category/kotlin/common.xml b/src/main/resources/category/kotlin/common.xml
@@ -48,6 +48,39 @@ class AvoidInMemoryStreamingDefaultConstructor {
         </example>
     </rule>
 
+    <rule name="AvoidStringBuffer" class="net.sourceforge.pmd.lang.rule.XPathRule"
+          since="7.0"
+          language="kotlin"
+          message="StringBuffer is used. It introduces locking overhead, use StringBuilder."
+          typeResolution="true"
+          externalInfoUrl="${doc_root}/JavaCodePerformance.md#isu01" >
+        <description>Problem: StringBuffer introduces locking overhead because it is thread safe. Its thread-safety is rarely needed.&#13;
+            Solution: Replace StringBuffer by StringBuilder.  (jpinpoint-rules)</description>
+        <priority>3</priority>
+        <properties>
+            <property name="tag" value="jpinpoint-rule" type="String" description="for-sonar"/>
+            <property name="version" value="3.1"/>
+            <property name="xpath">
+                <value><![CDATA[
+//VariableDeclaration/Type//T-Identifier[@Text='StringBuffer'],
+//Declaration//PrimaryExpression//T-Identifier[@Text='StringBuffer']
+]]>
+                </value>
+            </property>
+        </properties>
+        <example>
+            <![CDATA[
+class AvoidStringBuffer {
+    var fieldSb: StringBuffer? = null // bad
+    fun bad() {
+        val sb = StringBuffer() // bad
+    }
+    fun good() {
+        val sb = StringBuilder()
+    }
+]]>
+        </example>
+    </rule>
     <rule name="AvoidDecimalAndChoiceFormatAsField"
           since="7.0"
           language="kotlin"

diff --git a/src/test/java/com/jpinpoint/perf/lang/kotlin/ruleset/common/AvoidStringBufferTest.java b/src/test/java/com/jpinpoint/perf/lang/kotlin/ruleset/common/AvoidStringBufferTest.java
@@ -0,0 +1,6 @@
+package com.jpinpoint.perf.lang.kotlin.ruleset.common;
+
+import net.sourceforge.pmd.testframework.PmdRuleTst;
+
+public class AvoidStringBufferTest extends PmdRuleTst {
+}
diff --git a/src/test/resources/com/jpinpoint/perf/lang/java/ruleset/common/xml/AvoidStringBuffer.xml b/src/test/resources/com/jpinpoint/perf/lang/java/ruleset/common/xml/AvoidStringBuffer.xml
@@ -10,10 +10,14 @@
         <code><![CDATA[
 public class Foo {
 
-	private StringBuffer buffer = new StringBuffer(); // Avoid!!!
+	private StringBuffer buffer = new StringBuffer(); // bad
 
-    public void testUnConditionalStringOperations() {
-        StringBuffer sb = new StringBuffer(); // Avoid!!!
+    public void bad() {
+        StringBuffer sb = new StringBuffer(); // bad
+        // var sb2 = new StringBuffer(); // bad - TODO
+    }
+    public void good() {
+        StringBuilder sb = new StringBuilder(); // good
     }
 }
      ]]></code>

diff --git a/src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidStringBuffer.xml b/src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidStringBuffer.xml
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<test-data
+        xmlns="http://pmd.sourceforge.net/rule-tests"
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
+    <test-code>
+        <description>AvoidStringBuffer</description>
+        <expected-problems>2</expected-problems>
+        <expected-linenumbers>2,4</expected-linenumbers>
+        <code><![CDATA[
+class AvoidStringBuffer {
+    var fieldSb: StringBuffer? = null // bad
+    fun bad() {
+        val sb = StringBuffer() // bad
+    }
+    fun good() {
+        val sb = StringBuilder()
+    }
+}
+]]></code>
+    </test-code>
+
+</test-data>