From 96c30916873759cc9ac53c36c0c584fee00b5988 Mon Sep 17 00:00:00 2001
From: jborgers <jborgers@jpinpoint.com>
Date: Fri, 26 Jul 2024 18:03:17 +0200
Subject: [PATCH] Migrate java rules to pmd 7.x.x #309: 1 complex rule
 migrated, number 5: AvoidImplicitlyRecompilingRegex

---
 .../resources/category/java/common.xml        |  4 +-
 src/main/resources/category/java/common.xml   | 91 ++++++-------------
 .../AvoidImplicitlyRecompilingRegexTest.java  |  0
 .../xml/AvoidImplicitlyRecompilingRegex.xml   |  8 +-
 4 files changed, 34 insertions(+), 69 deletions(-)
 rename src/{test-todo => test/java/com/jpinpoint/perf/lang}/java/ruleset/common/AvoidImplicitlyRecompilingRegexTest.java (100%)
 rename src/{test-todo/resources-todo => test/resources/com/jpinpoint/perf}/lang/java/ruleset/common/xml/AvoidImplicitlyRecompilingRegex.xml (97%)

diff --git a/src/main-todo/resources/category/java/common.xml b/src/main-todo/resources/category/java/common.xml
index 594309d1..2559ea7a 100644
--- a/src/main-todo/resources/category/java/common.xml
+++ b/src/main-todo/resources/category/java/common.xml
@@ -133,7 +133,7 @@ or preceding-sibling::SwitchLabel[@Default=true()])
         </properties>
     </rule-->
 
-    <rule name="AvoidImplicitlyRecompilingRegex" class="net.sourceforge.pmd.lang.rule.xpath.XPathRule" dfa="false" language="java" message="String regex method, Pattern.matches or FileSystem.getPathMatcher is used.
+    <!--rule name="AvoidImplicitlyRecompilingRegex" class="net.sourceforge.pmd.lang.rule.xpath.XPathRule" dfa="false" language="java" message="String regex method, Pattern.matches or FileSystem.getPathMatcher is used.
 	   Implicitly compiles a regex pattern, can be expensive." typeResolution="true"
           externalInfoUrl="${doc_root}/JavaCodePerformance.md#ireu01">
         <description>A regular expression is compiled implicitly on every invocation. Problem: this can be expensive, depending on the length of the regular expression.&#13;
@@ -224,7 +224,7 @@ String good_replaceInnerLineBreakBySpace() {
 }
             ]]>
         </example>
-    </rule>
+    </rule-->
 
     <rule name="AvoidInMemoryStreamingDefaultConstructor" class="net.sourceforge.pmd.lang.rule.xpath.XPathRule" dfa="false" language="java"
           message="The default capacity or smaller is used for ByteArrayOutputStream or StringWriter, it usually needs expensive expansions." typeResolution="true"
diff --git a/src/main/resources/category/java/common.xml b/src/main/resources/category/java/common.xml
index c30543d4..95eaf130 100644
--- a/src/main/resources/category/java/common.xml
+++ b/src/main/resources/category/java/common.xml
@@ -151,79 +151,30 @@ and not (preceding-sibling::SwitchLabel[@Default=true()])]
         </properties>
     </rule>
 
-    <!--rule name="AvoidImplicitlyRecompilingRegex" class="net.sourceforge.pmd.lang.rule.xpath.XPathRule" dfa="false" language="java" message="String regex method, Pattern.matches or FileSystem.getPathMatcher is used.
+    <rule name="AvoidImplicitlyRecompilingRegex" class="net.sourceforge.pmd.lang.rule.xpath.XPathRule" dfa="false" language="java" message="String regex method, Pattern.matches or FileSystem.getPathMatcher is used.
 	   Implicitly compiles a regex pattern, can be expensive." typeResolution="true"
           externalInfoUrl="${doc_root}/JavaCodePerformance.md#ireu01">
         <description>A regular expression is compiled implicitly on every invocation. Problem: this can be expensive, depending on the length of the regular expression.&#13;
-            Solution: Compile the regex pattern only once and assign it to a private static final Pattern field. java.util.Pattern objects are thread-safe so they can be shared among threads.
+            Solution: Compile the regex pattern only once and assign it to a private static final Pattern field. java.util.Pattern objects are thread-safe, so they can be shared among threads.
             (jpinpoint-rules)</description>
         <priority>2</priority>
         <properties>
             <property name="tags" value="jpinpoint-rule,performance" type="String" description="classification"/>
             <property name="xpath">
                 <value><![CDATA[
-(//MethodDeclaration//(PrimaryPrefix/Name[ends-with(@Image, '.replaceAll') or ends-with(@Image, '.replaceFirst') or @Image='Pattern.matches']
-|PrimarySuffix[@Image = 'replaceAll' or ends-with(@Image, '.replaceFirst')])
-/following::PrimarySuffix[1]/Arguments[ArgumentList/@Size=2]//Expression[1]//PrimaryPrefix[
-Literal[string-length(@Image) > 5 and
-(matches(@Image, '[\.\$\|\(\)\[\]\{\}\^\?\*\+\\]+'))] or Name
-and not(../PrimarySuffix)
-and not (
-Name/@Image=ancestor::MethodDeclaration//VariableDeclaratorId/@Name)
-and not (
-Name[@Image=ancestor::ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/FieldDeclaration/VariableDeclarator[
-VariableInitializer/Expression/PrimaryExpression/PrimaryPrefix/Literal[string-length(@Image) < 6 or not
-(matches(@Image, '[\.\$\|\(\)\[\]\{\}\^\?\*\+\\]+'))]
-]/VariableDeclaratorId/@Name])
+(:- method calls for non-short regex literals and used fields defined with non-short regex literal, or not defined as field -:)
+//MethodDeclaration//MethodCall[pmd-java:matchesSig('java.lang.String#replaceAll(java.lang.String,java.lang.String)')
+    or pmd-java:matchesSig('java.lang.String#replaceFirst(java.lang.String,java.lang.String)')
+    or pmd-java:matchesSig('java.util.regex.Pattern#matches(java.lang.String,java.lang.CharSequence)')
+    or pmd-java:matchesSig('java.lang.String#split(java.lang.String)')
+    or pmd-java:matchesSig('java.lang.String#matches(java.lang.String)')
+    or pmd-java:matchesSig('java.nio.file.FileSystem#getPathMatcher(java.lang.String)')
 ]
-,
-//MethodDeclaration//PrimaryPrefix/Name[ends-with(@Image, '.split') or ends-with(@Image, 'getPathMatcher')]/../../PrimarySuffix/Arguments[ArgumentList/@Size=1]//Expression[1]//PrimaryPrefix[
-Literal[string-length(@Image) > 5 and
-matches(@Image, '[\.\$\|\(\)\[\]\{\}\^\?\*\+\\]+')] or Name
-and not(../PrimarySuffix)
-and not (
-Name/@Image=ancestor::MethodDeclaration//VariableDeclaratorId/@Name)
-and not (
-(:- not (if foreign field (with .), or field with short String or without regex char) -:)
-Name[contains(@Image, '.') or @Image=ancestor::ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/FieldDeclaration/VariableDeclarator[
-VariableInitializer/Expression/PrimaryExpression/PrimaryPrefix/Literal[string-length(@Image) < 6 or not
-(matches(@Image, '[\.\$\|\(\)\[\]\{\}\^\?\*\+\\]+'))]
-]/VariableDeclaratorId/@Name])
-]
-,
-//MethodDeclaration//PrimarySuffix[@Image='getPathMatcher']/../PrimarySuffix/Arguments[ArgumentList/@Size=1]/ArgumentList/Expression[1]/PrimaryExpression/PrimaryPrefix[
-Literal[string-length(@Image) > 5] or Name
-and not(../PrimarySuffix)
-and not (
-Name/@Image=ancestor::MethodDeclaration//VariableDeclaratorId/@Name)
-and not (
-Name[@Image=ancestor::ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/FieldDeclaration/VariableDeclarator[
-VariableInitializer/Expression/PrimaryExpression/PrimaryPrefix/Literal[string-length(@Image) < 6]]/VariableDeclaratorId/@Name])
-]
-,
-(: - String.matches called on formalparams, locals and fields - :)
-//MethodDeclaration//PrimaryPrefix/Name[ends-with(@Image, '.matches')]
-[
-(exists(index-of((ancestor::MethodDeclaration//FormalParameter[pmd-java:typeIs('java.lang.String')]/VariableDeclaratorId/@Name), substring-before(@Image,'.')))
+/ArgumentList/*[1][(local-name()='StringLiteral' and string-length(@Image) > 5 and
+(matches(@Image, '[\.\$\|\(\)\[\]\{\}\^\?\*\+\\]+')))
 or
-exists(index-of((ancestor::MethodDeclaration//LocalVariableDeclaration/Type[pmd-java:typeIs('java.lang.String')]/../VariableDeclarator/VariableDeclaratorId/@Name), substring-before(@Image,'.')))
-or
-exists(index-of((ancestor::ClassOrInterfaceBody//FieldDeclaration[pmd-java:typeIs('java.lang.String')]/VariableDeclarator/VariableDeclaratorId/@Name), substring-before(@Image,'.'))))
-and
-(: for matches param is >5 literal or something named :)
-../../PrimarySuffix/Arguments[ArgumentList/@Size=1]//Expression[1]//PrimaryPrefix[
-Literal[string-length(@Image) > 5] or Name
-(: exclude method calls :)
-and not(../PrimarySuffix)
-(: exclude for param is method arg or local :)
-and not (
-Name/@Image=ancestor::MethodDeclaration//VariableDeclaratorId/@Name)
-(: exclude for param is short fields :)
-and not (
-Name[@Image=ancestor::ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/FieldDeclaration/VariableDeclarator[
-VariableInitializer/Expression/PrimaryExpression/PrimaryPrefix/Literal[string-length(@Image) < 6]
-]/VariableDeclaratorId/@Name])
-]])
+(local-name()='VariableAccess') and @Name=ancestor::ClassBody/FieldDeclaration/VariableDeclarator[StringLiteral[string-length(@Image) > 5 and
+(matches(@Image, '[\.\$\|\(\)\[\]\{\}\^\?\*\+\\]+'))] or not(StringLiteral)]/VariableId/@Name]
 ]]></value>
             </property>
         </properties>
@@ -242,6 +193,20 @@ String good_replaceInnerLineBreakBySpace() {
 }
             ]]>
         </example>
-    </rule-->
+    </rule>
+
+    <!-- temp saved
+
+    (:- String#replaceAll or String#replaceFirst and Pattern.matches for non-short regex literals and used fields defined with non-short regex literal -:)
+//MethodDeclaration//MethodCall[pmd-java:matchesSig('java.lang.String#replaceAll(java.lang.String,java.lang.String)')
+    or pmd-java:matchesSig('java.lang.String#replaceFirst(java.lang.String,java.lang.String)')
+    or (pmd-java:matchesSig('java.util.regex.Pattern#matches(java.lang.String,java.lang.CharSequence)'))]
+/ArgumentList[@Size=2]/*[1][(local-name()='StringLiteral' and string-length(@Image) > 5 and
+(matches(@Image, '[\.\$\|\(\)\[\]\{\}\^\?\*\+\\]+')))
+or
+(local-name()='VariableAccess') and @Name=ancestor::ClassBody/FieldDeclaration/VariableDeclarator[StringLiteral[string-length(@Image) > 5 and
+(matches(@Image, '[\.\$\|\(\)\[\]\{\}\^\?\*\+\\]+'))]]/VariableId/@Name]
+
+-->
 
 </ruleset>
diff --git a/src/test-todo/java/ruleset/common/AvoidImplicitlyRecompilingRegexTest.java b/src/test/java/com/jpinpoint/perf/lang/java/ruleset/common/AvoidImplicitlyRecompilingRegexTest.java
similarity index 100%
rename from src/test-todo/java/ruleset/common/AvoidImplicitlyRecompilingRegexTest.java
rename to src/test/java/com/jpinpoint/perf/lang/java/ruleset/common/AvoidImplicitlyRecompilingRegexTest.java
diff --git a/src/test-todo/resources-todo/lang/java/ruleset/common/xml/AvoidImplicitlyRecompilingRegex.xml b/src/test/resources/com/jpinpoint/perf/lang/java/ruleset/common/xml/AvoidImplicitlyRecompilingRegex.xml
similarity index 97%
rename from src/test-todo/resources-todo/lang/java/ruleset/common/xml/AvoidImplicitlyRecompilingRegex.xml
rename to src/test/resources/com/jpinpoint/perf/lang/java/ruleset/common/xml/AvoidImplicitlyRecompilingRegex.xml
index cc7b0210..6de10712 100644
--- a/src/test-todo/resources-todo/lang/java/ruleset/common/xml/AvoidImplicitlyRecompilingRegex.xml
+++ b/src/test/resources/com/jpinpoint/perf/lang/java/ruleset/common/xml/AvoidImplicitlyRecompilingRegex.xml
@@ -5,8 +5,8 @@
         xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
     <test-code>
         <description>violation: Avoid implicit recompiling of regular expressions</description>
-        <expected-problems>27</expected-problems>
-        <expected-linenumbers>18,19,20, 24,25,27, 31,32,33, 39,40,41,43,45, 52,53,54, 59,60,61,61,64, 70, 76, 82,83,84</expected-linenumbers>
+        <expected-problems>26</expected-problems>
+        <expected-linenumbers>18,19,20, 24,25,27, 31,32,33, 39,40,41,43,45, 52,53,54, 59,60,61,64, 70, 76, 82,83,84</expected-linenumbers>
         <code><![CDATA[
 import java.io.File;
 import java.nio.file.FileSystem;
@@ -68,7 +68,7 @@ public class Foo {
     public void violatingPathMatcher4x(String action) {
         PathMatcher p = FileSystems.getDefault().getPathMatcher("(?=\\p{Lu})"); // bad: violation, should not be inside method, make static final
         FileSystems.getDefault().getPathMatcher(PAT_STRING); // bad: violation
-        FileSystems.getDefault().getPathMatcher(dynPatString).matches(path); // bad (2x):  field should be PathMatcher
+        FileSystems.getDefault().getPathMatcher(dynPatString).matches(path); // bad (2x(*)):  field should be PathMatcher -> why 2x?, just once: The matches to violate is String.matches, not PathMatcher.matches. With pmd7 easy to get right.
 
         FileSystem fs = FileSystems.getDefault();
         p = fs.getPathMatcher(PAT_STRING); // bad: violation
@@ -303,7 +303,7 @@ class Foo {
                 .stream()
                 .findFirst()
                 .toString()
-                .replaceAll("\\s+", "-"); // missing violation
+                .replaceAll("\\s+", "-"); // bad, missing violation
     }
 }
             ]]></code>