AvoidRecompilingXPathExpression for Kotlin

docs/EvaluateRulesForKotlin.md

@@ -12,7 +12,7 @@ Evaluation of Java PMD rules for use as Kotlin rules
 | 6.  | JB-1-M   | AvoidInMemoryStreamingDefaultConstructor | Yes    | Low        | Yes              | High        | Not found         | Kotlin types? -> No                                                                                                     |
 | 7.  | JB-5     | AvoidMultipleConcatStatements            | Yes    | Medium     | Yes              | High        | Not found         | How concat in Kotlin? Seems like Java                                                                                   | 
 | 8.  | PP-3-WIP | AvoidRecompilingPatterns                 | Yes    | Low/Medium | Yes              | High        | Not found         | Kotlin version?                                                                                                         |
-| 9.  | JB-3-WIP | AvoidRecompilingXPathExpression          | Yes    | Low        | Yes              | Medium/High | Not found         | Good example ThreadLocal in Kotlin?                                                                                     |
+| 9.  | JB-3-WIP | AvoidRecompilingXPathExpression          | Yes    | Low        | Yes              | Medium/High | Not found         | Good example ThreadLocal in Kotlin? TODO                                                                                |
 | 10. | PP-4     | AvoidRecreatingDateTimeFormatter         | Yes    | Medium     | Yes              | High        | Not found         | -                                                                                                                       |
 | 11. |          | AvoidReflectionInToStringAndHashCode     | Yes    | Low/Medium | Yes              | Low/Medium  | Not found         | -                                                                                                                       |
 | 12. | PP-2-PR  | AvoidSimpleDateFormat                    | Yes    | Low        | Yes              | Medium      | Not found         |                                                                                                                         |

rulesets/kotlin/jpinpoint-kotlin-rules.xml

@@ -11,6 +11,58 @@
 <!-- IMPORTANT NOTICE: The content of this file is generated. Do not edit this file directly since changes may be lost when this file is regenerated! -->
 
 <!-- BEGIN Included file 'common.xml' -->
+    <rule name="AvoidDecimalAndChoiceFormatAsField"
+          since="7.0"
+          language="kotlin"
+          message="Avoid using NumberFormat, DecimalFormat or ChoiceFormat as field since it is thread-unsafe."
+          class="net.sourceforge.pmd.lang.rule.XPathRule"
+          typeResolution="true"
+          externalInfoUrl="https://github.com/jborgers/PMD-jPinpoint-rules/tree/master/docs/JavaCodePerformance.md#isio01">
+        <description>
+            Problem: java.text.DecimalFormat and java.text.ChoiceFormat are thread-unsafe.
+            Solution: usual solution is to create a new local one when needed in a method.
+            (jpinpoint-rules)
+        (jpinpoint-kotlin-rules)</description>
+        <priority>1</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.text imports exists as filter for same named classes in other packages :)
+//ImportHeader[.//T-Identifier[@Text='java'] and .//T-Identifier[@Text='text']]/ancestor::KotlinFile
+(: PrimaryExpression finds the constructors instead of type declarations :)
+(: filter matches in functions :)
+//ClassDeclaration//PrimaryExpression//SimpleIdentifier//T-Identifier[
+ (@Text="DecimalFormat" or @Text="ChoiceFormat")
+ and not(ancestor::FunctionDeclaration)
+]
+]]>
+                </value>
+            </property>
+        </properties>
+        <example><![CDATA[
+class Foo {
+
+    companion object {
+        val NUMBER_FORMAT: DecimalFormat = DecimalFormat("###.###") // bad
+    }
+
+    private val numFormat: NumberFormat = DecimalFormat("###.###") //bad
+
+    private val limits = doubleArrayOf(1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0)
+    private val dayOfWeekNames = arrayOf("Sun", "Mon", "Tue", "Wed", "Thur", "Fri", "Sat")
+    private val form = ChoiceFormat(limits, dayOfWeekNames) // bad
+
+    fun shouldNotMatchInsideMethod() {
+        val format: NumberFormat = DecimalFormat("##.##")
+        val choiceFormat = ChoiceFormat(limits, dayOfWeekNames)
+    }
+}
+]]>
+        </example>
+    </rule>
+
     <rule name="AvoidInMemoryStreamingDefaultConstructor"
           since="7.0"
           language="kotlin"
@@ -29,7 +81,7 @@
             <property name="version" value="3.1"/>
             <property name="xpath">
                 <value><![CDATA[
-//ImportHeader[.//T-Identifier[@Text='java'] and .//T-Identifier[@Text='io']]/ancestor::KotlinFile
+//ImportHeader[.//T-Identifier[@Text='java'] and .//T-Identifier[@Text='io']][1]/ancestor::KotlinFile
 //Expression//T-Identifier[(@Text='ByteArrayOutputStream' and ../../../
 PostfixUnarySuffix//ValueArguments[not(ValueArgument) or ValueArgument//T-IntegerLiteral[number(@Text)<=32]])
 or (@Text='StringWriter' and ../../../
@@ -57,6 +109,54 @@ class AvoidInMemoryStreamingDefaultConstructor {
         </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."
+          typeResolution="true"
+          externalInfoUrl="https://github.com/jborgers/PMD-jPinpoint-rules/tree/master/docs/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 (sure? - TODO) .
+            (jpinpoint-rules)</description>
+        <priority>2</priority>
+        <properties>
+            <property name="tag" value="jpinpoint-rule" type="String" description="for-sonar"/>
+            <property name="version" value="2.0"/>
+            <property name="xpath">
+                <value><![CDATA[
+//ImportHeader[.//T-Identifier[@Text='javax'] and .//T-Identifier[@Text='xml'] and .//T-Identifier[@Text='xpath']]/ancestor::KotlinFile
+//FunctionBody//T-Identifier[@Text='newXPath']
+/ancestor::FunctionBody//T-Identifier[@Text='compile'
+or @Text='evaluate' and ancestor::PostfixUnarySuffix/..//ValueArguments[count(./ValueArgument) = 3]]
+(: note on limitation, the 2-argument version is not found, difficult to do the typeIs checking like in Java :)
+]]></value>
+            </property>
+        </properties>
+        <example>
+            <![CDATA[
+import org.w3c.dom.Document
+import org.w3c.dom.NodeList
+import javax.xml.xpath.*
+
+class AvoidRecompilingXPathExpressionKotlin {
+    fun bad1(doc: Document?): NodeList {
+        val xpath: XPath = XPathFactory.newInstance().newXPath()
+        val expr: XPathExpression = xpath.compile("//book[author='Isaac Asimov']/title/text()") // bad
+        return expr.evaluate(doc, XPathConstants.NODESET) as NodeList
+    }
+
+    @Throws(XPathExpressionException::class)
+    fun bad2(doc: Document?): NodeList {
+        val xpath: XPath = XPathFactory.newInstance().newXPath()
+        val xPathQuery = "//book[author='Isaac Asimov']/title/text()"
+        return xpath.evaluate(xPathQuery, doc, XPathConstants.NODESET) as NodeList // bad
+    }
+}
+// TODO Good example
+            ]]>
+        </example>
+    </rule>
+
     <rule name="AvoidStringBuffer" class="net.sourceforge.pmd.lang.rule.XPathRule"
           since="7.0"
           language="kotlin"

src/main/resources/category/kotlin/common.xml

@@ -37,7 +37,7 @@ class AvoidInMemoryStreamingDefaultConstructor {
     fun bad() {
         var baos = ByteArrayOutputStream() //bad
         val sw = StringWriter() //bad
-        baos = ByteArrayOutputStream(32) //bad - not larger than default
+        baos = ByteArrayOutputStream(32) //bad - not larger than default // TODO
     }
     fun good() {
         val baos = ByteArrayOutputStream(8192) // 8 kiB
@@ -99,8 +99,8 @@ class AvoidStringBuffer {
             <property name="version" value="3.1"/>
             <property name="xpath">
                 <value><![CDATA[
-(: check if java.text imports exists as filter for same named classes in other packages :)
-//ImportHeader[.//T-Identifier[@Text='java'] and .//T-Identifier[@Text='text']]/ancestor::KotlinFile
+(: check if at least one java.text imports exists as filter for same named classes in other packages :)
+//ImportHeader[.//T-Identifier[@Text='java'] and .//T-Identifier[@Text='text']][1]/ancestor::KotlinFile
 (: PrimaryExpression finds the constructors instead of type declarations :)
 (: filter matches in functions :)
 //ClassDeclaration//PrimaryExpression//SimpleIdentifier//T-Identifier[
@@ -133,4 +133,71 @@ class Foo {
         </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."
+          typeResolution="true"
+          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="tag" value="jpinpoint-rule" type="String" description="for-sonar"/>
+            <property name="version" value="2.0"/>
+            <property name="xpath">
+                <value><![CDATA[
+//ImportHeader[.//T-Identifier[@Text='javax'] and .//T-Identifier[@Text='xml'] and .//T-Identifier[@Text='xpath']]/ancestor::KotlinFile
+//FunctionBody//T-Identifier[@Text='newXPath']
+/ancestor::FunctionBody//T-Identifier[@Text='compile'
+or (@Text='evaluate' and ancestor::PostfixUnarySuffix/..//ValueArguments[count(./ValueArgument) = 3])]
+(: note on limitation, the 2-argument version is not found, difficult to do the typeIs checking like in Java :)
+]]></value>
+            </property>
+        </properties>
+        <example>
+            <![CDATA[
+import org.w3c.dom.Document
+import org.w3c.dom.NodeList
+import javax.xml.xpath.*
+
+class AvoidRecompilingXPathExpressionKotlin {
+    fun bad1(doc: Document?): NodeList {
+        val xpath: XPath = XPathFactory.newInstance().newXPath()
+        val expr: XPathExpression = xpath.compile("//book[author='Isaac Asimov']/title/text()") // bad
+        return expr.evaluate(doc, XPathConstants.NODESET) as NodeList
+    }
+
+    @Throws(XPathExpressionException::class)
+    fun bad2(doc: Document?): NodeList {
+        val xpath: XPath = XPathFactory.newInstance().newXPath()
+        val xPathQuery = "//book[author='Isaac Asimov']/title/text()"
+        return xpath.evaluate(xPathQuery, doc, XPathConstants.NODESET) as NodeList // bad
+    }
+}
+
+object GoodAvoidRecompilingXPathExpressionKt {
+    private val tlFac = ThreadLocal.withInitial { XPathFactory.newInstance() }
+    private val tlExpr: ThreadLocal<XPathExpression>
+
+    init {
+        val xpath = tlFac.get().newXPath()
+        val expr: XPathExpression = try {
+            xpath.compile("//book[author='Isaac Asimov']/title/text()")
+        } catch (e: XPathExpressionException) {
+            throw RuntimeException(e)
+        }
+        tlExpr = ThreadLocal.withInitial { expr } // good
+    }
+
+    @Throws(XPathExpressionException::class)
+    fun good(doc: Document?): NodeList {
+        return tlExpr!!.get().evaluate(doc, XPathConstants.NODESET) as NodeList // good
+    }
+}
+            ]]>
+        </example>
+    </rule>
+
 </ruleset>

src/test/java/com/jpinpoint/perf/lang/kotlin/ruleset/common/AvoidRecompilingXPathExpressionTest.java

@@ -0,0 +1,6 @@
+package com.jpinpoint.perf.lang.kotlin.ruleset.common;
+
+import net.sourceforge.pmd.testframework.PmdRuleTst;
+
+public class AvoidRecompilingXPathExpressionTest extends PmdRuleTst {
+}

src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidRecompilingXPathExpression.xml

@@ -0,0 +1,52 @@
+<?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>AvoidRecompilingXPathExpression</description>
+        <expected-problems>2</expected-problems>
+        <expected-linenumbers>8, 16</expected-linenumbers>
+        <code><![CDATA[
+import org.w3c.dom.Document
+import org.w3c.dom.NodeList
+import javax.xml.xpath.*
+
+class AvoidRecompilingXPathExpressionKotlin {
+    fun bad1(doc: Document?): NodeList {
+        val xpath: XPath = XPathFactory.newInstance().newXPath()
+        val expr: XPathExpression = xpath.compile("//book[author='Isaac Asimov']/title/text()") // bad
+        return expr.evaluate(doc, XPathConstants.NODESET) as NodeList
+    }
+
+    @Throws(XPathExpressionException::class)
+    fun bad2(doc: Document?): NodeList {
+        val xpath: XPath = XPathFactory.newInstance().newXPath()
+        val xPathQuery = "//book[author='Isaac Asimov']/title/text()"
+        return xpath.evaluate(xPathQuery, doc, XPathConstants.NODESET) as NodeList // bad
+    }
+}
+
+object GoodAvoidRecompilingXPathExpressionKt {
+    private val tlFac = ThreadLocal.withInitial { XPathFactory.newInstance() }
+    private val tlExpr: ThreadLocal<XPathExpression>
+
+    init {
+        val xpath = tlFac.get().newXPath()
+        val expr: XPathExpression = try {
+            xpath.compile("//book[author='Isaac Asimov']/title/text()")
+        } catch (e: XPathExpressionException) {
+            throw RuntimeException(e)
+        }
+        tlExpr = ThreadLocal.withInitial { expr } // good
+    }
+
+    @Throws(XPathExpressionException::class)
+    fun good(doc: Document?): NodeList {
+        return tlExpr!!.get().evaluate(doc, XPathConstants.NODESET) as NodeList // good
+    }
+}
+]]></code>
+    </test-code>
+
+</test-data>