From 30861b001d511b665377ded43f022e899ea47586 Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Wed, 27 May 2026 11:24:04 +0200 Subject: [PATCH 1/7] S5778 allow calling factory methods to create empty collections --- ...neExpectedRuntimeExceptionCheckSample.java | 52 +++++++++++++++++++ .../OneExpectedRuntimeExceptionCheck.java | 35 ++++++++++++- 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/OneExpectedRuntimeExceptionCheckSample.java b/java-checks-test-sources/default/src/test/java/checks/tests/OneExpectedRuntimeExceptionCheckSample.java index a660c099ce1..64dd09ec419 100644 --- a/java-checks-test-sources/default/src/test/java/checks/tests/OneExpectedRuntimeExceptionCheckSample.java +++ b/java-checks-test-sources/default/src/test/java/checks/tests/OneExpectedRuntimeExceptionCheckSample.java @@ -1,6 +1,18 @@ package checks.tests; import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Stream; + import org.junit.Assert; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; @@ -228,6 +240,39 @@ public void test_AssertJ() { org.assertj.core.api.Assertions.assertThat(thrown).isInstanceOf(IllegalStateException.class); } + @Test + void testAllowEmptyCollections() { + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1)); + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, foo(2))); // Noncompliant + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Collections.emptyList())); + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, foo(2), Collections.emptyList())); // Noncompliant + + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Collections.emptySet())); + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Collections.emptyMap())); + + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, List.of())); + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, List.of(foo(2)))); // Noncompliant + + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Set.of())); + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Set.of(foo(2)))); // Noncompliant + + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Map.of())); + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Map.of(foo(2), foo(3)))); // Noncompliant + + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, new ArrayList<>())); + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, new LinkedList<>())); + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, new HashSet<>())); + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, new HashMap<>())); + + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, new ArrayList<>(getCollection()))); // Noncompliant + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, new LinkedList<>(getCollection()))); // Noncompliant + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, new HashSet<>(getCollection()))); // Noncompliant + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, new HashMap<>(foo(2), foo(3)))); // Noncompliant + + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Stream.empty())); + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Optional.empty())); + } + int foo(int x) { return x; @@ -255,4 +300,11 @@ void foo() { } } + void objVarargMethod(Object... args) { + throw new IllegalArgumentException(); + } + + Collection getCollection() { + return new ArrayList<>(); + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/OneExpectedRuntimeExceptionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/OneExpectedRuntimeExceptionCheck.java index 4e5d83ed865..0060e132ffb 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/tests/OneExpectedRuntimeExceptionCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/OneExpectedRuntimeExceptionCheck.java @@ -33,15 +33,48 @@ public class OneExpectedRuntimeExceptionCheck extends AbstractOneExpectedExcepti .names("mock") .addParametersMatcher("java.lang.Class").addParametersMatcher("java.lang.Class", "java.lang.String") .build(); + private static final MethodMatchers ENUM_FINAL_METHODS = MethodMatchers.create() .ofSubTypes("java.lang.Enum") .names("name", "ordinal", "equals", "hashCode", "compareTo", "getDeclaringClass", "describeConstable") .withAnyParameters() .build(); + + private static final MethodMatchers COLLECTIONS_EMPTY = MethodMatchers.create() + .ofTypes("java.util.Collections") + .names("emptyList", "emptySet", "emptyMap") + .addWithoutParametersMatcher() + .build(); + + private static final MethodMatchers COLLECTION_OF = MethodMatchers.create() + .ofTypes("java.util.List", "java.util.Set", "java.util.Map") + .names("of") + .addWithoutParametersMatcher() + .build(); + + private static final MethodMatchers COLLECTION_CTOR = MethodMatchers.create() + .ofSubTypes("java.util.ArrayList", "java.util.LinkedList", "java.util.HashSet", "java.util.HashMap") + .constructor() + .addWithoutParametersMatcher() + .build(); + + private static final MethodMatchers EMPTY = MethodMatchers.create() + .ofTypes("java.util.Optional", "java.util.stream.Stream") + .names("empty") + .addWithoutParametersMatcher() + .build(); + + /** + * Frequently used methods known not to throw runtime exceptions. + */ private static final MethodMatchers AUTHORIZED_METHODS = MethodMatchers.or( FAIL_METHOD_MATCHER, MOCKITO_MOCK_METHOD_MATCHERS, - ENUM_FINAL_METHODS + ENUM_FINAL_METHODS, + COLLECTIONS_EMPTY, + COLLECTION_OF, + COLLECTION_CTOR, + EMPTY ); @Override From 70a899a9594dec8ffb786503fb104befa236da9c Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Wed, 27 May 2026 13:59:35 +0200 Subject: [PATCH 2/7] Fix autoscan diff --- its/autoscan/src/test/resources/autoscan/diffs/diff_S5778.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5778.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5778.json index 725e6ad2614..faa4a92df7c 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5778.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5778.json @@ -1,6 +1,6 @@ { "ruleKey": "S5778", "hasTruePositives": false, - "falseNegatives": 32, + "falseNegatives": 41, "falsePositives": 0 } From 0a70d6539e64517faa166dd007613d5c01909464 Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Wed, 27 May 2026 14:01:58 +0200 Subject: [PATCH 3/7] Fix ruling diff --- its/ruling/src/test/resources/sonar-server/java-S5778.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/its/ruling/src/test/resources/sonar-server/java-S5778.json b/its/ruling/src/test/resources/sonar-server/java-S5778.json index 9913bd9b7d1..c06a0840e65 100644 --- a/its/ruling/src/test/resources/sonar-server/java-S5778.json +++ b/its/ruling/src/test/resources/sonar-server/java-S5778.json @@ -5,9 +5,6 @@ "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/batch/ProjectDataLoaderTest.java": [ 121 ], -"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/issue/ScmAccountToUserLoaderTest.java": [ -89 -], "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/measure/MapBasedRawMeasureRepositoryTest.java": [ 187 ], From 545a690f685aaf02766fd011d05e76f815623c3c Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Wed, 27 May 2026 15:32:35 +0200 Subject: [PATCH 4/7] Add Collections.singleton* --- .../tests/OneExpectedRuntimeExceptionCheckSample.java | 9 +++++++++ .../checks/tests/OneExpectedRuntimeExceptionCheck.java | 7 +++++++ 2 files changed, 16 insertions(+) diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/OneExpectedRuntimeExceptionCheckSample.java b/java-checks-test-sources/default/src/test/java/checks/tests/OneExpectedRuntimeExceptionCheckSample.java index 64dd09ec419..3eaa322c163 100644 --- a/java-checks-test-sources/default/src/test/java/checks/tests/OneExpectedRuntimeExceptionCheckSample.java +++ b/java-checks-test-sources/default/src/test/java/checks/tests/OneExpectedRuntimeExceptionCheckSample.java @@ -250,6 +250,15 @@ void testAllowEmptyCollections() { assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Collections.emptySet())); assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Collections.emptyMap())); + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Collections.singleton(2))); + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Collections.singleton(foo(2)))); // Noncompliant + + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Collections.singletonList(2))); + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Collections.singletonList(foo(2)))); // Noncompliant + + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Collections.singletonMap(2, 3))); + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, Collections.singletonMap(foo(2), foo(3)))); // Noncompliant + assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, List.of())); assertThrows(IllegalArgumentException.class, () -> objVarargMethod(1, List.of(foo(2)))); // Noncompliant diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/OneExpectedRuntimeExceptionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/OneExpectedRuntimeExceptionCheck.java index 0060e132ffb..6b567f0554b 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/tests/OneExpectedRuntimeExceptionCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/OneExpectedRuntimeExceptionCheck.java @@ -46,6 +46,12 @@ public class OneExpectedRuntimeExceptionCheck extends AbstractOneExpectedExcepti .addWithoutParametersMatcher() .build(); + private static final MethodMatchers COLLECTIONS_SINGLETON = MethodMatchers.create() + .ofTypes("java.util.Collections") + .names("singleton", "singletonList", "singletonMap") + .withAnyParameters() + .build(); + private static final MethodMatchers COLLECTION_OF = MethodMatchers.create() .ofTypes("java.util.List", "java.util.Set", "java.util.Map") .names("of") @@ -72,6 +78,7 @@ public class OneExpectedRuntimeExceptionCheck extends AbstractOneExpectedExcepti MOCKITO_MOCK_METHOD_MATCHERS, ENUM_FINAL_METHODS, COLLECTIONS_EMPTY, + COLLECTIONS_SINGLETON, COLLECTION_OF, COLLECTION_CTOR, EMPTY From e52835708dab30291ac1d8cc66e87c27d5b46df6 Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Wed, 27 May 2026 15:39:30 +0200 Subject: [PATCH 5/7] comment --- .../java/checks/tests/OneExpectedRuntimeExceptionCheck.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/OneExpectedRuntimeExceptionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/OneExpectedRuntimeExceptionCheck.java index 6b567f0554b..785b151226c 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/tests/OneExpectedRuntimeExceptionCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/OneExpectedRuntimeExceptionCheck.java @@ -73,6 +73,8 @@ public class OneExpectedRuntimeExceptionCheck extends AbstractOneExpectedExcepti /** * Frequently used methods known not to throw runtime exceptions. */ + // If adding to this list, make sure that the methods do not call NullPointerExceptions + // (for example, `Arrays.asList(null)` throws). private static final MethodMatchers AUTHORIZED_METHODS = MethodMatchers.or( FAIL_METHOD_MATCHER, MOCKITO_MOCK_METHOD_MATCHERS, From e930298457a39997e873c362d70c128e1a67d679 Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Wed, 27 May 2026 15:52:23 +0200 Subject: [PATCH 6/7] 3 more autoscan FNs --- its/autoscan/src/test/resources/autoscan/diffs/diff_S5778.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5778.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5778.json index faa4a92df7c..9fc6644b170 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5778.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5778.json @@ -1,6 +1,6 @@ { "ruleKey": "S5778", "hasTruePositives": false, - "falseNegatives": 41, + "falseNegatives": 44, "falsePositives": 0 } From c03ac2f6a71d8b7bf8e603f1fe5ae1dd327c5dea Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Wed, 27 May 2026 16:14:54 +0200 Subject: [PATCH 7/7] S5961 +1 autoscan --- its/autoscan/src/test/resources/autoscan/diffs/diff_S5961.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5961.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5961.json index 39329ff6bae..078899b8899 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5961.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5961.json @@ -1,6 +1,6 @@ { "ruleKey": "S5961", "hasTruePositives": false, - "falseNegatives": 5, + "falseNegatives": 6, "falsePositives": 0 } \ No newline at end of file