From dde8c01dd3fc8c880742a6f46c6f89d05370b22d Mon Sep 17 00:00:00 2001
From: jborgers <jborgers@jpinpoint.com>
Date: Fri, 2 Aug 2024 23:12:38 +0200
Subject: [PATCH] Migrate java rules to pmd 7.x.x #309: rule number 10:
 AvoidRecreatingDateTimeFormatter

---
 .../resources/category/java/common.xml        |  55 ----------
 src/main/resources/category/java/common.xml   |  69 +++++++++---
 .../AvoidRecreatingDateTimeFormatterTest.java |   0
 .../xml/AvoidRecreatingDateTimeFormatter.xml  | 101 ++++++++++++++----
 4 files changed, 130 insertions(+), 95 deletions(-)
 rename src/{test-todo => test/java/com/jpinpoint/perf/lang}/java/ruleset/common/AvoidRecreatingDateTimeFormatterTest.java (100%)
 rename src/{test-todo/resources-todo => test/resources/com/jpinpoint/perf}/lang/java/ruleset/common/xml/AvoidRecreatingDateTimeFormatter.xml (51%)

diff --git a/src/main-todo/resources/category/java/common.xml b/src/main-todo/resources/category/java/common.xml
index c4040061..c433f69e 100644
--- a/src/main-todo/resources/category/java/common.xml
+++ b/src/main-todo/resources/category/java/common.xml
@@ -1,62 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <ruleset name="jpinpoint-common-rules" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd" xmlns="http://pmd.sourceforge.net/ruleset/2.0.0">
     <description>jPinpoint rule set for performance aware Java coding, sponsored by Rabobank.</description>
-    
 
-    <!--rule name="AvoidRecompilingXPathExpression" class="net.sourceforge.pmd.lang.rule.xpath.XPathRule" language="java" message="XPathExpression is created and compiled every time. Beware it is thread-unsafe."
-          externalInfoUrl="${doc_root}/JavaCodePerformance.md#ux02">
-        <description>XPathExpression is created and compiled on every method call, compiled possibly implicitly by XPath::evaluate.
-            Problem: Creation of XPath and compilation of XPathExpression takes time. It may slow down your application. &#13;
-            Solution: 1. Avoid XPath usage. 2. Compile the xpath expression as String into a XPathExpression. However, since XPath and XPathExpression classes are thread-unsafe, they are not easily cached. Caching it in a ThreadLocal may be a solution.
-            (jpinpoint-rules)</description>
-        <priority>2</priority>
-        <properties>
-            <property name="tags" value="jpinpoint-rule,performance" type="String" description="classification"/>
-            <property name="xpath">
-                <value><![CDATA[
-//TypeDeclaration//ClassOrInterfaceBodyDeclaration/MethodDeclaration
-//PrimaryExpression/(PrimarySuffix|PrimaryPrefix/Name)[ends-with(@Image,'newXPath')]
-/ancestor::MethodDeclaration
-//PrimaryExpression/(PrimarySuffix|PrimaryPrefix/Name)[ends-with(@Image, 'compile')
-or (pmd-java:typeIs('javax.xml.xpath.XPath') and ends-with(@Image, 'evaluate'))]
-]]></value>
-            </property>
-        </properties>
-        <example>
-            <![CDATA[
-class Bad {
-    public static NodeList bad1(Document doc) {
-        XPath xpath = XPathFactory.newInstance().newXPath();
-        XPathExpression expr = xpath.compile("//book[author='Isaac Asimov']/title/text()"); // bad
-        return (NodeList) expr.evaluate(doc, XPathConstants.NODESET);
-    }
-    public static NodeList bad2(Document doc) throws XPathExpressionException {
-        XPath xpath = XPathFactory.newInstance().newXPath();
-        String xPathQuery = "//book[author='Isaac Asimov']/title/text()";
-        return (NodeList) xpath.evaluate(xPathQuery, doc, XPathConstants.NODESET); // bad
-    }
-}
-
-class Good {
-    private static final ThreadLocal<XPathFactory> tlFac = ThreadLocal.withInitial(XPathFactory::newInstance);
-    private static final ThreadLocal<XPathExpression> tlExpr;
-    static {
-        XPath xpath = tlFac.get().newXPath();
-        try {
-            XPathExpression expr = xpath.compile("//book[author='Isaac Asimov']/title/text()");
-            tlExpr = ThreadLocal.withInitial(() -> expr); // good
-        } catch (XPathExpressionException e) {
-            throw new RuntimeException(e);
-        }
-    }
-    public static NodeList good(Document doc) throws XPathExpressionException {
-        return (NodeList) tlExpr.get().evaluate(doc, XPathConstants.NODESET); // good
-    }
-            ]]>
-        </example>
-
-
-    </rule-->
 
     <rule name="AvoidRecreatingDateTimeFormatter"
           message="Avoid recreating DateTimeFormatter, it is relatively expensive."
diff --git a/src/main/resources/category/java/common.xml b/src/main/resources/category/java/common.xml
index 8ad17ec5..de057b47 100644
--- a/src/main/resources/category/java/common.xml
+++ b/src/main/resources/category/java/common.xml
@@ -323,22 +323,22 @@ class Good {
 
     <rule name="AvoidRecompilingXPathExpression" class="net.sourceforge.pmd.lang.rule.xpath.XPathRule" language="java" message="XPathExpression is created and compiled every time. Beware it is thread-unsafe."
       externalInfoUrl="${doc_root}/JavaCodePerformance.md#ux02">
-    <description>XPathExpression is created and compiled on every method call, compiled possibly implicitly by XPath::evaluate.
-        Problem: Creation of XPath and compilation of XPathExpression takes time. It may slow down your application. &#13;
-        Solution: 1. Avoid XPath usage. 2. Compile the xpath expression as String into a XPathExpression. However, since XPath and XPathExpression classes are thread-unsafe, they are not easily cached. Caching it in a ThreadLocal may be a solution.
-        (jpinpoint-rules)</description>
-    <priority>2</priority>
-    <properties>
-        <property name="tags" value="jpinpoint-rule,performance" type="String" description="classification"/>
-        <property name="xpath">
-            <value><![CDATA[
+        <description>XPathExpression is created and compiled on every method call, compiled possibly implicitly by XPath::evaluate.
+            Problem: Creation of XPath and compilation of XPathExpression takes time. It may slow down your application. &#13;
+            Solution: 1. Avoid XPath usage. 2. Compile the xpath expression as String into a XPathExpression. However, since XPath and XPathExpression classes are thread-unsafe, they are not easily cached. Caching it in a ThreadLocal may be a solution.
+            (jpinpoint-rules)</description>
+        <priority>2</priority>
+        <properties>
+            <property name="tags" value="jpinpoint-rule,performance" type="String" description="classification"/>
+            <property name="xpath">
+                <value><![CDATA[
 //MethodDeclaration//MethodCall[pmd-java:matchesSig('javax.xml.xpath.XPath#compile(java.lang.String)') or
     pmd-java:matchesSig('javax.xml.xpath.XPath#evaluate(java.lang.String,java.lang.Object,javax.xml.namespace.QName)')]
-]]></value>
-        </property>
-    </properties>
-    <example>
-        <![CDATA[
+             ]]></value>
+            </property>
+        </properties>
+        <example>
+            <![CDATA[
 class Bad {
     public static NodeList bad1(Document doc) {
         XPath xpath = XPathFactory.newInstance().newXPath();
@@ -368,9 +368,44 @@ class Good {
         return (NodeList) tlExpr.get().evaluate(doc, XPathConstants.NODESET); // good
     }
 }
-        ]]>
-    </example>
-</rule>
+            ]]>
+        </example>
+    </rule>
 
+    <rule name="AvoidRecreatingDateTimeFormatter"
+          message="Avoid recreating DateTimeFormatter, it is relatively expensive."
+          class="net.sourceforge.pmd.lang.rule.xpath.XPathRule" language="java"
+          externalInfoUrl="${doc_root}/JavaCodePerformance.md#idtf02">
+        <description>
+            Problem: Recreating a DateTimeFormatter is relatively expensive.&#13;
+            Solution: Java 8+ java.time.DateTimeFormatter is thread-safe and can be shared among threads. Create the
+            formatter from a pattern only once, to initialize a static final field.
+            (jpinpoint-rules)</description>
+        <priority>2</priority>
+        <properties>
+            <property name="tags" value="jpinpoint-rule,performance" type="String" description="classification"/>
+            <property name="xpath">
+                <value>
+                    <![CDATA[
+//FieldDeclaration
+[ClassType[pmd-java:typeIs('java.time.format.DateTimeFormatter') or pmd-java:typeIs('org.joda.time.format.DateTimeFormatter')]]
+[(not(pmd-java:modifiers() = 'static') and VariableDeclarator[@Initializer=true()]) or not(pmd-java:modifiers() = 'final')]
+|
+//MethodDeclaration//ConstructorCall[pmd-java:matchesSig('org.joda.time.format.DateTimeFormatter#new(_,_)')]
+|
+//(MethodDeclaration|ConstructorDeclaration)//MethodCall[((pmd-java:matchesSig('java.time.format.DateTimeFormatter#ofPattern(_)')
+ or pmd-java:matchesSig('org.joda.time.format.DateTimeFormat#forPattern(_)'))
+   and not(ArgumentList/VariableAccess/@Name = ancestor::Block/..//FormalParameter/VariableId/@Name))
+ or pmd-java:matchesSig('java.time.format.DateTimeFormatterBuilder#toFormatter()')
+ or pmd-java:matchesSig('java.time.format.DateTimeFormatterBuilder#toFormatter(_)')
+ or pmd-java:matchesSig('org.joda.time.format.DateTimeFormatterBuilder#toFormatter()')
+ or pmd-java:matchesSig('org.joda.time.format.ISODateTimeFormat#_()')
+ or pmd-java:matchesSig('org.joda.time.format.DateTimeFormat#fullDateTime()')
+]
+]]>
+                </value>
+            </property>
+        </properties>
+    </rule>
 
 </ruleset>
diff --git a/src/test-todo/java/ruleset/common/AvoidRecreatingDateTimeFormatterTest.java b/src/test/java/com/jpinpoint/perf/lang/java/ruleset/common/AvoidRecreatingDateTimeFormatterTest.java
similarity index 100%
rename from src/test-todo/java/ruleset/common/AvoidRecreatingDateTimeFormatterTest.java
rename to src/test/java/com/jpinpoint/perf/lang/java/ruleset/common/AvoidRecreatingDateTimeFormatterTest.java
diff --git a/src/test-todo/resources-todo/lang/java/ruleset/common/xml/AvoidRecreatingDateTimeFormatter.xml b/src/test/resources/com/jpinpoint/perf/lang/java/ruleset/common/xml/AvoidRecreatingDateTimeFormatter.xml
similarity index 51%
rename from src/test-todo/resources-todo/lang/java/ruleset/common/xml/AvoidRecreatingDateTimeFormatter.xml
rename to src/test/resources/com/jpinpoint/perf/lang/java/ruleset/common/xml/AvoidRecreatingDateTimeFormatter.xml
index 221875b9..ff84f171 100644
--- a/src/test-todo/resources-todo/lang/java/ruleset/common/xml/AvoidRecreatingDateTimeFormatter.xml
+++ b/src/test/resources/com/jpinpoint/perf/lang/java/ruleset/common/xml/AvoidRecreatingDateTimeFormatter.xml
@@ -4,9 +4,57 @@
         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>Avoid recreation of DateTimeFormatter object.</description>
-        <expected-problems>13</expected-problems>
-        <expected-linenumbers>4,5,6,14,17,20,23,27,30,34,46,53,57</expected-linenumbers>
+        <description>Avoid recreation of java.time.format DateTimeFormatter object.</description>
+        <expected-problems>9</expected-problems>
+        <expected-linenumbers>5,6,7,12,16,19,23,32,38</expected-linenumbers>
+        <code><![CDATA[
+import java.time.format.*;
+import java.util.Locale;
+
+public class AvoidRecreatingJavaTime {
+    final DateTimeFormatter wrong = DateTimeFormatter.ofPattern("yyMMdd"); // bad, not static
+    static DateTimeFormatter wrongAgain = DateTimeFormatter.ofPattern("yyMMdd"); // bad, not final
+    DateTimeFormatter stillWrong = DateTimeFormatter.ofPattern("yyMMdd"); // bad, not final, not static
+    static final DateTimeFormatter ok1 = DateTimeFormatter.ofPattern("yyyy-MM-dd");
+    private static final DateTimeFormatter ok2 = DateTimeFormatter.ofPattern("xxx");
+
+    public void testViolation2() {
+        DateTimeFormatter dtf = new DateTimeFormatterBuilder().toFormatter(); // bad
+    }
+    public void testViolation3() {
+        Locale loc = new Locale("nl", "NL");
+        DateTimeFormatter dtf = new DateTimeFormatterBuilder().toFormatter(loc);  // bad
+    }
+    public void testViolation4() {
+        stillWrong = DateTimeFormatter.ofPattern(""); // bad
+    }
+    public void testViolation7_PCC_171() {
+        LocalDateTime time = LocalDateTime.now();
+        String s = DateTimeFormatter.ofPattern("yyDDD").format(time); // bad
+    }
+    public DateTimeFormatter testNoViolation() {
+        return null;
+    }
+    public void testNoViolation_JPCC_16(String dateFormat) {
+        DateTimeFormatter dateTimeFormatter = getDateTimeFormatterFromCacheViolation(dateFormat);
+    }
+    private DateTimeFormatter getDateTimeFormatterFromCacheViolation(String dateFormat) {
+        return new DateTimeFormatterBuilder().toFormatter(); // bad
+    }
+    public void testNoViolation_2(String dateFormat) {
+        DateTimeFormatter dateTimeFormatter = DateTimeFormatter.ofPattern(dateFormat); // good: possible different dataFormat params
+    }
+    public void testViolationJava8() {
+        java.time.format.DateTimeFormatter ftor = java.time.format.DateTimeFormatter.ofPattern("yyMMdd"); // bad
+    }
+}
+     ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Avoid recreation of joda DateTimeFormatter object.</description>
+        <expected-problems>11</expected-problems>
+        <expected-linenumbers>4,5,6,12,15,18,21,24,27,31,40</expected-linenumbers>
         <code><![CDATA[
 import org.joda.time.format.*;
 
@@ -16,8 +64,6 @@ public class Foo {
     DateTimeFormatter stillWrong = ISODateTimeFormat.basicDate(); // bad, not final, not static
     static final DateTimeFormatter ok1 = DateTimeFormat.forPattern("xxx");
     private static final DateTimeFormatter ok2 = DateTimeFormat.forPattern("xxx");
-    public static final org.threeten.bp.format.DateTimeFormatter YMD_FORMATTER = org.threeten.bp.format.DateTimeFormatter.ofPattern("yyyy-MM-dd");
-
 
     public void testViolation1(DateTimePrinter printer, DateTimeParser parser)  {
         DateTimeFormatter dtf = null;
@@ -32,45 +78,34 @@ public class Foo {
     public void testViolation4()  {
         stillWrong = DateTimeFormat.fullDateTime(); // bad
     }
-
     public void testViolation5()  {
         stillWrong = DateTimeFormat.forPattern(""); // bad
     }
     public void testViolation6(DateTimePrinter printer,DateTimeParser parser)  {
-        stillWrong =  new DateTimeFormatter(printer, parser); // bad
+        stillWrong = new DateTimeFormatter(printer, parser); // bad
     }
     public void testViolation7_PCC_171() {
         long dt = 1L;
         String s = DateTimeFormat.forPattern("yyDDD").print(dt); // bad
     }
-
     public DateTimeFormatter testNoViolation(){
         return null;
     }
-
     public void testNoViolation_JPCC_16(String dateFormat) {
         DateTimeFormatter dateTimeFormatter = getDateTimeFormatterFromCacheViolation(dateFormat);
     }
-
     private DateTimeFormatter getDateTimeFormatterFromCacheViolation(String dateFormat) {
         return new DateTimeFormatterBuilder().toFormatter(); // bad
     }
     public void testNoViolation_2(String dateFormat) {
         DateTimeFormatter dateTimeFormatter = DateTimeFormat.forPattern(dateFormat); // good: possible different dataFormat params
     }
-
-    public void testViolationThreeten() {
-        org.threeten.bp.format.DateTimeFormatter ftor = org.threeten.bp.format.DateTimeFormatter.ofPattern("yyMMdd"); // bad
-    }
-
-    public void testViolationJava8() {
-        java.time.format.DateTimeFormatter ftor = java.time.format.DateTimeFormatter.ofPattern("yyMMdd"); // bad
-    }
 }
      ]]></code>
     </test-code>
+
     <test-code>
-        <description>Avoid recreation of DateTimeFormatter object. But allow in constructor if member variable is final.</description>
+        <description>Avoid recreation of joda DateTimeFormatter object. But allow in constructor if member variable is final.</description>
         <expected-problems>3</expected-problems>
         <expected-linenumbers>5,6,9</expected-linenumbers>
         <code><![CDATA[
@@ -88,9 +123,9 @@ public class Foo {
      ]]></code>
     </test-code>
     <test-code>
-        <description>Avoid recreation of DateTimeFormatter object. But allow if there is a parameter from method call involved.</description>
-        <expected-problems>3</expected-problems>
-        <expected-linenumbers>7,8,9</expected-linenumbers>
+        <description>Avoid recreation of joda DateTimeFormatter object. But allow if there is a parameter from method call involved.</description>
+        <expected-problems>4</expected-problems>
+        <expected-linenumbers>7,8,9,10</expected-linenumbers>
         <code><![CDATA[
 import org.joda.time.format.*;
 
@@ -101,9 +136,29 @@ public class Foo {
         final DateTimeFormatter formatter2 = DateTimeFormat.forPattern(DEFAULT_DATETIME_FORMAT); // bad
         final DateTimeFormatter formatter3 = DateTimeFormat.forPattern(DEFAULT_DATETIME_FORMAT).withZoneUTC(); // bad
         final DateTimeFormatter formatter4 = DateTimeFormat.forPattern(DEFAULT_DATETIME_FORMAT).withZone(ZoneId.of("UTC")); // bad
-        final DateTimeFormatter formatter5 = DateTimeFormat.forPattern(DEFAULT_DATETIME_FORMAT).withZone(myTimeZone); // good: has variable parameter
+        final DateTimeFormatter formatter5 = DateTimeFormat.forPattern(DEFAULT_DATETIME_FORMAT).withZone(myTimeZone); // bad (was good: has variable parameter - actually bad, can create formatter)
         return formatter1.print(milliseconds);
     }
+}
+     ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Avoid recreation of java.time.DateTimeFormatter object. But allow if there is a parameter from method call involved.</description>
+        <expected-problems>3</expected-problems>
+        <expected-linenumbers>7,8,9</expected-linenumbers>
+        <code><![CDATA[
+import java.time.format.*;
+
+class Foo4 {
+    private static final String DEFAULT_DATETIME_FORMAT = "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'";
+    public static String format(final LocalDateTime dateTime, String myFormat, String myTimeZone) {
+        final DateTimeFormatter formatter1 = DateTimeFormatter.ofPattern(myFormat);// good: has variable parameter
+        final DateTimeFormatter formatter2 = DateTimeFormatter.ofPattern(DEFAULT_DATETIME_FORMAT); // bad
+        final DateTimeFormatter formatter4 = DateTimeFormatter.ofPattern(DEFAULT_DATETIME_FORMAT).withZone(ZoneId.of("UTC")); // bad
+        final DateTimeFormatter formatter5 = DateTimeFormatter.ofPattern(DEFAULT_DATETIME_FORMAT).withZone(ZoneId.of(myTimeZone)); // good: has variable parameter
+        return formatter1.format(dateTime);
+    }
 }
      ]]></code>
     </test-code>