SONARJAVA-6273 S3706: "stream" should not be used for Collection "forEach" calls

[NoemieBenard]

Implementation of rule S3706.

[hashicorp-vault-sonar-prod]

SONARJAVA-6274

[sonar-review-alpha]

Summary

This PR implements SonarQube rule S3706: "stream" should not be used for Collection "forEach" calls.

The rule detects when code unnecessarily chains .stream().forEach() on a Collection instead of calling .forEach() directly. Since all Collection types already have their own forEach() method, the intermediate stream() call is redundant and adds no value.

Changes include:

• New check class using method matchers to identify the pattern (.stream() on Collection followed by .forEach() on Stream)
• Test cases covering the rule scope: detects violations on Collection, Set, List; correctly ignores .parallelStream().forEach(), filtered streams, and stored stream variables
• Rule documentation (HTML/JSON) with rationale and code examples
• Integration tests showing 4 real violations found in SonarQube codebase
• Addition of rule to both Sonar Way and Agentic AI quality profiles

What reviewers should know

Where to focus:

1. Core logic (StreamForeachCheck.java:54-61): The method matchers correctly identify .stream() on Collection subtypes followed by .forEach() on Stream. The check stops at 2 levels deep (direct chaining), which intentionally avoids false positives when the stream is assigned to a variable.

2. Test coverage (StreamForeachCheck.java test file): Review both noncompliant cases (lines 10, 23, 27) and the edge cases marked "Compliant" (lines 20, 31, 36). Note the comment on line 37 explaining the design choice about stored streams.

3. Error reporting (line 59): The issue is anchored to both method names—.stream to .forEach—making the problem and fix clear.

4. False positive avoidance: The check correctly does NOT flag parallelStream() (different semantics) or cases where the stream is stored first, which aligns with the simple "direct chaining" pattern this rule targets.

5. Profile integration: Verify the rule count increase from 467 to 468 in JavaAgenticWayProfileTest.java matches the addition to both profiles.

---

• Generate Walkthrough
• Generate Diagram

🗣️ Give feedback

[sonar-review-alpha]

The implementation logic is correct and well-scoped, but there is a critical missing artifact that will break the build.

🗣️ Give feedback

[sonar-review-alpha]

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

[tomasz-tylenda-sonarsource]

Since stream() doesn't take any parameters, could we make this match stricter?

[tomasz-tylenda-sonarsource]

You can try something along these lines:

    if (mitForEach.methodSelect() instanceof MemberSelectExpressionTree mset
      && mset.expression() instanceof MethodInvocationTree mitStream
      && STREAM_METHOD.matches(mitStream)) {
        reportIssue(mitStream, "Replace unnecessary call to .stream().forEach() by .forEach()");
      }
    }

Also, we can call strings streamMit and forEachMit or something like this.

[tomasz-tylenda-sonarsource]

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

[tomasz-tylenda-sonarsource]

If we are pointing at collection.stream() then maybe the message should be Remove unnecessary call to .stream() ? It's possible to tweak the location of the issue quite freely with some adjustments to report issue.

Btw, I told you that quckfixes are difficult. This is generally true, but in this case it's not too difficult, so maybe we could do it in a follow-up PR later.

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
90.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[sonar-review-alpha]

LGTM! ✅

The previously flagged issues around test coverage (concrete Collection subtypes, ArrayList) and the missing documentation about the semantic difference between Stream.forEach() (nondeterministic) and Collection.forEach() (encounter-order) are still unresolved — neither the test file nor the rule description were updated in this commit.

🗣️ Give feedback