Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"ruleKey": "S3706",
"hasTruePositives": true,
"falseNegatives": 0,
"falsePositives": 0
}
12 changes: 12 additions & 0 deletions its/ruling/src/test/resources/sonar-server/java-S3706.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/permission/ws/template/SearchTemplatesDataLoader.java": [
96
],
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/rule/ws/ShowAction.java": [
140
],
"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/qualityprofile/RuleActivatorTest.java": [
863,
898
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package checks;

import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.stream.Stream;

public class StreamForeachCheck {

void unnecessaryStreamForEach(Collection<Integer> collection) {
collection.stream().forEach(System.out::println); // Noncompliant {{Simplify the code by replacing .stream().forEach() with .forEach().}}
// ^^^^^^^^^^^^^^^^
}

void compliantCollectionForEach(Collection<Integer> collection) {
collection.forEach(System.out::println); // Compliant
}

void necessaryFilterForEach(Collection<String> col) {
col.stream().filter(s -> !s.isEmpty()).forEach(System.out::println);
}

void unnecessaryForEachOnSet(Set<String> set) {
set.stream().forEach(e -> System.out.println("Element: " + e)); // Noncompliant
}

void unnecessaryForEachOnList(List<String> list) {
list.stream().forEach(e -> System.out.println("Element: " + e)); // Noncompliant
}

void necessaryForEachOnParallelStream(Set<String> set) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The noncompliant cases cover Collection, Set, and List, but there is no test for a stream().forEach() call where .stream() returns a custom Stream produced by an overriding implementation. More importantly, there is no test using a concrete ArrayList or other class that resolves through the full ofSubTypes hierarchy.

More critically: Stream.forEach() carries a weaker contract than Iterable.forEach(). Per the JDK spec, Stream.forEach() is "explicitly nondeterministic" even on sequential streams, while Collection.forEach() (via Iterable) iterates in encounter order. Code that relies on stream().forEach() for its documented non-determinism guarantee — even if that's unusual — would have its semantics silently changed. Add a test case comment explaining this known limitation, or document it explicitly in the rule description.

  • Mark as noise

set.parallelStream().forEach(System.out::println); // Compliant
}

void sequentialStreamForEach(Collection<String> col) {
Stream<String> s = col.stream();
s.forEach(System.out::println); // Compliant (the rule ignores this case)
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* SonarQube Java
* Copyright (C) SonarSource Sàrl
* mailto:info AT sonarsource DOT com
*
* You can redistribute and/or modify this program under the terms of
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
* See the Sonar Source-Available License for more details.
*
* You should have received a copy of the Sonar Source-Available License
* along with this program; if not, see https://sonarsource.com/license/ssal/
*/
package org.sonar.java.checks;

import java.util.List;
import org.sonar.check.Rule;
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
import org.sonar.plugins.java.api.semantic.MethodMatchers;
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
import org.sonar.plugins.java.api.tree.Tree;

@Rule(key = "S3706")
Comment thread
sonar-review-alpha[bot] marked this conversation as resolved.
public class StreamForeachCheck extends IssuableSubscriptionVisitor {

private static final MethodMatchers STREAM_METHOD = MethodMatchers.create()
.ofSubTypes("java.util.Collection")
.names("stream")
.addWithoutParametersMatcher()
.build();

private static final MethodMatchers STREAM_FOREACH_METHOD = MethodMatchers.create()
.ofSubTypes("java.util.stream.Stream")
.names("forEach")
.withAnyParameters()
.build();

@Override
public List<Tree.Kind> nodesToVisit() {
return List.of(Tree.Kind.METHOD_INVOCATION);
}

@Override
public void visitNode(Tree tree) {
if (tree instanceof MethodInvocationTree mit && STREAM_FOREACH_METHOD.matches(mit)) {
checkUnnecessaryForEach(mit);
}
}

private void checkUnnecessaryForEach(MethodInvocationTree mitForEach) {
if (mitForEach.methodSelect() instanceof MemberSelectExpressionTree msetForEach
&& msetForEach.expression() instanceof MethodInvocationTree mitStream
&& STREAM_METHOD.matches(mitStream)
&& mitStream.methodSelect() instanceof MemberSelectExpressionTree msetStream) {
reportIssue(msetStream.identifier(), msetForEach.identifier(), "Simplify the code by replacing .stream().forEach() with .forEach().");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* SonarQube Java
* Copyright (C) SonarSource Sàrl
* mailto:info AT sonarsource DOT com
*
* You can redistribute and/or modify this program under the terms of
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
* See the Sonar Source-Available License for more details.
*
* You should have received a copy of the Sonar Source-Available License
* along with this program; if not, see https://sonarsource.com/license/ssal/
*/
package org.sonar.java.checks;

import org.junit.jupiter.api.Test;
import org.sonar.java.checks.verifier.CheckVerifier;

import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;

class StreamForeachCheckTest {
@Test
void test() {
StreamForeachCheck check = new StreamForeachCheck();
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/StreamForeachCheck.java"))
.withCheck(check)
.verifyIssues();
}
Comment thread
NoemieBenard marked this conversation as resolved.

@Test
void testWithoutSemantic() {
StreamForeachCheck check = new StreamForeachCheck();
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/StreamForeachCheck.java"))
.withoutSemantic()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I meant, we should have two tests, one with semantic, one without.

.withCheck(check)
.verifyIssues();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<h2>Why is this an issue?</h2>
<p>There’s no need to invoke <code>stream()</code> on a <code>Collection</code> before a <code>forEach</code> call because each
<code>Collection</code> has its own <code>forEach</code> method.</p>
<h3>Noncompliant code example</h3>
<pre>
identifiers.stream().forEach(System.out::println); // Noncompliant
</pre>
<h3>Compliant solution</h3>
<pre>
identifiers.forEach(System.out::println); // Compliant
</pre>

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"title": "\"stream\" should not be used for Collection \"forEach\" calls",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "2min"
},
"tags": [],
"defaultSeverity": "Minor",
"ruleSpecification": "RSPEC-3706",
"sqKey": "S3706",
"scope": "All",
"quickfix": "unknown"
}
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@
"S3599",
"S3626",
"S3631",
"S3706",
"S3740",
"S3751",
"S3752",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@
"S3599",
"S3626",
"S3631",
"S3706",
"S3740",
"S3751",
"S3752",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ void profile_is_registered_as_expected() {
BuiltInQualityProfilesDefinition.BuiltInQualityProfile actualProfile = profilesPerLanguages.get("java").get("Sonar agentic AI");
assertThat(actualProfile.isDefault()).isFalse();
assertThat(actualProfile.rules())
.hasSize(467)
.hasSize(468)
.extracting(BuiltInQualityProfilesDefinition.BuiltInActiveRule::ruleKey)
.doesNotContainAnyElementsOf(List.of(
"S101",
Expand Down
Loading