Merge branch 'pr/3395' into 7.0.x

.ci/files/all-java.xml

@@ -291,7 +291,7 @@
     <rule ref="category/java/performance.xml/AddEmptyString"/>
     <rule ref="category/java/performance.xml/AppendCharacterWithChar"/>
     <!-- <rule ref="category/java/performance.xml/AvoidArrayLoops"/> -->
-    <!-- <rule ref="category/java/performance.xml/AvoidCalendarDateCreation"/> -->
+    <rule ref="category/java/performance.xml/AvoidCalendarDateCreation"/>
     <rule ref="category/java/performance.xml/AvoidFileStream"/>
     <rule ref="category/java/performance.xml/AvoidInstantiatingObjectsInLoops"/>
     <rule ref="category/java/performance.xml/AvoidUsingShortType"/>

pmd-java/src/main/resources/category/java/performance.xml

@@ -116,12 +116,13 @@ public class Test {
     <rule name="AvoidCalendarDateCreation"
           since="6.25.0"
           language="java"
-          message="A Calendar is used to create a Date or DateTime, this is expensive."
+          message="A Calendar is used to get the current time, this is expensive."
           class="net.sourceforge.pmd.lang.rule.XPathRule"
           typeResolution="true"
           externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_performance.html#avoidcalendardatecreation">
         <description>
-Problem: A Calendar is a heavyweight object and expensive to create.
+Problem: `java.util.Calendar` is a heavyweight object and expensive to create. It should only be used, if
+calendar calculations are needed.
 
 Solution: Use `new Date()`, Java 8+ `java.time.LocalDateTime.now()` or `ZonedDateTime.now()`.
         </description>
@@ -129,26 +130,32 @@ Solution: Use `new Date()`, Java 8+ `java.time.LocalDateTime.now()` or `ZonedDat
         <properties>
             <property name="xpath">
                 <value><![CDATA[
-//PrimaryPrefix[Name[ends-with(@Image, 'Calendar.getInstance')]] [count(../PrimarySuffix) > 2 and ../PrimarySuffix[last()-1][@Image = 'getTime' or @Image='getTimeInMillis']]
+//MethodCall[pmd-java:matchesSig("java.util.Calendar#getTime()") or pmd-java:matchesSig("java.util.Calendar#getTimeInMillis()")]
+  [*[1][local-name() = ('MethodCall', 'ConstructorCall')]
+                               [pmd-java:matchesSig("java.util.Calendar#getInstance()")
+                             or pmd-java:matchesSig("java.util.GregorianCalendar#getInstance()")
+                             or pmd-java:matchesSig("java.util.GregorianCalendar#new()")]
+  ]
 |
-//MethodDeclaration[not(MethodDeclarator/FormalParameters//ClassOrInterfaceType[pmd-java:typeIs('java.util.Calendar')])]
-  /Block/BlockStatement//PrimaryExpression
-  /PrimaryPrefix/Name
-    [pmd-java:typeIs('java.util.Calendar')]
-    [every $var in @Image satisfies (
-       (ends-with($var, '.getTime') or ends-with($var, '.getTimeInMillis'))
-       and
-       (: ignore if .set* or .add* or .clear is called on the variable :)
-       not(ancestor::Block/BlockStatement//Name[
-          starts-with(@Image, concat((tokenize($var, '\.'), $var)[1], '.set'))
-          or
-          starts-with(@Image, concat((tokenize($var, '\.'), $var)[1], '.add'))
-          or
-          starts-with(@Image, concat((tokenize($var, '\.'), $var)[1], '.clear'))
-       ])
-    )]
+//MethodCall[pmd-java:matchesSig("java.util.Calendar#getTime()") or pmd-java:matchesSig("java.util.Calendar#getTimeInMillis()")]
+   [*[1][local-name() = 'VariableAccess']]
+  (: ignore if .set* or .add or .clear or .roll is called on the variable :)
+  [not(VariableAccess/@Name = ancestor::Block//MethodCall[starts-with(@MethodName, "set") or @MethodName = ("add", "clear", "roll")]/VariableAccess/@Name)]
+  (: variable must be initialized with getInstance :)
+  [VariableAccess/@Name = ancestor::Block//LocalVariableDeclaration/VariableDeclarator[
+     (MethodCall | ConstructorCall)
+      [pmd-java:matchesSig("java.util.Calendar#getInstance()")
+       or pmd-java:matchesSig("java.util.GregorianCalendar#getInstance()")
+       or pmd-java:matchesSig("java.util.GregorianCalendar#new()")]
+  ]/VariableDeclaratorId/@Name]
 |
-//ClassOrInterfaceType[pmd-java:typeIs('org.joda.time.DateTime') or pmd-java:typeIs('org.joda.time.LocalDateTime')][../Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Name[ends-with(@Image, 'Calendar.getInstance')]]
+//ConstructorCall[pmd-java:typeIs("org.joda.time.DateTime") or pmd-java:typeIs("org.joda.time.LocalDateTime")]
+  [ArgumentList[(MethodCall | ConstructorCall)
+       [pmd-java:matchesSig("java.util.Calendar#getInstance()")
+     or pmd-java:matchesSig("java.util.GregorianCalendar#getInstance()")
+     or pmd-java:matchesSig("java.util.GregorianCalendar#new()")]]
+  ]
+
                 ]]></value>
             </property>
         </properties>

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidCalendarDateCreationTest.java

@@ -6,6 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.performance;
 
 import net.sourceforge.pmd.testframework.PmdRuleTst;
 
-@org.junit.Ignore("Rule has not been updated yet")
 public class AvoidCalendarDateCreationTest extends PmdRuleTst {
+    // no additional unit tests
 }

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidCalendarDateCreation.xml

@@ -135,6 +135,7 @@ public class Foo {
         <code><![CDATA[
 import java.util.Calendar;
 import java.util.Date;
+import java.util.GregorianCalendar;
 
 public class Foo {
     public Date onlySet(Date date) {
@@ -158,6 +159,117 @@ public class Foo {
         long originalTimestamp = calendar.getTimeInMillis();
         return calendar.getTime();
     }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>False positive with Calendar not created here</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.util.Calendar;
+import java.util.Date;
+
+public class AvoidCalenderDateCreationFalsePositive {
+    public static void main(String[] args) {
+        AvoidCalenderDateCreationFalsePositive fp = new AvoidCalenderDateCreationFalsePositive();
+        Date parsedDate = fp.create().getTime();
+    }
+
+    public Calendar create() {
+        return null;
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>False positive when this is a Calendar</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.util.Date;
+import java.util.GregorianCalendar;
+
+public class MyCalendar extends GregorianCalendar {
+    public Date getTime2() {
+        return new Date(getTimeInMillis());
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>False positive with calendar builder</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.util.Calendar;
+import java.util.Date;
+import java.util.TimeZone;
+
+public class AvoidCalendarDateCreationBuilder {
+    public void AvoidCalendarDateCreationBuilder(int year, int month, int day) {
+        Calendar c;
+        c = new Calendar.Builder().setTimeZone(TimeZone.getTimeZone("GMT"))
+                    .setDate(year, month - 1, day).build();
+        Date date = c.getTime();
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>False positive with specific date</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.util.Date;
+import java.util.GregorianCalendar;
+
+public class AvoidCalendarDateCreation {
+    public static void main(String[] args) {
+        Date date = new GregorianCalendar(2004, 1, 1).getTime();
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>False positive with calendar not created by getInstance</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.util.Calendar;
+import java.util.Date;
+
+public class AvoidCalendarDateCreation {
+    private Calendar calendar;
+
+    public void test() {
+        Date date = calendar.getTime();
+    }
+
+    public void test(Object in) {
+        if (in instanceof Calendar) {
+            Calendar cal = (Calendar) in;
+            Date date = cal.getTime();
+        }
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>New GregorianCalendar default constructor</description>
+        <expected-problems>2</expected-problems>
+        <expected-linenumbers>7,8</expected-linenumbers>
+        <code><![CDATA[
+import java.util.Calendar;
+import java.util.GregorianCalendar;
+
+public class AvoidCalendarDateCreation {
+    public void test() {
+        Calendar cal = new GregorianCalendar();
+        System.out.println(cal.getTime());
+        System.out.println(new GregorianCalendar().getTime());
+    }
 }
         ]]></code>
     </test-code>