Skip to content

Commit 445816f

Browse files
erwan-serandourerwan-serandour
authored
SONARJAVA-5518 implement S7482: ForStatelessGatherersOmitInitializerCheck (#5213)
1 parent 7e72424 commit 445816f

File tree

9 files changed

+424
-4
lines changed

9 files changed

+424
-4
lines changed

‎its/autoscan/src/test/java/org/sonar/java/it/AutoScanTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ public void javaCheckTestSources() throws Exception {
199199
softly.assertThat(newDiffs).containsExactlyInAnyOrderElementsOf(knownDiffs.values());
200200
softly.assertThat(newTotal).isEqualTo(knownTotal);
201201
softly.assertThat(rulesCausingFPs).hasSize(9);
202-
softly.assertThat(rulesNotReporting).hasSize(12);
202+
softly.assertThat(rulesNotReporting).hasSize(13);
203203

204204
/**
205205
* 4. Check total number of differences (FPs + FNs)

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

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

‎java-checks-test-sources/default/src/main/java/checks/ForStatelessGatherersOmitInitializerCheckSample.java‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
package checks;
2+
3+
import java.util.LinkedList;
4+
import java.util.List;
5+
import java.util.Optional;
6+
import java.util.function.Supplier;
7+
import java.util.stream.Gatherer;
8+
import java.util.stream.Stream;
9+
10+
public class ForStatelessGatherersOmitInitializerCheckSample {
11+
void nonCompliantSingleReturnNull() {
12+
Gatherer<Integer, ?, Integer> lambdaNull = Gatherer.<Integer, Void, Integer>ofSequential(
13+
() -> null, // Noncompliant {{Replace `Gatherer.ofSequential(initializer, integrator)` with `Gatherer.ofSequential(integrator)`}}
14+
// ^^^^
15+
(_, element, downstream) -> downstream.push(element)
16+
);
17+
Stream.of(1, 2, 3).gather(lambdaNull).forEach(System.out::println);
18+
19+
Gatherer<Integer, ?, Integer> defaultInitializer = Gatherer.<Integer, Void, Integer>ofSequential(
20+
Gatherer.defaultInitializer(), // Noncompliant
21+
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
22+
(_, element, downstream) -> downstream.push(element)
23+
);
24+
Stream.of(1, 2, 3).gather(lambdaNull).forEach(System.out::println);
25+
26+
Gatherer<Integer, ?, Integer> lambdaWithBody = Gatherer.<Integer, Void, Integer>ofSequential(
27+
() -> {
28+
System.out.println("initializer");
29+
return null; // Noncompliant
30+
// ^^^^^^^^^^^^
31+
},
32+
(_, element, downstream) -> downstream.push(element)
33+
);
34+
Stream.of(1, 2, 3).gather(lambdaWithBody).forEach(System.out::println);
35+
}
36+
37+
38+
void nonCompliantSingleReturnOptional() {
39+
Gatherer<Integer, ?, Integer> trivial = Gatherer.<Integer, Optional<Object>, Integer>ofSequential(
40+
() -> Optional.empty(), // Noncompliant
41+
// ^^^^^^^^^^^^^^^^
42+
(_, element, downstream) -> downstream.push(element)
43+
);
44+
Stream.of(1, 2, 3).gather(trivial).forEach(System.out::println);
45+
46+
Gatherer<Integer, ?, Integer> finisher = Gatherer.<Integer, Optional<Long>, Integer>ofSequential(
47+
() -> Optional.empty(), // Noncompliant {{Replace `Gatherer.ofSequential(initializer, integrator, finisher)` with `Gatherer.ofSequential(integrator, finisher)`}}
48+
(_, element, downstream) -> downstream.push(element),
49+
(_, downstream) -> System.out.println("finisher")
50+
);
51+
Stream.of(1, 2, 3).gather(finisher).forEach(System.out::println);
52+
}
53+
54+
55+
void compliantStateLess() {
56+
Gatherer<Integer, ?, Integer> trivial = Gatherer.<Integer, Integer>ofSequential(
57+
(_, element, downstream) -> downstream.push(element)
58+
);
59+
Stream.of(1, 2, 3).gather(trivial).forEach(System.out::println);
60+
61+
Gatherer<Integer, ?, Integer> finisher = Gatherer.<Integer, Integer>ofSequential(
62+
(_, element, downstream) -> downstream.push(element),
63+
(_, downstream) -> System.out.println("finisher")
64+
);
65+
Stream.of(1, 2, 3).gather(finisher).forEach(System.out::println);
66+
}
67+
68+
69+
void compliantStateful() {
70+
Gatherer<Integer, ?, Integer> object = Gatherer.<Integer, Object, Integer>ofSequential(
71+
Object::new,
72+
(_, element, downstream) -> downstream.push(element)
73+
);
74+
Stream.of(1, 2, 3).gather(object).forEach(System.out::println);
75+
76+
Gatherer<Integer, ?, Integer> option = Gatherer.<Integer, Optional<String>, Integer>ofSequential(
77+
() -> Optional.of("state"),
78+
(_, element, downstream) -> downstream.push(element)
79+
);
80+
Stream.of(1, 2, 3).gather(option).forEach(System.out::println);
81+
82+
Gatherer<Integer, List<Integer>, Integer> finisher = Gatherer.<Integer, List<Integer>, Integer>ofSequential(
83+
LinkedList::new,
84+
(_, element, downstream) -> downstream.push(element),
85+
(_, downstream) -> System.out.println("finisher")
86+
);
87+
Stream.of(1, 2, 3).gather(finisher).forEach(System.out::println);
88+
}
89+
90+
91+
void noncompliantMultipleReturns(boolean condition) {
92+
Gatherer<Integer, ?, Integer> g = Gatherer.<Integer, Void, Integer>ofSequential(
93+
() -> {
94+
if (condition) {
95+
return null; // Noncompliant {{Replace `Gatherer.ofSequential(initializer, integrator)` with `Gatherer.ofSequential(integrator)`}}
96+
// ^^^^^^^^^^^^
97+
}
98+
return null;
99+
//^^^^^^^^^^^^< {{}}
100+
},
101+
(_, element, downstream) -> downstream.push(element)
102+
);
103+
Stream.of(1, 2, 3).gather(g).forEach(System.out::println);
104+
}
105+
106+
void compliantMultipleReturns(boolean condition) {
107+
Gatherer<Integer, ?, Integer> g = Gatherer.<Integer, Object, Integer>ofSequential(
108+
() -> {
109+
if (condition) {
110+
return new Object();
111+
}
112+
return null;
113+
},
114+
(_, element, downstream) -> downstream.push(element)
115+
);
116+
Stream.of(1, 2, 3).gather(g).forEach(System.out::println);
117+
}
118+
119+
void falseNegatives() {
120+
Gatherer<Integer, ?, Integer> methodReference = Gatherer.<Integer, Integer, Integer>ofSequential(
121+
ForStatelessGatherersOmitInitializerCheckSample::initialState, // FN we do not check method reference
122+
(_, element, downstream) -> downstream.push(element)
123+
);
124+
Gatherer<Integer, ?, Integer> makeInitializer = Gatherer.<Integer, Integer, Integer>ofSequential(
125+
createInitializer(), // FN we do not go inside method call
126+
(_, element, downstream) -> downstream.push(element)
127+
);
128+
}
129+
130+
static Supplier<Integer> createInitializer(){
131+
return () -> null;
132+
}
133+
134+
static Integer initialState() {
135+
return null;
136+
}
137+
138+
}

‎java-checks/src/main/java/org/sonar/java/checks/ForStatelessGatherersOmitInitializerCheck.java‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
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;
18+
19+
import java.util.ArrayList;
20+
import java.util.List;
21+
import java.util.function.Function;
22+
import org.sonar.check.Rule;
23+
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
24+
import org.sonar.plugins.java.api.JavaFileScannerContext;
25+
import org.sonar.plugins.java.api.semantic.MethodMatchers;
26+
import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
27+
import org.sonar.plugins.java.api.tree.BlockTree;
28+
import org.sonar.plugins.java.api.tree.ExpressionTree;
29+
import org.sonar.plugins.java.api.tree.LambdaExpressionTree;
30+
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
31+
import org.sonar.plugins.java.api.tree.ReturnStatementTree;
32+
import org.sonar.plugins.java.api.tree.Tree;
33+
34+
35+
@Rule(key = "S7482")
36+
public class ForStatelessGatherersOmitInitializerCheck extends IssuableSubscriptionVisitor {
37+
/**
38+
* inner classes
39+
*/
40+
41+
/**
42+
* Contains all information necessary to find out if a method invocation should be replaced by another method invocation.
43+
*/
44+
private record Case(MethodMatchers matcher, ArgumentPredicate pred, String msg) {
45+
}
46+
47+
private record ArgumentPredicate(int argIdx, Function<ExpressionTree, List<? extends Tree>> predicate) {
48+
}
49+
50+
/**
51+
* constants
52+
*/
53+
private static final String GATHERER = "java.util.stream.Gatherer";
54+
private static final String SUPPLIER = "java.util.function.Supplier";
55+
private static final String INTEGRATOR = "java.util.stream.Gatherer$Integrator";
56+
private static final String BI_CONSUMER = "java.util.function.BiConsumer";
57+
58+
private static final MethodMatchers OPTIONAL_EMPTY = MethodMatchers.create()
59+
.ofTypes("java.util.Optional")
60+
.names("empty")
61+
.addWithoutParametersMatcher()
62+
.build();
63+
private static final MethodMatchers DEFAULT_INITIALIZER = MethodMatchers.create()
64+
.ofTypes(GATHERER)
65+
.names("defaultInitializer")
66+
.addWithoutParametersMatcher()
67+
.build();
68+
private static final MethodMatchers OF_SEQUENTIAL_WITHOUT_FINISHER = MethodMatchers.create()
69+
.ofTypes(GATHERER)
70+
.names("ofSequential")
71+
.addParametersMatcher(SUPPLIER, INTEGRATOR)
72+
.build();
73+
private static final MethodMatchers OF_SEQUENTIAL_WITH_FINISHER = MethodMatchers.create()
74+
.ofTypes(GATHERER)
75+
.names("ofSequential")
76+
.addParametersMatcher(SUPPLIER, INTEGRATOR, BI_CONSUMER)
77+
.build();
78+
79+
private static final ArgumentPredicate INITIALIZER_PREDICATE = new ArgumentPredicate(0, ForStatelessGatherersOmitInitializerCheck::isStatelessInitializer);
80+
private static final List<Case> CASES = List.of(
81+
new Case(OF_SEQUENTIAL_WITHOUT_FINISHER, INITIALIZER_PREDICATE, "Replace `Gatherer.ofSequential(initializer, integrator)` with `Gatherer.ofSequential(integrator)`"),
82+
new Case(OF_SEQUENTIAL_WITH_FINISHER, INITIALIZER_PREDICATE, "Replace `Gatherer.ofSequential(initializer, integrator, finisher)` with `Gatherer.ofSequential(integrator, " +
83+
"finisher)`")
84+
);
85+
86+
87+
@Override
88+
public List<Tree.Kind> nodesToVisit() {
89+
return List.of(Tree.Kind.METHOD_INVOCATION);
90+
}
91+
92+
@Override
93+
public void visitNode(Tree tree) {
94+
MethodInvocationTree mit = (MethodInvocationTree) tree;
95+
96+
for (Case c : CASES) {
97+
if (!c.matcher.matches(mit)) {
98+
continue;
99+
}
100+
101+
var argPredicate = c.pred;
102+
var issues = argPredicate.predicate.apply(mit.arguments().get(argPredicate.argIdx));
103+
if (!issues.isEmpty()) {
104+
105+
var secondaries = issues.subList(1, issues.size())
106+
.stream()
107+
.map(element -> new JavaFileScannerContext.Location("", element))
108+
.toList();
109+
110+
context.reportIssue(this, issues.get(0), c.msg, secondaries, null);
111+
return;
112+
}
113+
}
114+
}
115+
116+
/**
117+
* Checks if the given initializer function always return a stateless value.
118+
* An initializer is a Supplier<A> where A is not void.
119+
*/
120+
private static List<? extends Tree> isStatelessInitializer(ExpressionTree initializer) {
121+
if (initializer instanceof LambdaExpressionTree lambda) {
122+
var body = lambda.body();
123+
if (isStateless(body)) {
124+
return List.of(body);
125+
}
126+
127+
if (body instanceof BlockTree block) {
128+
List<ReturnStatementTree> all = ReturnStatementCollector.collect(block);
129+
130+
if (all.stream().allMatch(rs -> isStateless(rs.expression()))) {
131+
return all;
132+
}
133+
}
134+
} else if (initializer instanceof MethodInvocationTree mit && DEFAULT_INITIALIZER.matches(mit)) {
135+
return List.of(mit);
136+
}
137+
138+
return List.of();
139+
}
140+
141+
private static boolean isStateless(Tree tree) {
142+
return tree.is(Tree.Kind.NULL_LITERAL)
143+
|| (tree instanceof MethodInvocationTree mit && OPTIONAL_EMPTY.matches(mit));
144+
}
145+
146+
/**
147+
* Utility visitor that collects all return statements from a given tree.
148+
*/
149+
private static class ReturnStatementCollector extends BaseTreeVisitor {
150+
private final List<ReturnStatementTree> returnStatements = new ArrayList<>();
151+
152+
public List<ReturnStatementTree> getReturnStatements() {
153+
return returnStatements;
154+
}
155+
156+
@Override
157+
public void visitReturnStatement(ReturnStatementTree tree) {
158+
returnStatements.add(tree);
159+
super.visitReturnStatement(tree);
160+
}
161+
162+
163+
public static List<ReturnStatementTree> collect(Tree tree) {
164+
ReturnStatementCollector collector = new ReturnStatementCollector();
165+
tree.accept(collector);
166+
return collector.getReturnStatements();
167+
}
168+
}
169+
}

‎java-checks/src/test/java/org/sonar/java/checks/ForStatelessGatherersOmitInitializerCheckTest.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
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;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.java.checks.verifier.CheckVerifier;
21+
import org.sonar.java.checks.verifier.TestUtils;
22+
23+
class ForStatelessGatherersOmitInitializerCheckTest {
24+
25+
@Test
26+
void test() {
27+
CheckVerifier.newVerifier()
28+
.onFile(TestUtils.mainCodeSourcesPath("checks/ForStatelessGatherersOmitInitializerCheckSample.java"))
29+
.withCheck(new ForStatelessGatherersOmitInitializerCheck())
30+
.verifyIssues();
31+
}
32+
33+
@Test
34+
void without_semantic() {
35+
CheckVerifier.newVerifier()
36+
.onFile(TestUtils.mainCodeSourcesPath("checks/ForStatelessGatherersOmitInitializerCheckSample.java"))
37+
.withCheck(new ForStatelessGatherersOmitInitializerCheck())
38+
.withoutSemantic()
39+
.verifyIssues();
40+
}
41+
42+
}

0 commit comments

Comments
 (0)