Skip to content

SONARJAVA-4426 S5778 allow calling factory methods to create empty collections #5633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tomasz-tylenda-sonarsource merged 7 commits into from
May 27, 2026
Merged

SONARJAVA-4426 S5778 allow calling factory methods to create empty collections #5633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
30861b0
S5778 allow calling factory methods to create empty collections
tomasz-tylenda-sonarsource May 27, 2026
70a899a
Fix autoscan diff
tomasz-tylenda-sonarsource May 27, 2026
0a70d65
Fix ruling diff
tomasz-tylenda-sonarsource May 27, 2026
545a690
Add Collections.singleton*
tomasz-tylenda-sonarsource May 27, 2026
e528357
comment
tomasz-tylenda-sonarsource May 27, 2026
e930298
3 more autoscan FNs
tomasz-tylenda-sonarsource May 27, 2026
c03ac2f
S5961 +1 autoscan
tomasz-tylenda-sonarsource May 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion its/autoscan/src/test/resources/autoscan/diffs/diff_S5778.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S5778",
"hasTruePositives": false,
"falseNegatives": 32,
"falseNegatives": 44,
"falsePositives": 0
}
2 changes: 1 addition & 1 deletion its/autoscan/src/test/resources/autoscan/diffs/diff_S5961.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S5961",
"hasTruePositives": false,
"falseNegatives": 5,
"falseNegatives": 6,
"falsePositives": 0
}
3 changes: 0 additions & 3 deletions its/ruling/src/test/resources/sonar-server/java-S5778.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
],
Expand Down
61 changes: 61 additions & 0 deletions ...st-sources/default/src/test/java/checks/tests/OneExpectedRuntimeExceptionCheckSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -228,6 +240,48 @@ 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, 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

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;
Expand Down Expand Up @@ -255,4 +309,11 @@ void foo() {
}
}

void objVarargMethod(Object... args) {
throw new IllegalArgumentException();
}

<T> Collection<T> getCollection() {
return new ArrayList<>();
}
}
44 changes: 43 additions & 1 deletion java-checks/src/main/java/org/sonar/java/checks/tests/OneExpectedRuntimeExceptionCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,57 @@ 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 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")
.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.
*/
// 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,
ENUM_FINAL_METHODS
ENUM_FINAL_METHODS,
COLLECTIONS_EMPTY,
COLLECTIONS_SINGLETON,
COLLECTION_OF,
COLLECTION_CTOR,
EMPTY
Copy link
Copy Markdown
Contributor

@rombirli rombirli May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I've seen your comment under the jira ticket, but there are many otheres non-raising functions, here are a few commonly used that could make sense to add.

  • Arrays.asList(...)
  • Collections.singleton* : Collections.singleton(o) / Collections.singletonList(o) / Collections.singletonMap(k, v)...
  • *.of: Stream.of(t) / Optional.ofNullable(v)...
  • boxing functions :Byte.valueOf(b), Boolean.valueOf(b), Integer.valueOf(i)
  • ...
    I'm not requesting any change, just proposing very commonly used functions you can add if you think it makes sense

Copy link
Copy Markdown
Contributor Author

@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added Collections.singleton*. Arrays.asList can throw NPE. While the other examples can lead to FPs, I don't think we should be adding a large collection of cases unless we have some evidence that they happen in practice.

I think we can validate this PR on Peachee and follow up if we see more cases worth excluding.

Copy link
Copy Markdown
Contributor

@rombirli rombirli May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!

);

@Override
Expand Down
Loading