Skip to content

SONARJAVA-6289 Fix QG / Fix issues with stream().foreach(...)#5585

Merged
rombirli merged 1 commit intomasterfrom
rombirli/SONARJAVA-6289
Apr 28, 2026
Merged

SONARJAVA-6289 Fix QG / Fix issues with stream().foreach(...)#5585
rombirli merged 1 commit intomasterfrom
rombirli/SONARJAVA-6289

Conversation

@rombirli
Copy link
Copy Markdown
Contributor

No description provided.

@hashicorp-vault-sonar-prod
Copy link
Copy Markdown
Contributor

hashicorp-vault-sonar-prod Bot commented Apr 28, 2026

SONARJAVA-6289

@sonar-review-alpha
Copy link
Copy Markdown
Contributor

sonar-review-alpha Bot commented Apr 28, 2026

Summary

This PR removes unnecessary .stream() chains from forEach() operations and replaces a stream+forEach pattern with direct collection operations. The changes address three instances where streams were being used inefficiently:

  1. ValueBasedUtilsTest: Changed members.stream().forEach(...) to members.forEach(...)
  2. SonarComponents: Changed undefinedTypes.stream().forEach(...) to undefinedTypes.forEach(...)
  3. DefaultJavaFileScannerContext: Replaced mainLocations.subList(...).stream().forEach(completedSecondaries::add) with completedSecondaries.addAll(mainLocations.subList(...))

These are performance and clarity improvements—using the built-in forEach() method on collections and addAll() is simpler and more efficient than creating an unnecessary stream layer.

What reviewers should know

What to look for:

  • All three changes follow the same pattern: eliminating redundant streams
  • The functional behavior is identical—this is purely a refactoring for code quality
  • Changes affect one test file and two production files

Review strategy:

  • Verify that each change maintains the original behavior (it should—they're direct simplifications)
  • Check if there are other similar patterns in the codebase that might need the same treatment
  • Confirm the modifications compile and tests still pass

  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@sonarqube-next
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@sonar-review-alpha sonar-review-alpha Bot left a comment

Choose a reason for hiding this comment

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

LGTM! ✅

Clean, straightforward PR. All three changes are behaviorally equivalent and correct:

  • completedSecondaries is a fresh ArrayList (line 106), so addAll is safe and idiomatic.
  • undefinedTypes is a Set<JProblem> — iteration order was never guaranteed by either approach.
  • The (VariableTree) cast in the test is safe: the fixture class contains only field declarations, so members will never contain a non-VariableTree element.

🗣️ Give feedback

// handle other main locations as secondaries with same message
mainLocations.subList(1, mainLocations.size())
.stream()
.forEach(completedSecondaries::add);
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.

@NoemieBenard Fyi, this use of forEach(collection::add) is another thing that may benefit from simplification.

No action needed.

@rombirli rombirli merged commit 2f9782b into master Apr 28, 2026
17 checks passed
@rombirli rombirli deleted the rombirli/SONARJAVA-6289 branch April 28, 2026 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants