New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AvoidRecompilingXPathExpression for Kotlin #295
Changes from 4 commits
de508ae
cc5687a
3d3e39d
7da8d59
7824328
1ed00dd
99198af
e921a27
58c759f
7452395
74d572f
aca86ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
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() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we also have a rule to flag use of XPath.newInstance() each time? Also preferred to move to ThreadLocal? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No we don't have it at the moment. Might be useful. |
||
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" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -133,4 +133,52 @@ 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. | ||
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"/> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also move to 3.1? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also remove version attribute |
||
<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> | ||
|
||
</ruleset> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
package com.jpinpoint.perf.lang.kotlin.ruleset.common; | ||
|
||
import net.sourceforge.pmd.testframework.PmdRuleTst; | ||
|
||
public class AvoidRecompilingXPathExpressionTest extends PmdRuleTst { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
<?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 | ||
} | ||
} | ||
// TODO Good example | ||
]]></code> | ||
</test-code> | ||
|
||
</test-data> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use 3.1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I think it is the default. PMD7 doc even says:
PMD uses XPath 3.1 for its XPath rules since PMD 7. Before then, the default version was XPath 1.0, with opt-in support for XPath 2.0.
The property version of XPathRule is deprecated and has been removed with PMD 7.