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

Merge pull request #3345 from

Browse Source 
adangel:issue-3331-use-arrays-as-list-foreach

[java] UseArraysAsList false negative with for-each loop #3345
This commit is contained in:
Andreas Dangel
2021-06-24 11:37:08 +02:00
parent 6b29f89349 63971f0fb8
commit 5dd45d09f0
3 changed files with 84 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

70
pmd-java/src/main/resources/category/java/performance.xml
View File

@ -932,13 +932,14 @@ public class SimpleTest extends TestCase {
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_performance.html#usearraysaslist">
<description>
<![CDATA[
The java.util.Arrays class has a "asList" method that should be used when you want to create a new List from
The `java.util.Arrays` class has a `asList()` method that should be used when you want to create a new List from
an array of objects. It is faster than executing a loop to copy all the elements of the array one by one.
Note that the result of Arrays.asList() is backed by the specified array,
Note that the result of `Arrays.asList()` is backed by the specified array,
changes in the returned list will result in the array to be modified.
For that reason, it is not possible to add new elements to the returned list of Arrays.asList() (UnsupportedOperationException).
You must use new ArrayList<>(Arrays.asList(...)) if that is inconvenient for you (e.g. because of concurrent access).
For that reason, it is not possible to add new elements to the returned list of `Arrays.asList()`
(UnsupportedOperationException).
You must use `new ArrayList<>(Arrays.asList(...))` if that is inconvenient for you (e.g. because of concurrent access).
]]>
</description>
<priority>3</priority>
@ -947,35 +948,40 @@ You must use new ArrayList<>(Arrays.asList(...)) if that is inconvenient for you
<property name="xpath">
<value>
<![CDATA[
//Statement[
(ForStatement) and (ForStatement//VariableInitializer//Literal[@IntLiteral= true() and @Image='0']) and (not(.//IfStatement))
]
//StatementExpression[
PrimaryExpression/PrimaryPrefix/Name[
substring-before(@Image,'.add') = ancestor::MethodDeclaration//LocalVariableDeclaration[
./Type//ClassOrInterfaceType[
@Image = 'Collection' or
@Image = 'List' or @Image='ArrayList'
//Statement/ForStatement
[not(.//IfStatement)]
[@Foreach = true() or ForInit//VariableInitializer//Literal[@IntLiteral= true() and @Image='0']]
//StatementExpression
[PrimaryExpression
[PrimaryPrefix/Name
(: method call foo.add(), where foo is initialized as a new ArrayList :)
[ends-with(@Image, '.add')]
[substring-before(@Image, '.add') = ancestor::MethodDeclaration[1]//VariableDeclarator
[VariableInitializer/Expression/PrimaryExpression/PrimaryPrefix/AllocationExpression[pmd-java:typeIs('java.util.ArrayList')]]
/@Name]
]
[PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression
(: checking arguments of method call foo.add() :)
[
(: foreach loop var :)
PrimaryPrefix/Name/@Image = ancestor::ForStatement[1][@Foreach = true()]/LocalVariableDeclaration/VariableDeclarator/@Name
and (
(: foreach loop over local var which is an array :)
ancestor::ForStatement[1][@Foreach = true()]/Expression/PrimaryExpression/PrimaryPrefix/Name/@Image = ancestor::MethodDeclaration[1]//LocalVariableDeclaration/VariableDeclarator/VariableDeclaratorId[@ArrayType=true()]/@Name
(: foreach loop over method parameter which is an array :)
or ancestor::ForStatement[1][@Foreach = true()]/Expression/PrimaryExpression/PrimaryPrefix/Name/@Image = ancestor::MethodDeclaration[1]//FormalParameter/VariableDeclaratorId[@ArrayType=true()]/@Name
)
or
(: local var which is an array :)
PrimaryPrefix/Name/@Image = ancestor::MethodDeclaration[1]//LocalVariableDeclaration/VariableDeclarator/VariableDeclaratorId[@ArrayType=true()]/@Name
and PrimarySuffix/Expression/PrimaryExpression/PrimaryPrefix/Name
or
(: method parameter which is an array :)
PrimaryPrefix/Name/@Image = ancestor::MethodDeclaration[1]//FormalParameter/VariableDeclaratorId[@ArrayType=true()]/@Name
and PrimarySuffix/Expression/PrimaryExpression/PrimaryPrefix/Name
]
]
]
]
/VariableDeclarator/VariableDeclaratorId[
count(..//AllocationExpression/ClassOrInterfaceType[
@Image="ArrayList"
]
)=1
]/@Name
]
and
PrimaryExpression/PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Name
[
@Image = ancestor::MethodDeclaration[1]//LocalVariableDeclaration/VariableDeclarator/VariableDeclaratorId[@ArrayType=true()]/@Name
or
@Image = ancestor::MethodDeclaration[1]//FormalParameter/VariableDeclaratorId/@Name
]
/../..[count(.//PrimarySuffix)
=1]/PrimarySuffix/Expression/PrimaryExpression/PrimaryPrefix
/Name
]
]]>
</value>
</property>

47
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/UseArraysAsList.xml
View File

@ -7,7 +7,11 @@
<test-code>
<description>failure case</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>9</expected-linenumbers>
<code><![CDATA[
import java.util.ArrayList;
import java.util.List;
public class Bar {
void foo() {
Integer[] ints = new Integer(10);
@ -24,6 +28,9 @@ public class Bar {
<description>adding first element repeatedly</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.ArrayList;
import java.util.List;
public class Bar {
void foo() {
Integer[] ints = new Integer(10);
@ -40,6 +47,9 @@ public class Bar {
<description>inside conditional</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.ArrayList;
import java.util.List;
public class Bar {
void foo() {
Integer[] ints = new Integer(10);
@ -58,6 +68,9 @@ public class Bar {
<description>adding new object</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.ArrayList;
import java.util.List;
public class Bar {
void foo() {
Integer[] ints = new Integer(10);
@ -74,6 +87,9 @@ public class Bar {
<description>calling method</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.ArrayList;
import java.util.List;
public class Bar {
void foo() {
Integer[] ints = new Integer(10);
@ -89,16 +105,20 @@ public class Bar {
<test-code>
<description>Integer array passed as argument</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>9</expected-linenumbers>
<code><![CDATA[
import java.util.ArrayList;
import java.util.List;
public class Test {
public void foo(Integer[] ints) {
// could just use Arrays.asList(ints)
List l = new ArrayList(10);
for (int i = 0; i < 100; i++) {
l.add(ints[i]);
l.add(ints[i]);
}
for (int i = 0; i < 100; i++) {
l.add(a[i].toString()); // won't trigger the rule
l.add(a[i].toString()); // won't trigger the rule
}
}
}
@ -109,6 +129,9 @@ public class Test {
<description>#1099 UseArraysAsList false positives</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.ArrayList;
import java.util.List;
public class Test {
public void foo() {
String [] result = new String[10000];
@ -120,6 +143,26 @@ public class Test {
resultList.add(result[i]);
}
}
}
]]></code>
</test-code>
<test-code>
<description>[java] UseArraysAsList false negative with for-each loop #3331</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>8</expected-linenumbers>
<code><![CDATA[
import java.util.ArrayList;
import java.util.List;
public class UseArraysAsListFN {
public List<String> convert(String[] arr) {
List<String> result = new ArrayList<>();
for (String s : arr) {
result.add(s); // violation expected
}
return result;
}
}
]]></code>
</test-code>