Skip to content

Commit

Permalink
Migrate java rules to pmd 7.x.x #309: 1 complex rule migrated, number…
Browse files Browse the repository at this point in the history
… 5: AvoidImplicitlyRecompilingRegex
  • Loading branch information
jborgers committed Jul 26, 2024
1 parent 4655806 commit 96c3091
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 69 deletions.
4 changes: 2 additions & 2 deletions src/main-todo/resources/category/java/common.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"
Expand Down
91 changes: 28 additions & 63 deletions src/main/resources/category/java/common.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -303,7 +303,7 @@ class Foo {
.stream()
.findFirst()
.toString()
.replaceAll("\\s+", "-"); // missing violation
.replaceAll("\\s+", "-"); // bad, missing violation
}
}
]]></code>
Expand Down

0 comments on commit 96c3091

Please sign in to comment.