You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gary Gregory <ga...@gmail.com> on 2022/06/01 13:03:04 UTC
[Text] PMD Warnings
Hi,
If you run 'mvn pmd:check' on Commons Text, you'll get:
<?xml version="1.0" encoding="UTF-8"?>
<pmd xmlns="http://pmd.sourceforge.net/report/2.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/report/2.0.0
http://pmd.sourceforge.net/report_2_0_0.xsd" version="6.46.0"
timestamp="2022-06-01T08:42:15.132">
<file name="/Users/garydgregory/git/commons-text/src/main/java/org/apache/commons/text/RandomStringGenerator.java">
<violation beginline="85" endline="85" begincolumn="44" endcolumn="97"
rule="UnnecessaryFullyQualifiedName" ruleset="Code Style"
package="org.apache.commons.text"
class="RandomStringGenerator$Builder"
externalInfoUrl="https://pmd.github.io/pmd-6.46.0/pmd_rules_java_codestyle.html#unnecessaryfullyqualifiedname"
priority="4">
Unnecessary use of fully qualified name
'org.apache.commons.text.Builder' due to existing same package import
'org.apache.commons.text.*'
</violation>
</file>
<file name="/Users/garydgregory/git/commons-text/src/main/java/org/apache/commons/text/StrBuilder.java">
<violation beginline="2033" endline="2033" begincolumn="13"
endcolumn="21" rule="AvoidBranchingStatementAsLastInLoop"
ruleset="Error Prone" package="org.apache.commons.text"
class="StrBuilder" method="indexOf"
externalInfoUrl="https://pmd.github.io/pmd-6.46.0/pmd_rules_java_errorprone.html#avoidbranchingstatementaslastinloop"
priority="2">
Avoid using a branching statement as the last in a loop.
</violation>
<violation beginline="2374" endline="2374" begincolumn="17"
endcolumn="25" rule="AvoidBranchingStatementAsLastInLoop"
ruleset="Error Prone" package="org.apache.commons.text"
class="StrBuilder" method="lastIndexOf"
externalInfoUrl="https://pmd.github.io/pmd-6.46.0/pmd_rules_java_errorprone.html#avoidbranchingstatementaslastinloop"
priority="2">
Avoid using a branching statement as the last in a loop.
</violation>
</file>
<file name="/Users/garydgregory/git/commons-text/src/main/java/org/apache/commons/text/StrLookup.java">
<violation beginline="137" endline="139" begincolumn="19"
endcolumn="17" rule="EmptyCatchBlock" ruleset="Error Prone"
package="org.apache.commons.text"
class="StrLookup$SystemPropertiesStrLookup" method="lookup"
externalInfoUrl="https://pmd.github.io/pmd-6.46.0/pmd_rules_java_errorprone.html#emptycatchblock"
priority="3">
Avoid empty catch blocks
</violation>
</file>
<file name="/Users/garydgregory/git/commons-text/src/main/java/org/apache/commons/text/TextStringBuilder.java">
<violation beginline="2057" endline="2057" begincolumn="13"
endcolumn="21" rule="AvoidBranchingStatementAsLastInLoop"
ruleset="Error Prone" package="org.apache.commons.text"
class="TextStringBuilder" method="indexOf"
externalInfoUrl="https://pmd.github.io/pmd-6.46.0/pmd_rules_java_errorprone.html#avoidbranchingstatementaslastinloop"
priority="2">
Avoid using a branching statement as the last in a loop.
</violation>
<violation beginline="2390" endline="2390" begincolumn="17"
endcolumn="25" rule="AvoidBranchingStatementAsLastInLoop"
ruleset="Error Prone" package="org.apache.commons.text"
class="TextStringBuilder" method="lastIndexOf"
externalInfoUrl="https://pmd.github.io/pmd-6.46.0/pmd_rules_java_errorprone.html#avoidbranchingstatementaslastinloop"
priority="2">
Avoid using a branching statement as the last in a loop.
</violation>
</file>
</pmd>
The first once seems like a bug in PMD, does anyone agree. If you have
a solution for the AvoidBranchingStatementAsLastInLoop issue, I'd
welcome a PR :-)
Gary
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Text] PMD Warnings
Posted by Alex Herbert <al...@gmail.com>.
On Thu, 2 Jun 2022 at 11:07, Gary Gregory <ga...@gmail.com> wrote:
> Albert (and all):
>
> Do you think this is a false positive:
>
> <violation beginline="85" endline="85" begincolumn="44" endcolumn="97"
> rule="UnnecessaryFullyQualifiedName" ruleset="Code Style"
> package="org.apache.commons.text"
> class="RandomStringGenerator$Builder"
> externalInfoUrl="
> https://pmd.github.io/pmd-6.46.0/pmd_rules_java_codestyle.html#unnecessaryfullyqualifiedname
> "
> priority="4">
> Unnecessary use of fully qualified name
> 'org.apache.commons.text.Builder' due to existing same package import
> 'org.apache.commons.text.*'
> </violation>
>
> ?
>
Yes. PMD is not recognising the type erasure on the Builder<T>. You cannot
compile the code if you remove the fully qualified reference to the
Builder<T>.
This requires a warning suppression which requires a non-default
configuration for the PMD plugin. IIRC the alternative is a '// NOPMD'
trailing comment on the offending line which is noise in the source code.
Alex
Re: [Text] PMD Warnings
Posted by Gary Gregory <ga...@gmail.com>.
Albert (and all):
Do you think this is a false positive:
<violation beginline="85" endline="85" begincolumn="44" endcolumn="97"
rule="UnnecessaryFullyQualifiedName" ruleset="Code Style"
package="org.apache.commons.text"
class="RandomStringGenerator$Builder"
externalInfoUrl="https://pmd.github.io/pmd-6.46.0/pmd_rules_java_codestyle.html#unnecessaryfullyqualifiedname"
priority="4">
Unnecessary use of fully qualified name
'org.apache.commons.text.Builder' due to existing same package import
'org.apache.commons.text.*'
</violation>
?
TY,
Gary
On Wed, Jun 1, 2022 at 11:11 AM Alex Herbert <al...@gmail.com> wrote:
>
> On Wed, 1 Jun 2022 at 14:04, Gary Gregory <ga...@gmail.com> wrote:
>
> > Hi,
> >
> > If you run 'mvn pmd:check' on Commons Text, you'll get:
> >
>
> Note: You can suppress the 'Avoid empty catch blocks' by using 'ignored' as
> the exception name. That is an easy fix.
>
>
> > <file
> > name="/Users/garydgregory/git/commons-text/src/main/java/org/apache/commons/text/TextStringBuilder.java">
> > <violation beginline="2057" endline="2057" begincolumn="13"
> > endcolumn="21" rule="AvoidBranchingStatementAsLastInLoop"
> > ruleset="Error Prone" package="org.apache.commons.text"
> > class="TextStringBuilder" method="indexOf"
> > externalInfoUrl="
> > https://pmd.github.io/pmd-6.46.0/pmd_rules_java_errorprone.html#avoidbranchingstatementaslastinloop
> > "
> > priority="2">
> > Avoid using a branching statement as the last in a loop.
> > </violation>
> >
>
> The maven-pmd-plugin is using the default configuration. So it is not easy
> to just drop in a suppression for this.
>
> You can configure the rule to ignore checking for return statements [1].
> I took the default ruleset [2] and changed the following which suppressed
> the violation:
>
> <rule
> ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop">
> <properties>
> <property name="checkReturnLoopTypes" value="" />
> </properties>
> </rule>
>
> You can also suppress this with some xpath. This works and is a finer tool
> than a blunt disabling of the rule:
>
> <rule
> ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop">
> <properties>
> <property name="violationSuppressXPath"
> value="./ancestor-or-self::MethodDeclaration[@Name='indexOf' or
> @Name='lastIndexOf']
>
> /ancestor::ClassOrInterfaceDeclaration[@SimpleName='TextStringBuilder' or
> @SimpleName='StrBuilder']" />
> </properties>
> </rule>
>
> Or you rewrite the code to remove the return as the last statement in the
> loop. This works at one violation site be removing the inner loop to a
> method:
>
> ---
>
> final char[] thisBuf = buffer;
> final int len = size - strLen + 1;
> for (int i = startIndex; i < len; i++) {
> if (match(str, thisBuf, i, strLen)) {
> return i;
> }
> }
> return StringUtils.INDEX_NOT_FOUND;
> }
>
> private static boolean match(String str, char[] buffer, int i, int strLen) {
> for (int j = 0; j < strLen; j++) {
> if (str.charAt(j) != buffer[i + j]) {
> return false;
> }
> }
> return true;
> }
>
> ---
>
> This adds a method call inside the loop and may be less efficient. I prefer
> to add suppressions to the PMD ruleset. But since text is using the default
> ruleset then you have some extra work to do in configuring the POM.
>
> Alex
>
>
>
> [1]
> https://pmd.github.io/pmd-6.46.0/pmd_rules_java_errorprone.html#avoidbranchingstatementaslastinloop
> [2]
> https://gitbox.apache.org/repos/asf?p=maven-pmd-plugin.git;a=blob_plain;f=src/main/resources/rulesets/java/maven-pmd-plugin-default.xml;hb=HEAD
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Text] PMD Warnings
Posted by Gary Gregory <ga...@gmail.com>.
On Wed, Jun 1, 2022 at 11:11 AM Alex Herbert <al...@gmail.com> wrote:
>
> On Wed, 1 Jun 2022 at 14:04, Gary Gregory <ga...@gmail.com> wrote:
>
> > Hi,
> >
> > If you run 'mvn pmd:check' on Commons Text, you'll get:
> >
>
> Note: You can suppress the 'Avoid empty catch blocks' by using 'ignored' as
> the exception name. That is an easy fix.
Good tip! Done.
Gary
>
>
> > <file
> > name="/Users/garydgregory/git/commons-text/src/main/java/org/apache/commons/text/TextStringBuilder.java">
> > <violation beginline="2057" endline="2057" begincolumn="13"
> > endcolumn="21" rule="AvoidBranchingStatementAsLastInLoop"
> > ruleset="Error Prone" package="org.apache.commons.text"
> > class="TextStringBuilder" method="indexOf"
> > externalInfoUrl="
> > https://pmd.github.io/pmd-6.46.0/pmd_rules_java_errorprone.html#avoidbranchingstatementaslastinloop
> > "
> > priority="2">
> > Avoid using a branching statement as the last in a loop.
> > </violation>
> >
>
> The maven-pmd-plugin is using the default configuration. So it is not easy
> to just drop in a suppression for this.
>
> You can configure the rule to ignore checking for return statements [1].
> I took the default ruleset [2] and changed the following which suppressed
> the violation:
>
> <rule
> ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop">
> <properties>
> <property name="checkReturnLoopTypes" value="" />
> </properties>
> </rule>
>
> You can also suppress this with some xpath. This works and is a finer tool
> than a blunt disabling of the rule:
>
> <rule
> ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop">
> <properties>
> <property name="violationSuppressXPath"
> value="./ancestor-or-self::MethodDeclaration[@Name='indexOf' or
> @Name='lastIndexOf']
>
> /ancestor::ClassOrInterfaceDeclaration[@SimpleName='TextStringBuilder' or
> @SimpleName='StrBuilder']" />
> </properties>
> </rule>
>
> Or you rewrite the code to remove the return as the last statement in the
> loop. This works at one violation site be removing the inner loop to a
> method:
>
> ---
>
> final char[] thisBuf = buffer;
> final int len = size - strLen + 1;
> for (int i = startIndex; i < len; i++) {
> if (match(str, thisBuf, i, strLen)) {
> return i;
> }
> }
> return StringUtils.INDEX_NOT_FOUND;
> }
>
> private static boolean match(String str, char[] buffer, int i, int strLen) {
> for (int j = 0; j < strLen; j++) {
> if (str.charAt(j) != buffer[i + j]) {
> return false;
> }
> }
> return true;
> }
>
> ---
>
> This adds a method call inside the loop and may be less efficient. I prefer
> to add suppressions to the PMD ruleset. But since text is using the default
> ruleset then you have some extra work to do in configuring the POM.
>
> Alex
>
>
>
> [1]
> https://pmd.github.io/pmd-6.46.0/pmd_rules_java_errorprone.html#avoidbranchingstatementaslastinloop
> [2]
> https://gitbox.apache.org/repos/asf?p=maven-pmd-plugin.git;a=blob_plain;f=src/main/resources/rulesets/java/maven-pmd-plugin-default.xml;hb=HEAD
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Text] PMD Warnings
Posted by Alex Herbert <al...@gmail.com>.
On Wed, 1 Jun 2022 at 14:04, Gary Gregory <ga...@gmail.com> wrote:
> Hi,
>
> If you run 'mvn pmd:check' on Commons Text, you'll get:
>
Note: You can suppress the 'Avoid empty catch blocks' by using 'ignored' as
the exception name. That is an easy fix.
> <file
> name="/Users/garydgregory/git/commons-text/src/main/java/org/apache/commons/text/TextStringBuilder.java">
> <violation beginline="2057" endline="2057" begincolumn="13"
> endcolumn="21" rule="AvoidBranchingStatementAsLastInLoop"
> ruleset="Error Prone" package="org.apache.commons.text"
> class="TextStringBuilder" method="indexOf"
> externalInfoUrl="
> https://pmd.github.io/pmd-6.46.0/pmd_rules_java_errorprone.html#avoidbranchingstatementaslastinloop
> "
> priority="2">
> Avoid using a branching statement as the last in a loop.
> </violation>
>
The maven-pmd-plugin is using the default configuration. So it is not easy
to just drop in a suppression for this.
You can configure the rule to ignore checking for return statements [1].
I took the default ruleset [2] and changed the following which suppressed
the violation:
<rule
ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop">
<properties>
<property name="checkReturnLoopTypes" value="" />
</properties>
</rule>
You can also suppress this with some xpath. This works and is a finer tool
than a blunt disabling of the rule:
<rule
ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop">
<properties>
<property name="violationSuppressXPath"
value="./ancestor-or-self::MethodDeclaration[@Name='indexOf' or
@Name='lastIndexOf']
/ancestor::ClassOrInterfaceDeclaration[@SimpleName='TextStringBuilder' or
@SimpleName='StrBuilder']" />
</properties>
</rule>
Or you rewrite the code to remove the return as the last statement in the
loop. This works at one violation site be removing the inner loop to a
method:
---
final char[] thisBuf = buffer;
final int len = size - strLen + 1;
for (int i = startIndex; i < len; i++) {
if (match(str, thisBuf, i, strLen)) {
return i;
}
}
return StringUtils.INDEX_NOT_FOUND;
}
private static boolean match(String str, char[] buffer, int i, int strLen) {
for (int j = 0; j < strLen; j++) {
if (str.charAt(j) != buffer[i + j]) {
return false;
}
}
return true;
}
---
This adds a method call inside the loop and may be less efficient. I prefer
to add suppressions to the PMD ruleset. But since text is using the default
ruleset then you have some extra work to do in configuring the POM.
Alex
[1]
https://pmd.github.io/pmd-6.46.0/pmd_rules_java_errorprone.html#avoidbranchingstatementaslastinloop
[2]
https://gitbox.apache.org/repos/asf?p=maven-pmd-plugin.git;a=blob_plain;f=src/main/resources/rulesets/java/maven-pmd-plugin-default.xml;hb=HEAD