phoedos/pmd
1
1 Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #4159 from adangel:AvoidArrayLoops-improvements

Browse Source 
[java] AvoidArrayLoops improvements #4159
This commit is contained in:
Andreas Dangel committed 2022-10-28 09:39:41 +02:00
2 parents af013d61e1 a9f00ecadb
commit 59cfe99325
3 files changed
+188 -25
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

docs/pages/release_notes.md
+3
View File
@@ -34,9 +34,12 @@ The rule is part of the quickstart.xml ruleset.
* java-documentation
* [#4141](https://github.com/pmd/pmd/issues/4141): \[java] UncommentedEmptyConstructor FP when constructor annotated with @<!-- -->Autowired
* java-performance
* [#1167](https://github.com/pmd/pmd/issues/1167): \[java] AvoidArrayLoops false positive on double assignment
* [#2080](https://github.com/pmd/pmd/issues/2080): \[java] StringToString rule false-positive with field access
* [#2692](https://github.com/pmd/pmd/issues/2692): \[java] \[doc] AvoidArrayLoops flags copy assignment in same array as sub-optimal
* [#3437](https://github.com/pmd/pmd/issues/3437): \[java] StringToString doesn't trigger on Bar.class.getSimpleName().toString()
* [#3681](https://github.com/pmd/pmd/issues/3681): \[java] StringToString doesn't trigger on string literals
* [#3847](https://github.com/pmd/pmd/issues/3847): \[java] AvoidArrayLoops should consider final variables
* [#3977](https://github.com/pmd/pmd/issues/3977): \[java] StringToString false-positive with local method name confusion
* [#4091](https://github.com/pmd/pmd/issues/4091): \[java] AvoidArrayLoops false negative with do-while loops
* [#4148](https://github.com/pmd/pmd/issues/4148): \[java] UseArrayListInsteadOfVector ignores Vector when other classes are imported
pmd-java/src/main/resources/category/java/performance.xml
+57 -24
View File
@@ -72,7 +72,12 @@ sb.append('a'); // use this instead
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_performance.html#avoidarrayloops">
<description>
Instead of manually copying data between two arrays, use the efficient Arrays.copyOf or System.arraycopy method instead.
Instead of manually copying data between two arrays, use the more efficient `Arrays.copyOf`
or `System.arraycopy` method instead.
To copy only part of the array, use `Arrays.copyOfRange` or `System.arraycopy`.
If you want to copy/move elements inside the _same_ array (e.g. shift the elements), use `System.arraycopy`.
</description>
<priority>3</priority>
<properties>
@@ -80,43 +85,71 @@ Instead of manually copying data between two arrays, use the efficient Arrays.co
<property name="xpath">
<value>
<![CDATA[
//Statement[(ForStatement or WhileStatement or DoStatement) and
count(*//AssignmentOperator[@Image = '='])=1
and
*/Statement
[
./Block/BlockStatement/Statement/StatementExpression/PrimaryExpression
/PrimaryPrefix/Name/../../PrimarySuffix/Expression
[(PrimaryExpression or AdditiveExpression) and count
(.//PrimaryPrefix/Name)=1]//PrimaryPrefix/Name/@Image
and
./Block/BlockStatement/Statement/StatementExpression/Expression/PrimaryExpression
/PrimaryPrefix/Name/../../PrimarySuffix[count
(..//PrimarySuffix)=1]/Expression[(PrimaryExpression
or AdditiveExpression) and count(.//PrimaryPrefix/Name)=1]
//PrimaryPrefix/Name/@Image
]]
//Statement
[(ForStatement or WhileStatement or DoStatement)]
[count(*/Statement//AssignmentOperator[@Image = '='])=1]
[*/Statement/Block/BlockStatement/Statement
/StatementExpression[AssignmentOperator[@Image = '=']]
(: LHS - left hand side :)
[PrimaryExpression[count(PrimarySuffix) = 1]/PrimarySuffix[@ArrayDereference = true()]
/Expression[PrimaryExpression or AdditiveExpression]
[count(.//PrimaryPrefix/Name) = 1 or
(count(.//PrimaryPrefix/Name) = 2 and .//PrimaryPrefix/Name/@Image = //VariableDeclaratorId[@Final = true()]/@Name)
]
//PrimaryPrefix/Name/@Image]
(: RHS - right hand side :)
[Expression/PrimaryExpression/PrimarySuffix[@ArrayDereference = true()][count(..//PrimarySuffix) = 1]
/Expression[PrimaryExpression or AdditiveExpression]
[count(.//PrimaryPrefix/Name) = 1 or
(count(.//PrimaryPrefix/Name) = 2 and .//PrimaryPrefix/Name/@Image = //VariableDeclaratorId[@Final = true()]/@Name)
]
//PrimaryPrefix/Name/@Image]
]
]]>
</value>
</property>
</properties>
<example>
<![CDATA[
public class Test {
public void bar() {
class Scratch {
void copy_a_to_b() {
int[] a = new int[10];
int[] b = new int[10];
for (int i=0;i<10;i++) {
b[i]=a[i];
for (int i = 0; i < a.length; i++) {
b[i] = a[i];
}
// equivalent
b = Arrays.copyOf(a, a.length);
// equivalent
System.arraycopy(a, 0, b, 0, a.length);
int[] c = new int[10];
// this will trigger the rule
for (int i=0;i<10;i++) {
b[i]=a[c[i]];
// this will not trigger the rule
for (int i = 0; i < c.length; i++) {
b[i] = a[c[i]];
}
}
}
]]>
</example>
<example>
<![CDATA[
class Scratch {
void shift_left(int[] a) {
for (int i = 0; i < a.length - 1; i++) {
a[i] = a[i + 1];
}
// equivalent
System.arraycopy(a, 1, a, 0, a.length - 1);
}
void shift_right(int[] a) {
for (int i = a.length - 1; i > 0; i--) {
a[i] = a[i - 1];
}
// equivalent
System.arraycopy(a, 0, a, 1, a.length - 1);
}
}
]]>
</example>
</rule>
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidArrayLoops.xml
+128 -1
View File
@@ -126,12 +126,139 @@ public class Foo {
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public void bar() {
public void bar1() {
for (int i = 0; i < 10; i++) {
a[i] += b[i];
}
}
public void bar2() {
int i = 0;
for (i = 0; i < 10; i++) {
a[i] += b[i];
}
}
}
]]></code>
</test-code>
<test-code>
<description>[java] AvoidArrayLoops false positive on double assignment #1167</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Test {
public static void main( String[] args ) {
double[] foo = new double[100];
double bar = 0.0;
int[] exps = new int[10];
for (int i = 0; i < exps.length; i++) {
double value = Math.random();
foo[i] = Math.exp(value);
bar += foo[i];
}
}
}
]]></code>
</test-code>
<test-code>
<description>Nested array on RHS</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
class AvoidArrayLoops {
void copy_a_to_b(int[] a, int[] b) {
int[] c = new int[10];
// this will not trigger the rule
for (int i = 0; i < 10; i++) {
b[i] = a[c[i]];
}
}
}
]]></code>
</test-code>
<test-code>
<description>Ignore multi-dim array assignment #1167</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.Random;
class AvoidArrayLoops {
void bar1(int[][] target, int[] source) {
for (int i = 0; i < 10; i++) {
for(int j = 0; j < 10; j++) {
target[i][j] = source[i * 10 + j];
}
}
}
void bar2(int[][] target, int[] source) {
for (int i = 0; i < 10; i++) {
for(int j = 0; j < 10; j++) {
int sourceIndex = new Random().nextInt(source.length);
target[i][j] = source[sourceIndex];
}
}
}
}
]]></code>
</test-code>
<test-code>
<description>[java] AvoidArrayLoops should consider final variables #3847</description>
<expected-problems>3</expected-problems>
<expected-linenumbers>6,10,13</expected-linenumbers>
<code><![CDATA[
class AvoidArrayLoops {
void sample() {
int[] a = new int[10];
int[] b = new int[10];
final int c = 6;
for (int i = 0; i < 10; i++) {
b[i] = a[i + c]; // should report a warning at this line
// b[i] = a[i + 6]; // This line can be detected
}
for (int i = 0; i < 10; i++) {
b[i + c] = a[i]; // should report a warning at this line
}
for (int i = 0; i < 10; i++) {
b[i + c] = a[i + c]; // should report a warning at this line
}
}
}
]]></code>
</test-code>
<test-code>
<description>Shifting left and right #2692</description>
<expected-problems>2</expected-problems>
<expected-linenumbers>9,19</expected-linenumbers>
<code><![CDATA[
class Scratch {
public static void main(String[] args) {
int[] ints;
ints = new int[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
System.out.println(Arrays.toString(ints)); // [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
// shift left
ints = new int[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
for (int i = 0; i < ints.length - 1; i++) {
ints[i] = ints[i + 1];
}
System.out.println(Arrays.toString(ints)); // [2, 3, 4, 5, 6, 7, 8, 9, 10, 10]
ints = new int[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
System.arraycopy(ints, 1, ints, 0, ints.length - 1);
System.out.println(Arrays.toString(ints)); // [2, 3, 4, 5, 6, 7, 8, 9, 10, 10]
// shift right
ints = new int[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
for (int i = ints.length - 1; i > 0; i--) {
ints[i] = ints[i - 1];
}
System.out.println(Arrays.toString(ints)); // [1, 1, 2, 3, 4, 5, 6, 7, 8, 9]
ints = new int[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
System.arraycopy(ints, 0, ints, 1, ints.length - 1);
System.out.println(Arrays.toString(ints)); // [1, 1, 2, 3, 4, 5, 6, 7, 8, 9]
}
}
]]></code>
</test-code>
</test-data>