Skip to content

Commit e93a2a9

Browse files
erwan-serandourerwan-serandour
authored
SONARJAVA-5520 implement S7481: UseOfSequentialForSequentialGathererCheck (#5217)
1 parent e8874c9 commit e93a2a9

File tree

8 files changed

+339
-1
lines changed

8 files changed

+339
-1
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(13);
202+
softly.assertThat(rulesNotReporting).hasSize(14);
203203

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

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

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

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

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package checks;
2+
3+
import java.util.concurrent.atomic.AtomicInteger;
4+
import java.util.function.BinaryOperator;
5+
import java.util.stream.Gatherer;
6+
import java.util.stream.Stream;
7+
8+
public class UseOfSequentialForSequentialGathererCheckSample {
9+
10+
11+
void nonCompliant() {
12+
Gatherer<Integer, AtomicInteger, Integer> defaultCombiner = Gatherer.of(
13+
AtomicInteger::new,
14+
(state, element, downstream) -> downstream.push(element - state.getAndSet(element)),
15+
Gatherer.defaultCombiner(), // Noncompliant {{Replace `Gatherer.of(initializer, integrator, combiner, finisher)` with `ofSequential(initializer, integrator, finisher)`}}
16+
//^^^^^^^^^^^^^^^^^^^^^^^^^^
17+
Gatherer.defaultFinisher());
18+
19+
Gatherer<Integer, AtomicInteger, Integer> throwException = Gatherer.of(
20+
AtomicInteger::new,
21+
(state, element, downstream) -> downstream.push(element - state.getAndSet(element)),
22+
(s1, s2) -> {
23+
throw new IllegalStateException(); // Noncompliant
24+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
25+
},
26+
Gatherer.defaultFinisher());
27+
28+
}
29+
30+
31+
void compliant(boolean b) {
32+
Gatherer<Integer, AtomicInteger, Integer> noBlock = Gatherer.<Integer, AtomicInteger, Integer>of(
33+
AtomicInteger::new,
34+
(state, element, downstream) -> true,
35+
(left, right) -> new AtomicInteger(0),
36+
(res, _) -> res.get()
37+
);
38+
39+
Gatherer<Integer, AtomicInteger, Integer> blockOneStmt = Gatherer.of(
40+
AtomicInteger::new,
41+
(state, element, downstream) -> true,
42+
(left, right) -> {
43+
return new AtomicInteger(0);
44+
},
45+
(res, _) -> res.get()
46+
);
47+
48+
Gatherer<Integer, AtomicInteger, Integer> severalBranchAndThrow = Gatherer.<Integer, AtomicInteger, Integer>of(
49+
AtomicInteger::new,
50+
(state, element, downstream) -> true,
51+
(left, right) -> {
52+
if (b) {
53+
throw new IllegalStateException();
54+
}
55+
return left;
56+
},
57+
(res, _) -> res.get()
58+
);
59+
}
60+
61+
void falseNegatives() {
62+
Gatherer<Integer, AtomicInteger, Integer> twoStmtBody = Gatherer.<Integer, AtomicInteger, Integer>of(
63+
AtomicInteger::new,
64+
(state, element, downstream) -> true,
65+
(s1, s2) -> {
66+
System.out.println("sequential gatherer");
67+
throw new IllegalStateException(); // FN
68+
},
69+
Gatherer.defaultFinisher());
70+
71+
Gatherer<Integer, AtomicInteger, Integer> methodRef = Gatherer.of(
72+
AtomicInteger::new,
73+
(state, element, downstream) -> true,
74+
UseOfSequentialForSequentialGathererCheckSample::methodRefNotSupported, // FN
75+
Gatherer.defaultFinisher());
76+
77+
Gatherer<Integer, AtomicInteger, Integer> methodInvocation = Gatherer.of(
78+
AtomicInteger::new,
79+
(state, element, downstream) -> true,
80+
noSupported(), // FN
81+
Gatherer.defaultFinisher());
82+
}
83+
84+
85+
static AtomicInteger methodRefNotSupported(AtomicInteger a, AtomicInteger b) {
86+
throw new IllegalStateException();
87+
}
88+
89+
static BinaryOperator<AtomicInteger> noSupported() {
90+
return (left, right) -> {
91+
throw new IllegalStateException();
92+
};
93+
}
94+
}

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

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
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.List;
20+
import java.util.function.Function;
21+
import org.sonar.check.Rule;
22+
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
23+
import org.sonar.plugins.java.api.JavaFileScannerContext;
24+
import org.sonar.plugins.java.api.semantic.MethodMatchers;
25+
import org.sonar.plugins.java.api.tree.BlockTree;
26+
import org.sonar.plugins.java.api.tree.ExpressionTree;
27+
import org.sonar.plugins.java.api.tree.LambdaExpressionTree;
28+
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
29+
import org.sonar.plugins.java.api.tree.ThrowStatementTree;
30+
import org.sonar.plugins.java.api.tree.Tree;
31+
32+
33+
@Rule(key = "S7481")
34+
public class UseOfSequentialForSequentialGathererCheck extends IssuableSubscriptionVisitor {
35+
/**
36+
* inner classes
37+
*/
38+
39+
/**
40+
* Contains all information necessary to find out if a method invocation should be replaced by another method invocation.
41+
*/
42+
private record Case(MethodMatchers matcher, ArgumentPredicate pred, String msg) {
43+
}
44+
45+
/**
46+
* @param predicate return locations of the issue if empty no issue must be reported
47+
*/
48+
private record ArgumentPredicate(int argIdx, Function<ExpressionTree, List<? extends Tree>> predicate) {
49+
}
50+
51+
/**
52+
* constants
53+
*/
54+
private static final String GATHERER = "java.util.stream.Gatherer";
55+
private static final String SUPPLIER = "java.util.function.Supplier";
56+
private static final String INTEGRATOR = "java.util.stream.Gatherer$Integrator";
57+
private static final String BI_CONSUMER = "java.util.function.BiConsumer";
58+
private static final String BINARY_OPERATOR = "java.util.function.BinaryOperator";
59+
60+
61+
private static final MethodMatchers DEFAULT_COMBINER = MethodMatchers.create()
62+
.ofTypes(GATHERER)
63+
.names("defaultCombiner")
64+
.addWithoutParametersMatcher()
65+
.build();
66+
private static final MethodMatchers GATHERER_OF = MethodMatchers.create()
67+
.ofTypes(GATHERER)
68+
.names("of")
69+
.addParametersMatcher(SUPPLIER, INTEGRATOR, BINARY_OPERATOR, BI_CONSUMER)
70+
.build();
71+
72+
private static final ArgumentPredicate SEQUENTIAL_PREDICATE = new ArgumentPredicate(2, UseOfSequentialForSequentialGathererCheck::isSequentialCombiner);
73+
private static final List<Case> CASES = List.of(
74+
new Case(
75+
GATHERER_OF,
76+
SEQUENTIAL_PREDICATE,
77+
"Replace `Gatherer.of(initializer, integrator, combiner, finisher)` with `ofSequential(initializer, integrator, finisher)`")
78+
);
79+
80+
81+
@Override
82+
public List<Tree.Kind> nodesToVisit() {
83+
return List.of(Tree.Kind.METHOD_INVOCATION);
84+
}
85+
86+
@Override
87+
public void visitNode(Tree tree) {
88+
MethodInvocationTree mit = (MethodInvocationTree) tree;
89+
90+
for (Case caze : CASES) {
91+
if (!caze.matcher.matches(mit)) {
92+
continue;
93+
}
94+
var argumentPredicate = caze.pred;
95+
var issues = argumentPredicate.predicate.apply(mit.arguments().get(argumentPredicate.argIdx));
96+
if (!issues.isEmpty()) {
97+
98+
var secondaries = issues.subList(1, issues.size())
99+
.stream()
100+
.map(element -> new JavaFileScannerContext.Location("", element))
101+
.toList();
102+
103+
context.reportIssue(this, issues.get(0), caze.msg, secondaries, null);
104+
return;
105+
}
106+
}
107+
108+
}
109+
110+
private static List<? extends Tree> isSequentialCombiner(ExpressionTree expr) {
111+
112+
if (expr instanceof LambdaExpressionTree lambda && lambda.body() instanceof BlockTree block) {
113+
if (block.body().size() == 1 && block.body().get(0) instanceof ThrowStatementTree throwStmt) {
114+
return List.of(throwStmt);
115+
}
116+
} else if (expr instanceof MethodInvocationTree mit && DEFAULT_COMBINER.matches(mit)) {
117+
return List.of(mit);
118+
}
119+
120+
return List.of();
121+
}
122+
}

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

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

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<h2>Why is this an issue?</h2>
2+
<p>When a method uses a <code>Gatherer.of(…​)</code> factory and provides a combiner function that always throws an exception, this implicitly signals
3+
that the gatherer is designed for sequential processing. The <code>java.util.stream.Gatherer</code> API provides
4+
<code>Gatherer.ofSequential(…​)</code> factories which clearly indicates that the gatherer is intended for sequential streams. Using such a factory
5+
improves code clarity, makes the intended processing model explicit, and avoids the need for a dummy or throwing combiner.</p>
6+
<h2>How to fix it</h2>
7+
<p>Use <code>Gatherer.ofSequential</code> instead of <code>Gatherer.of</code>.</p>
8+
<h3>Code examples</h3>
9+
<h4>Noncompliant code example</h4>
10+
<pre data-diff-id="1" data-diff-type="noncompliant">
11+
public static List&lt;Integer&gt; diffWithFirstPositive(List&lt;Integer&gt; list) {
12+
Gatherer&lt;Integer, AtomicInteger, Integer&gt; gatherer = Gatherer.of(
13+
() -&gt; new AtomicInteger(-1),
14+
(state, number, downstream) -&gt; {
15+
if (state.get() &lt; 0) {
16+
state.set(number);
17+
return true;
18+
}
19+
return downstream.push(number - state.get());
20+
},
21+
(_, _) -&gt; {
22+
throw new IllegalStateException();
23+
},
24+
Gatherer.defaultFinisher());
25+
return list.stream().gather(gatherer).toList();
26+
}
27+
</pre>
28+
<h4>Compliant solution</h4>
29+
<pre data-diff-id="1" data-diff-type="compliant">
30+
public static List&lt;Integer&gt; diffWithFirstPositive(List&lt;Integer&gt; list) {
31+
Gatherer&lt;Integer, AtomicInteger, Integer&gt; gatherer = Gatherer.ofSequential(
32+
() -&gt; new AtomicInteger(-1),
33+
(state, number, downstream) -&gt; {
34+
if (state.get() &lt; 0) {
35+
state.set(number);
36+
return true;
37+
}
38+
return downstream.push(number - state.get());
39+
},
40+
Gatherer.defaultFinisher());
41+
return list.stream().gather(gatherer).toList();
42+
}
43+
</pre>
44+
<h2>Resources</h2>
45+
<ul>
46+
<li> <a href="https://docs.oracle.com/en/java/javase/24/docs/api/java.base/java/util/stream/Gatherer.html">Gatherer API</a> </li>
47+
<li> <a href="https://docs.oracle.com/en/java/javase/24/core/stream-gatherers.html">Stream gatherers documentation</a> </li>
48+
</ul>
49+

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"title": "Gatherer.ofSequential() should be used to build sequential gathers",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "1min"
8+
},
9+
"tags": [
10+
"java24"
11+
],
12+
"defaultSeverity": "Minor",
13+
"ruleSpecification": "RSPEC-7481",
14+
"sqKey": "S7481",
15+
"scope": "All",
16+
"quickfix": "targeted",
17+
"code": {
18+
"impacts": {
19+
"MAINTAINABILITY": "LOW"
20+
},
21+
"attribute": "CLEAR"
22+
}
23+
}

‎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
@@ -511,6 +511,7 @@
511511
"S7474",
512512
"S7475",
513513
"S7476",
514+
"S7481",
514515
"S7482"
515516
]
516517
}

0 commit comments

Comments
 (0)