From 46a38de87baa8eb2b9a4fc6df780f7bbe219abcf Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Mon, 23 Jun 2025 13:26:43 +0200 Subject: [PATCH 1/5] S4030: Do not raise if there is not usage at all --- .../java/checks/unused/UnusedCollectionCheckSample.java | 7 +++++++ .../sonar/java/checks/unused/UnusedCollectionCheck.java | 1 + 2 files changed, 8 insertions(+) diff --git a/java-checks-test-sources/default/src/main/java/checks/unused/UnusedCollectionCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/unused/UnusedCollectionCheckSample.java index eda07ce2e9f..4bd23e676b7 100644 --- a/java-checks-test-sources/default/src/main/java/checks/unused/UnusedCollectionCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/unused/UnusedCollectionCheckSample.java @@ -17,6 +17,13 @@ int getLengthComp(String a, String b, String c) { return a.length() + b.length() + c.length(); } + int getLengthNoUsageAtAll(String a, String b, String c) { + // Do not report in this case: lack of usage may indicate unreliable data about the symbol + // and this case should be another rule anyway. + List strings = new ArrayList<>(); + return a.length() + b.length() + c.length(); + } + int getLengthCompTested(String a, String b, String c) { List strings = new ArrayList<>(); strings.add(a); diff --git a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java index f90c7e518e7..37c473daa6e 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java @@ -52,6 +52,7 @@ public void visitNode(Tree tree) { if (symbol.type().isSubtypeOf("java.util.Collection") && symbol.isLocalVariable() && isInitializedByConstructor(variableTree.initializer()) && + !symbol.usages().isEmpty() && // no usage could be caused by unreliable data, so do not raise symbol.usages().stream().allMatch(UnusedCollectionCheck::isWriteOnly)) { reportIssue(variableTree.simpleName(), "Consume or remove this unused collection"); } From ac445dca03e1c1bb1371d90aaa7ca09960e34cbd Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Mon, 23 Jun 2025 13:39:50 +0200 Subject: [PATCH 2/5] SQ --- .../org/sonar/java/checks/unused/UnusedCollectionCheck.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java index 37c473daa6e..23794247d60 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java @@ -52,7 +52,9 @@ public void visitNode(Tree tree) { if (symbol.type().isSubtypeOf("java.util.Collection") && symbol.isLocalVariable() && isInitializedByConstructor(variableTree.initializer()) && - !symbol.usages().isEmpty() && // no usage could be caused by unreliable data, so do not raise + // Do not raise without any usage: usages() may be unreliable, + // and it should be handled as a regular unused variable (different rule). + !symbol.usages().isEmpty() && symbol.usages().stream().allMatch(UnusedCollectionCheck::isWriteOnly)) { reportIssue(variableTree.simpleName(), "Consume or remove this unused collection"); } From 59196727efd9fe6a84efdbbc183666dec442589e Mon Sep 17 00:00:00 2001 From: tomasz-tylenda-sonarsource Date: Mon, 23 Jun 2025 14:42:44 +0200 Subject: [PATCH 3/5] Update java-checks-test-sources/default/src/main/java/checks/unused/UnusedCollectionCheckSample.java Co-authored-by: Dorian Burihabwa <75226315+dorian-burihabwa-sonarsource@users.noreply.github.com> --- .../main/java/checks/unused/UnusedCollectionCheckSample.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/unused/UnusedCollectionCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/unused/UnusedCollectionCheckSample.java index 4bd23e676b7..8425c193b3e 100644 --- a/java-checks-test-sources/default/src/main/java/checks/unused/UnusedCollectionCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/unused/UnusedCollectionCheckSample.java @@ -19,7 +19,7 @@ int getLengthComp(String a, String b, String c) { int getLengthNoUsageAtAll(String a, String b, String c) { // Do not report in this case: lack of usage may indicate unreliable data about the symbol - // and this case should be another rule anyway. + // and this case should be covered by S1481 and S1854. List strings = new ArrayList<>(); return a.length() + b.length() + c.length(); } From c05cc94ac7d0787d8428a1b877f047d15636c2f7 Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Mon, 23 Jun 2025 14:46:42 +0200 Subject: [PATCH 4/5] Improve comments --- .../main/java/checks/unused/UnusedCollectionCheckSample.java | 2 +- .../org/sonar/java/checks/unused/UnusedCollectionCheck.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/unused/UnusedCollectionCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/unused/UnusedCollectionCheckSample.java index 8425c193b3e..c0be62fdf92 100644 --- a/java-checks-test-sources/default/src/main/java/checks/unused/UnusedCollectionCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/unused/UnusedCollectionCheckSample.java @@ -19,7 +19,7 @@ int getLengthComp(String a, String b, String c) { int getLengthNoUsageAtAll(String a, String b, String c) { // Do not report in this case: lack of usage may indicate unreliable data about the symbol - // and this case should be covered by S1481 and S1854. + // and this case should also be covered by S1481 and S1854. List strings = new ArrayList<>(); return a.length() + b.length() + c.length(); } diff --git a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java index 23794247d60..3926ea72fb6 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java @@ -53,7 +53,7 @@ public void visitNode(Tree tree) { symbol.isLocalVariable() && isInitializedByConstructor(variableTree.initializer()) && // Do not raise without any usage: usages() may be unreliable, - // and it should be handled as a regular unused variable (different rule). + // moreover, this case is in scope of S1481 and S1854 (unused variable and assignment) !symbol.usages().isEmpty() && symbol.usages().stream().allMatch(UnusedCollectionCheck::isWriteOnly)) { reportIssue(variableTree.simpleName(), "Consume or remove this unused collection"); From 162a804c8527f3ad1214d25b9e03b9c6d3b1e9e1 Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Mon, 23 Jun 2025 14:49:38 +0200 Subject: [PATCH 5/5] Fix check class JavaDoc --- .../org/sonar/java/checks/unused/UnusedCollectionCheck.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java index 3926ea72fb6..95b5d76f6f8 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedCollectionCheck.java @@ -33,7 +33,8 @@ import org.sonar.plugins.java.api.tree.VariableTree; /** - * Check for collections that are new read, that is the only operations are adding and removing elements. + * Check for collections whose content is not used, + * that is, the only operations on them are adding and removing elements. */ @Rule(key = "S4030") public class UnusedCollectionCheck extends IssuableSubscriptionVisitor {