From ae12b77ab7d503baeb4b16d51476009216b8787e Mon Sep 17 00:00:00 2001
From: Peter Paul Bakker <peterpaulbakker@stokpop.nl>
Date: Tue, 12 Mar 2024 13:46:27 +0100
Subject: [PATCH 1/3] added AvoidRecompilingPatterns including test (Convert
 applicable rules to Kotlin #185)

---
 src/main/resources/category/kotlin/common.xml |  57 +++++++++-
 .../common/AvoidRecompilingPatternsTest.java  |   6 ++
 .../common/xml/AvoidRecompilingPatterns.xml   | 100 ++++++++++++++++++
 3 files changed, 162 insertions(+), 1 deletion(-)
 create mode 100644 src/test/java/com/jpinpoint/perf/lang/kotlin/ruleset/common/AvoidRecompilingPatternsTest.java
 create mode 100644 src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidRecompilingPatterns.xml

diff --git a/src/main/resources/category/kotlin/common.xml b/src/main/resources/category/kotlin/common.xml
index 9b2eb4f9..7a2cbc50 100644
--- a/src/main/resources/category/kotlin/common.xml
+++ b/src/main/resources/category/kotlin/common.xml
@@ -105,7 +105,7 @@ class AvoidStringBuffer {
 (: filter matches in functions :)
 //ClassDeclaration//PrimaryExpression//SimpleIdentifier//T-Identifier[
  (@Text="DecimalFormat" or @Text="ChoiceFormat")
- and not(ancestor::FunctionDeclaration)
+ and not(ancestor::FunctionBody)
 ]
 ]]>
                 </value>
@@ -133,4 +133,59 @@ class Foo {
         </example>
     </rule>
 
+    <rule name="AvoidRecompilingPatterns"
+          class="net.sourceforge.pmd.lang.rule.XPathRule"
+          dfa="false"
+          language="kotlin"
+          message="Pattern.compile is used in a method. Compiling a regex pattern can be expensive, make it a constant or instance field."
+          typeResolution="true"
+          externalInfoUrl="${doc_root}/JavaCodePerformance.md#ireu02">
+        <description>A regular expression is compiled on every invocation. Problem: this can be expensive, depending on the length of the regular expression.&#13;
+            Solution: Usually a pattern is a literal, not dynamic and can be compiled only once. Assign it to a private constant or instance field.
+            java.util.Pattern objects are thread-safe so they can be shared among threads. (jpinpoint-rules)</description>
+        <priority>2</priority>
+        <properties>
+            <property name="tag" value="jpinpoint-rule" type="String" description="for-sonar"/>
+            <property name="version" value="3.1"/>
+            <property name="xpath">
+                <value><![CDATA[
+(: check if java.util.regex imports exists as filter for same named classes in other packages :)
+//ImportHeader[.//T-Identifier[@Text='java'] and .//T-Identifier[@Text='util'] and .//T-Identifier[@Text='regex']][1]/ancestor::KotlinFile
+
+//FunctionBody//T-Identifier[@Text='Pattern' and ../../../PostfixUnarySuffix//T-Identifier[@Text='compile']
+    (: check for variables $action or ${action} in String of compile() call (first in possible call chain): allow dynamic input :)
+    and not(((../../..//ValueArgument)[1]//PostfixUnaryExpression//T-Identifier|(../../..//ValueArgument)[1]//T-LineStrRef)[
+       @Text=ancestor::FunctionDeclaration//FunctionValueParameter/Parameter/SimpleIdentifier/T-Identifier/@Text
+       or @Text=concat('$', ancestor::FunctionDeclaration//FunctionValueParameter/Parameter/SimpleIdentifier/T-Identifier/@Text)])
+]
+]]>
+                </value>
+            </property>
+        </properties>
+        <example>
+            <![CDATA[
+import java.util.regex.Pattern
+
+internal object Bad {
+    const val STR_PAT1: String = "[A-Z][a-z]+"
+
+    fun bad() {
+        val p1: Pattern = Pattern.compile(STR_PAT1) // bad
+        val p2: Pattern = Pattern.compile("(?=\\p{Lu})") // bad
+        val b: Boolean = p1.matcher("Start ").matches()
+    }
+}
+
+internal object Good {
+    val PAT1: Pattern = Pattern.compile("[A-Z][a-z]+")
+    val PAT2: Pattern = Pattern.compile("(?=\\p{Lu})")
+
+    fun good() {
+        val b: Boolean = PAT1.matcher("Start ").matches()
+    }
+}
+            ]]>
+        </example>
+    </rule>
+
 </ruleset>
diff --git a/src/test/java/com/jpinpoint/perf/lang/kotlin/ruleset/common/AvoidRecompilingPatternsTest.java b/src/test/java/com/jpinpoint/perf/lang/kotlin/ruleset/common/AvoidRecompilingPatternsTest.java
new file mode 100644
index 00000000..d9718fb3
--- /dev/null
+++ b/src/test/java/com/jpinpoint/perf/lang/kotlin/ruleset/common/AvoidRecompilingPatternsTest.java
@@ -0,0 +1,6 @@
+package com.jpinpoint.perf.lang.kotlin.ruleset.common;
+
+import net.sourceforge.pmd.testframework.PmdRuleTst;
+
+public class AvoidRecompilingPatternsTest extends PmdRuleTst {
+}
diff --git a/src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidRecompilingPatterns.xml b/src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidRecompilingPatterns.xml
new file mode 100644
index 00000000..5acb4dd0
--- /dev/null
+++ b/src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidRecompilingPatterns.xml
@@ -0,0 +1,100 @@
+<?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>violation: Avoid recompiling patterns</description>
+        <expected-problems>3</expected-problems>
+        <expected-linenumbers>5,12,13</expected-linenumbers>
+        <code><![CDATA[
+import java.util.regex.Pattern
+
+class Foo {
+    fun createNameFromAction(action: String?) {
+        val p = Pattern.compile("(?=\\p{Lu})") // bad
+    }
+
+    companion object {
+        const val PATTERN: String = "^\\s*%s=(.*)"
+
+        fun methodBad(action: String?) {
+            val p = Pattern.compile("(?=\\p{Lu})") // bad: violation, should not be inside method, make static final
+            val s = Pattern.compile(PATTERN) // bad: violation
+        }
+    }
+}
+    ]]></code>
+    </test-code>
+    <test-code>
+        <description>no violation: Avoid recompiling patterns</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.util.ArrayList;
+import java.util.List;
+import java.util.regex.Pattern;
+
+object Bar {
+    const val PATTERN: String = "^\\s*%s=(.*)"
+
+    fun createEventNameFromAction(action: String): List<*> {
+        val q1 = Pattern.compile("($action?=\\p{Lu})") // ok, dynamic -- TODO make function for this?
+        val q2 = Pattern.compile("(" + action + "?=\\p{Lu})") // ok, dynamic
+        val q3 = Pattern.compile("(${action}?=\\p{Lu})") // ok, dynamic
+        val r = Pattern.compile(String.format(PATTERN, action)) // ok, dynamic
+        val t = Pattern.compile(action) // dynamic, so ok (fix for JPCC-63)
+
+        val actionSplit = q.split(action) // ok
+        val list: MutableList<Array<String>> = ArrayList()
+        list.add(actionSplit)
+        return list
+    }
+}
+     ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>no violation: Avoid recompiling patterns, other compile</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+class GoodHere {
+    fun otherCompile() {
+        MyCompiler.compile("stuff")
+    }
+}
+
+internal object MyCompiler {
+    fun compile(s: String?) {
+    }
+}
+     ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>violation: Avoid recompiling patterns, with chained operations, issue #180</description>
+        <expected-problems>2</expected-problems>
+        <expected-linenumbers>5,10</expected-linenumbers>
+        <code><![CDATA[
+import java.util.regex.Pattern;
+
+class Foo {
+    private fun isAmountAllowed(amount: String): Boolean {
+        return LIMIT > Pattern.compile(SEP).matcher(amount).replaceAll("").length() // bad
+    }
+
+    private val isAmountAllowed2: Boolean
+        get() {
+            val bla: String = Pattern.compile("[.|,]").matcher("5.000,00").replaceAll("") // bad
+            val b = (11 > bla.length)
+            return b
+        }
+
+    companion object {
+        private const val LIMIT = 11
+        private const val SEP = "[.|,]"
+    }
+}
+     ]]></code>
+    </test-code>
+
+</test-data>

From c221ab7dc9a47fd52347769efb165631c3476358 Mon Sep 17 00:00:00 2001
From: Peter Paul Bakker <peterpaulbakker@stokpop.nl>
Date: Tue, 12 Mar 2024 13:46:27 +0100
Subject: [PATCH 2/3] added AvoidRecompilingPatterns including test (Convert
 applicable rules to Kotlin #185)

---
 src/main/resources/category/kotlin/common.xml |  57 +++++++++-
 .../common/AvoidRecompilingPatternsTest.java  |   6 ++
 .../common/xml/AvoidRecompilingPatterns.xml   | 100 ++++++++++++++++++
 3 files changed, 162 insertions(+), 1 deletion(-)
 create mode 100644 src/test/java/com/jpinpoint/perf/lang/kotlin/ruleset/common/AvoidRecompilingPatternsTest.java
 create mode 100644 src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidRecompilingPatterns.xml

diff --git a/src/main/resources/category/kotlin/common.xml b/src/main/resources/category/kotlin/common.xml
index 18e3b7d5..f562b706 100644
--- a/src/main/resources/category/kotlin/common.xml
+++ b/src/main/resources/category/kotlin/common.xml
@@ -102,7 +102,7 @@ class AvoidStringBuffer {
 (: filter matches in functions :)
 //ClassDeclaration//PrimaryExpression//SimpleIdentifier//T-Identifier[
  (@Text="DecimalFormat" or @Text="ChoiceFormat")
- and not(ancestor::FunctionDeclaration)
+ and not(ancestor::FunctionBody)
 ]
 ]]>
                 </value>
@@ -130,6 +130,61 @@ class Foo {
         </example>
     </rule>
 
+    <rule name="AvoidRecompilingPatterns"
+          class="net.sourceforge.pmd.lang.rule.XPathRule"
+          dfa="false"
+          language="kotlin"
+          message="Pattern.compile is used in a method. Compiling a regex pattern can be expensive, make it a constant or instance field."
+          typeResolution="true"
+          externalInfoUrl="${doc_root}/JavaCodePerformance.md#ireu02">
+        <description>A regular expression is compiled on every invocation. Problem: this can be expensive, depending on the length of the regular expression.&#13;
+            Solution: Usually a pattern is a literal, not dynamic and can be compiled only once. Assign it to a private constant or instance field.
+            java.util.Pattern objects are thread-safe so they can be shared among threads. (jpinpoint-rules)</description>
+        <priority>2</priority>
+        <properties>
+            <property name="tag" value="jpinpoint-rule" type="String" description="for-sonar"/>
+            <property name="version" value="3.1"/>
+            <property name="xpath">
+                <value><![CDATA[
+(: check if java.util.regex imports exists as filter for same named classes in other packages :)
+//ImportHeader[.//T-Identifier[@Text='java'] and .//T-Identifier[@Text='util'] and .//T-Identifier[@Text='regex']][1]/ancestor::KotlinFile
+
+//FunctionBody//T-Identifier[@Text='Pattern' and ../../../PostfixUnarySuffix//T-Identifier[@Text='compile']
+    (: check for variables $action or ${action} in String of compile() call (first in possible call chain): allow dynamic input :)
+    and not(((../../..//ValueArgument)[1]//PostfixUnaryExpression//T-Identifier|(../../..//ValueArgument)[1]//T-LineStrRef)[
+       @Text=ancestor::FunctionDeclaration//FunctionValueParameter/Parameter/SimpleIdentifier/T-Identifier/@Text
+       or @Text=concat('$', ancestor::FunctionDeclaration//FunctionValueParameter/Parameter/SimpleIdentifier/T-Identifier/@Text)])
+]
+]]>
+                </value>
+            </property>
+        </properties>
+        <example>
+            <![CDATA[
+import java.util.regex.Pattern
+
+internal object Bad {
+    const val STR_PAT1: String = "[A-Z][a-z]+"
+
+    fun bad() {
+        val p1: Pattern = Pattern.compile(STR_PAT1) // bad
+        val p2: Pattern = Pattern.compile("(?=\\p{Lu})") // bad
+        val b: Boolean = p1.matcher("Start ").matches()
+    }
+}
+
+internal object Good {
+    val PAT1: Pattern = Pattern.compile("[A-Z][a-z]+")
+    val PAT2: Pattern = Pattern.compile("(?=\\p{Lu})")
+
+    fun good() {
+        val b: Boolean = PAT1.matcher("Start ").matches()
+    }
+}
+            ]]>
+        </example>
+    </rule>
+
     <rule name="AvoidRecompilingXPathExpression" class="net.sourceforge.pmd.lang.rule.XPathRule"
           language="kotlin"
           message="XPathExpression is created and compiled every time. Beware it is thread-unsafe."
diff --git a/src/test/java/com/jpinpoint/perf/lang/kotlin/ruleset/common/AvoidRecompilingPatternsTest.java b/src/test/java/com/jpinpoint/perf/lang/kotlin/ruleset/common/AvoidRecompilingPatternsTest.java
new file mode 100644
index 00000000..d9718fb3
--- /dev/null
+++ b/src/test/java/com/jpinpoint/perf/lang/kotlin/ruleset/common/AvoidRecompilingPatternsTest.java
@@ -0,0 +1,6 @@
+package com.jpinpoint.perf.lang.kotlin.ruleset.common;
+
+import net.sourceforge.pmd.testframework.PmdRuleTst;
+
+public class AvoidRecompilingPatternsTest extends PmdRuleTst {
+}
diff --git a/src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidRecompilingPatterns.xml b/src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidRecompilingPatterns.xml
new file mode 100644
index 00000000..5acb4dd0
--- /dev/null
+++ b/src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidRecompilingPatterns.xml
@@ -0,0 +1,100 @@
+<?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>violation: Avoid recompiling patterns</description>
+        <expected-problems>3</expected-problems>
+        <expected-linenumbers>5,12,13</expected-linenumbers>
+        <code><![CDATA[
+import java.util.regex.Pattern
+
+class Foo {
+    fun createNameFromAction(action: String?) {
+        val p = Pattern.compile("(?=\\p{Lu})") // bad
+    }
+
+    companion object {
+        const val PATTERN: String = "^\\s*%s=(.*)"
+
+        fun methodBad(action: String?) {
+            val p = Pattern.compile("(?=\\p{Lu})") // bad: violation, should not be inside method, make static final
+            val s = Pattern.compile(PATTERN) // bad: violation
+        }
+    }
+}
+    ]]></code>
+    </test-code>
+    <test-code>
+        <description>no violation: Avoid recompiling patterns</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.util.ArrayList;
+import java.util.List;
+import java.util.regex.Pattern;
+
+object Bar {
+    const val PATTERN: String = "^\\s*%s=(.*)"
+
+    fun createEventNameFromAction(action: String): List<*> {
+        val q1 = Pattern.compile("($action?=\\p{Lu})") // ok, dynamic -- TODO make function for this?
+        val q2 = Pattern.compile("(" + action + "?=\\p{Lu})") // ok, dynamic
+        val q3 = Pattern.compile("(${action}?=\\p{Lu})") // ok, dynamic
+        val r = Pattern.compile(String.format(PATTERN, action)) // ok, dynamic
+        val t = Pattern.compile(action) // dynamic, so ok (fix for JPCC-63)
+
+        val actionSplit = q.split(action) // ok
+        val list: MutableList<Array<String>> = ArrayList()
+        list.add(actionSplit)
+        return list
+    }
+}
+     ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>no violation: Avoid recompiling patterns, other compile</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+class GoodHere {
+    fun otherCompile() {
+        MyCompiler.compile("stuff")
+    }
+}
+
+internal object MyCompiler {
+    fun compile(s: String?) {
+    }
+}
+     ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>violation: Avoid recompiling patterns, with chained operations, issue #180</description>
+        <expected-problems>2</expected-problems>
+        <expected-linenumbers>5,10</expected-linenumbers>
+        <code><![CDATA[
+import java.util.regex.Pattern;
+
+class Foo {
+    private fun isAmountAllowed(amount: String): Boolean {
+        return LIMIT > Pattern.compile(SEP).matcher(amount).replaceAll("").length() // bad
+    }
+
+    private val isAmountAllowed2: Boolean
+        get() {
+            val bla: String = Pattern.compile("[.|,]").matcher("5.000,00").replaceAll("") // bad
+            val b = (11 > bla.length)
+            return b
+        }
+
+    companion object {
+        private const val LIMIT = 11
+        private const val SEP = "[.|,]"
+    }
+}
+     ]]></code>
+    </test-code>
+
+</test-data>

From 25bc9289fdba3c7c7832d320897c10f6a8bc092c Mon Sep 17 00:00:00 2001
From: Peter Paul Bakker <peterpaulbakker@stokpop.nl>
Date: Fri, 15 Mar 2024 11:44:50 +0100
Subject: [PATCH 3/3] remove deprecated version tags

---
 rulesets/kotlin/jpinpoint-kotlin-rules.xml | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/rulesets/kotlin/jpinpoint-kotlin-rules.xml b/rulesets/kotlin/jpinpoint-kotlin-rules.xml
index 4922536e..6be7b067 100644
--- a/rulesets/kotlin/jpinpoint-kotlin-rules.xml
+++ b/rulesets/kotlin/jpinpoint-kotlin-rules.xml
@@ -78,7 +78,6 @@ class Foo {
         <priority>2</priority>
         <properties>
             <property name="tag" value="jpinpoint-rule" type="String" description="for-sonar"/>
-            <property name="version" value="3.1"/>
             <property name="xpath">
                 <value><![CDATA[
 //ImportHeader[.//T-Identifier[@Text='java'] and .//T-Identifier[@Text='io']][1]/ancestor::KotlinFile
@@ -168,7 +167,6 @@ class AvoidRecompilingXPathExpressionKotlin {
         <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'],
@@ -205,7 +203,6 @@ class AvoidStringBuffer {
         <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[
 //ImportHeader[starts-with(@joinTokenText, 'importcom.netflix.hystrix')]
@@ -232,7 +229,6 @@ class AvoidStringBuffer {
         <priority>2</priority>
         <properties>
             <property name="tag" value="jpinpoint-rule" type="String" description="for-sonar"/>
-            <property name="version" value="3.1"/>
             <property name="xpath">
                 <value><![CDATA[
 (:locally created http client builder without disableConnectionState :)