Skip to content

Commit 71f1ca4

Browse files
tomasz-tylenda-sonarsourcetomasz-tylenda-sonarsource
authored
SONARJAVA-2511 S4030 Collection contents should be used (#5203)
1 parent f4f7017 commit 71f1ca4

10 files changed

Lines changed: 336 additions & 0 deletions

File tree

‎its/autoscan/src/test/resources/autoscan/diffs/diff_S4030.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"ruleKey": "S4030",
3+
"hasTruePositives": true,
4+
"falseNegatives": 0,
5+
"falsePositives": 0
6+
}

‎its/ruling/src/test/resources/commons-beanutils/java-S4030.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/LazyDynaListTestCase.java": [
3+
229
4+
]
5+
}

‎its/ruling/src/test/resources/eclipse-jetty/java-S4030.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/handler/NcsaRequestLogTest.java": [
3+
485
4+
]
5+
}

‎its/ruling/src/test/resources/sonar-server/java-S4030.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/computation/task/projectanalysis/step/PersistTestsStep.java": [
3+
234
4+
]
5+
}

‎java-checks-test-sources/default/src/main/java/checks/unused/UnusedCollectionCheckSample.java‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
package checks.unused;
2+
3+
import java.util.List;
4+
import java.util.ArrayList;
5+
import java.util.function.Supplier;
6+
7+
public class UnusedCollectionCheckSample {
8+
int getLengthNonComp(String a, String b, String c) {
9+
List<String> strings = new ArrayList<>(); // Noncompliant {{Consume or remove this unused collection}}
10+
strings.add(a);
11+
strings.add(b);
12+
strings.add(c);
13+
return a.length() + b.length() + c.length();
14+
}
15+
16+
int getLengthComp(String a, String b, String c) {
17+
return a.length() + b.length() + c.length();
18+
}
19+
20+
int getLengthCompTested(String a, String b, String c) {
21+
List<String> strings = new ArrayList<>();
22+
strings.add(a);
23+
strings.add(b);
24+
strings.add(c);
25+
return strings.isEmpty() ? -1 : a.length() + b.length() + c.length();
26+
}
27+
28+
int getLengthCompForeach(String a, String b, String c) {
29+
List<String> strings = new ArrayList<>();
30+
strings.add(a);
31+
strings.add(b);
32+
strings.add(c);
33+
34+
for (String s : strings) {
35+
System.out.println(s);
36+
}
37+
38+
return a.length() + b.length() + c.length();
39+
}
40+
41+
int getLengthCompStream(String a, String b, String c) {
42+
List<String> strings = new ArrayList<>();
43+
strings.add(a);
44+
strings.add(b);
45+
strings.add(c);
46+
47+
strings.stream().forEach(System.out::println);
48+
49+
return a.length() + b.length() + c.length();
50+
}
51+
52+
int getLengthSuppliedCollection(String a, String b, String c, Supplier<List<String>> supplier) {
53+
List<String> strings = supplier.get();
54+
strings.add(a);
55+
strings.add(b);
56+
strings.add(c);
57+
return a.length() + b.length() + c.length();
58+
}
59+
60+
int getLengthAddResultUsed(String a, String b, String c) {
61+
List<String> strings = new ArrayList<>();
62+
strings.add(a);
63+
strings.add(b);
64+
// Add returns value indicating if the collection was changed.
65+
if(strings.add(c)) {
66+
return a.length() + b.length() + c.length();
67+
}
68+
return -1;
69+
}
70+
71+
int getLengthMoreMethods(String a, String b, String c) {
72+
List<String> strings = new ArrayList<>(); // Noncompliant {{Consume or remove this unused collection}}
73+
// ^^^^^^^
74+
strings.add(a);
75+
strings.addAll(List.of("x", "y"));
76+
strings.remove(b);
77+
strings.removeAll(List.of("z", "w"));
78+
strings.retainAll(List.of("c", "d"));
79+
strings.removeIf(s -> s.length() == 3);
80+
strings.clear();
81+
82+
return a.length() + b.length() + c.length();
83+
}
84+
85+
int unrelated(String a, String b, String c) {
86+
StringBuilder strings = new StringBuilder();
87+
strings.append(a);
88+
strings.append(b);
89+
strings.append(c);
90+
return a.length() + b.length() + c.length();
91+
}
92+
93+
static class LengthAccumulator {
94+
int total;
95+
// FN: we do not consider fields, but ideally we would like to support them as well.
96+
List<String> strings;
97+
98+
public LengthAccumulator() {
99+
this.total = 0;
100+
this.strings = new ArrayList<>();
101+
}
102+
103+
public void add(String s) {
104+
total += s.length();
105+
strings.add(s);
106+
}
107+
108+
public int getTotal() {
109+
return total;
110+
}
111+
}
112+
113+
static class MyContainer<T> {
114+
T obj;
115+
int count;
116+
117+
public MyContainer() {
118+
this.obj = null;
119+
this.count = 0;
120+
}
121+
122+
public void add(T obj) {
123+
this.obj = obj;
124+
this.count++;
125+
}
126+
127+
public boolean isEven() {
128+
return count % 2 == 0;
129+
}
130+
}
131+
132+
public void weird() {
133+
MyContainer<String> myContainer = new MyContainer<>();
134+
myContainer.add("a");
135+
myContainer.add("b");
136+
}
137+
}

‎java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks.unused;
18+
19+
import java.util.List;
20+
import java.util.Optional;
21+
import java.util.Set;
22+
import javax.annotation.Nullable;
23+
import org.sonar.check.Rule;
24+
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
25+
import org.sonar.plugins.java.api.semantic.Symbol;
26+
import org.sonar.plugins.java.api.tree.BlockTree;
27+
import org.sonar.plugins.java.api.tree.ExpressionStatementTree;
28+
import org.sonar.plugins.java.api.tree.ExpressionTree;
29+
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
30+
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
31+
import org.sonar.plugins.java.api.tree.NewClassTree;
32+
import org.sonar.plugins.java.api.tree.Tree;
33+
import org.sonar.plugins.java.api.tree.VariableTree;
34+
35+
/**
36+
* Check for collections that are new read, that is the only operations are adding and removing elements.
37+
*/
38+
@Rule(key = "S4030")
39+
public class UnusedCollectionCheck extends IssuableSubscriptionVisitor {
40+
private static final Set<String> MUTATE_METHODS = Set.of("add", "addAll", "clear", "remove", "removeAll", "removeIf", "retainAll");
41+
42+
@Override
43+
public List<Tree.Kind> nodesToVisit() {
44+
return List.of(Tree.Kind.VARIABLE);
45+
}
46+
47+
@Override
48+
public void visitNode(Tree tree) {
49+
if (tree instanceof VariableTree variableTree) {
50+
Symbol symbol = variableTree.symbol();
51+
52+
if (symbol.type().isSubtypeOf("java.util.Collection") &&
53+
symbol.isLocalVariable() &&
54+
isInitializedByConstructor(variableTree.initializer()) &&
55+
symbol.usages().stream().allMatch(UnusedCollectionCheck::isWriteOnly)) {
56+
reportIssue(variableTree.simpleName(), "Consume or remove this unused collection");
57+
}
58+
}
59+
}
60+
61+
/**
62+
* Verify that the initializer is a call to a constructor, for example `new ArrayList()`,
63+
* and not a method call or missing, which happens when the variable is a parameter.
64+
*/
65+
private static boolean isInitializedByConstructor(@Nullable ExpressionTree initializer) {
66+
return initializer instanceof NewClassTree;
67+
}
68+
69+
/**
70+
* Check that we have a call to one of the mutate methods and the return value is ignored.
71+
*/
72+
private static boolean isWriteOnly(Tree tree) {
73+
return Optional.ofNullable(tree)
74+
.map(Tree::parent)
75+
.filter(MemberSelectExpressionTree.class::isInstance)
76+
.map(MemberSelectExpressionTree.class::cast)
77+
.filter(memberSelectExpressionTree -> MUTATE_METHODS.contains(memberSelectExpressionTree.identifier().name()))
78+
.map(Tree::parent)
79+
.filter(MethodInvocationTree.class::isInstance)
80+
.map(Tree::parent)
81+
.filter(ExpressionStatementTree.class::isInstance)
82+
.map(Tree::parent)
83+
.filter(BlockTree.class::isInstance)
84+
.isPresent();
85+
}
86+
}

‎java-checks/src/test/java/org/sonar/java/checks/unused/UnusedCollectionCheckTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks.unused;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.java.checks.verifier.CheckVerifier;
21+
22+
import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
23+
24+
class UnusedCollectionCheckTest {
25+
@Test
26+
void test() {
27+
CheckVerifier.newVerifier()
28+
.onFile(mainCodeSourcesPath("checks/unused/UnusedCollectionCheckSample.java"))
29+
.withCheck(new UnusedCollectionCheck())
30+
.verifyIssues();
31+
}
32+
33+
@Test
34+
void test_without_semantics() {
35+
CheckVerifier.newVerifier()
36+
.onFile(mainCodeSourcesPath("checks/unused/UnusedCollectionCheckSample.java"))
37+
.withCheck(new UnusedCollectionCheck())
38+
.withoutSemantic()
39+
.verifyIssues();
40+
}
41+
}

‎sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S4030.html‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<h2>Why is this an issue?</h2>
2+
<p>If a collection is declared and populated but its values are never read anywhere in the code, it can be considered unused. This often arises from
3+
incomplete refactoring, copy-pasting errors, or typos. Unused collections can lead to wasted memory and degraded application performance.
4+
Additionally, their presence makes the code harder to read and understand.</p>
5+
<h2>How to fix it</h2>
6+
<p>Remove unused collection.</p>
7+
<h3>Code examples</h3>
8+
<h4>Noncompliant code example</h4>
9+
<pre data-diff-id="1" data-diff-type="noncompliant">
10+
int getLength(String a, String b, String c) {
11+
List&lt;String&gt; strings = new ArrayList&lt;&gt;(); // Noncompliant: List is declared and populated but never read.
12+
strings.add(a);
13+
strings.add(b);
14+
strings.add(c);
15+
16+
return a.length() + b.length() + c.length();
17+
}
18+
</pre>
19+
<h4>Compliant solution</h4>
20+
<pre data-diff-id="1" data-diff-type="compliant">
21+
int getLength(String a, String b, String c) {
22+
return a.length() + b.length() + c.length();
23+
}
24+
</pre>
25+

‎sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S4030.json‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
{
2+
"title": "Collection contents should be used",
3+
"type": "CODE_SMELL",
4+
"code": {
5+
"impacts": {
6+
"MAINTAINABILITY": "MEDIUM"
7+
},
8+
"attribute": "CLEAR"
9+
},
10+
"status": "ready",
11+
"remediation": {
12+
"func": "Constant\/Issue",
13+
"constantCost": "2min"
14+
},
15+
"tags": [
16+
"unused",
17+
"suspicious",
18+
"performance"
19+
],
20+
"defaultSeverity": "Minor",
21+
"ruleSpecification": "RSPEC-4030",
22+
"sqKey": "S4030",
23+
"scope": "All",
24+
"quickfix": "targeted"
25+
}

‎sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,7 @@
289289
"S3984",
290290
"S3985",
291291
"S3986",
292+
"S4030",
292293
"S4032",
293294
"S4034",
294295
"S4036",

0 commit comments

Comments
 (0)