Add Stream templates for CollectionRules and StringRules#2119
Add Stream templates for CollectionRules and StringRules#2119timtebeek wants to merge 1 commit intoPicnicSupermarket:masterfrom
Conversation
|
Looks good. No mutations were possible for these changes. |
📝 WalkthroughWalkthroughTwo new Refaster rules are added: SetOfVarargs in CollectionRules converts Stream.of().collect(toUnmodifiableSet()) to Set.of(), and StringJoinDelimiterVarargs in StringRules replaces Stream join operations with String.join(). Corresponding test cases are added for both rules. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java`:
- Around line 213-227: The SetOfVarargs refaster rule changes duplicate handling
(Stream.of(...).collect(toUnmodifiableSet()) silently deduplicates, whereas
Set.of(...) throws IllegalArgumentException on duplicates); update the
SetOfVarargs block to include an XXX comment noting this behavioral change and
the risk of introducing runtime IllegalArgumentException when input contains
duplicate elements, following the existing comment style used elsewhere in this
file (see similar XXX notes near other rules) and place it immediately above the
rule (or above the AfterTemplate) so reviewers and users are aware of the
caveat.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.javaerror-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StringRules.javaerror-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.javaerror-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.javaerror-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StringRulesTestInput.javaerror-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StringRulesTestOutput.java
| /** Prefer {@link Set#of(Object[])} over more contrived alternatives. */ | ||
| // XXX: Ideally we rewrite both of these expressions directly to `ImmutableSet.of(..)` (and | ||
| // locate this rule in `ImmutableSetRules`), but for now this rule is included as-is for use with | ||
| // OpenRewrite. | ||
| static final class SetOfVarargs<T> { | ||
| @BeforeTemplate | ||
| Set<T> before(@Repeated T elements) { | ||
| return Stream.of(Refaster.asVarargs(elements)).collect(toUnmodifiableSet()); | ||
| } | ||
|
|
||
| @AfterTemplate | ||
| Set<T> after(@Repeated T elements) { | ||
| return Set.of(Refaster.asVarargs(elements)); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there are any existing rules in this repo that document behavioral differences
# around Set.of vs toUnmodifiableSet, or duplicate-handling caveats.
rg -n -C3 'Set\.of|toUnmodifiableSet|duplicate' --type=java -g '!**/test/**' | head -80Repository: PicnicSupermarket/error-prone-support
Length of output: 10039
🏁 Script executed:
# Also get the full context of the file to see the XXX notes pattern mentioned
cat -n error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java | head -250Repository: PicnicSupermarket/error-prone-support
Length of output: 10407
🏁 Script executed:
# Search specifically for XXX or TODO comments in the CollectionRules file to understand documentation style
rg -n 'XXX|TODO' error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.javaRepository: PicnicSupermarket/error-prone-support
Length of output: 1147
🏁 Script executed:
# Search for tests of SetOfVarargs rule
find . -name "*.java" -type f -exec grep -l "SetOfVarargs" {} \;Repository: PicnicSupermarket/error-prone-support
Length of output: 391
🏁 Script executed:
# Check if there are any conditions/matchers on the rule that prevent duplicates
grep -B 5 -A 15 "class SetOfVarargs" error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.javaRepository: PicnicSupermarket/error-prone-support
Length of output: 929
🏁 Script executed:
# Check the test input file for SetOfVarargs tests
grep -B 5 -A 10 "SetOfVarargs\|Stream.of.*toUnmodifiableSet" error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.javaRepository: PicnicSupermarket/error-prone-support
Length of output: 493
🏁 Script executed:
# Check the test output file for SetOfVarargs tests
grep -B 5 -A 10 "SetOfVarargs\|Stream.of.*toUnmodifiableSet\|Set\.of" error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java | head -100Repository: PicnicSupermarket/error-prone-support
Length of output: 3130
Add a comment documenting the duplicate-handling behavioral change in the SetOfVarargs rule.
The rewrite from Stream.of(...).collect(toUnmodifiableSet()) to Set.of(...) introduces a semantic difference: Set.of() throws IllegalArgumentException on duplicate values, while toUnmodifiableSet() deduplicates silently. For example, Stream.of(1, 1).collect(toUnmodifiableSet()) succeeds, but Set.of(1, 1) throws an exception.
Add an XXX comment documenting this caveat, following the pattern established elsewhere in this file (e.g., lines 196–197, 326, 590–605).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java`
around lines 213 - 227, The SetOfVarargs refaster rule changes duplicate
handling (Stream.of(...).collect(toUnmodifiableSet()) silently deduplicates,
whereas Set.of(...) throws IllegalArgumentException on duplicates); update the
SetOfVarargs block to include an XXX comment noting this behavioral change and
the risk of introducing runtime IllegalArgumentException when input contains
duplicate elements, following the existing comment style used elsewhere in this
file (see similar XXX notes near other rules) and place it immediately above the
rule (or above the AfterTemplate) so reviewers and users are aware of the
caveat.
1 participant
Replaces the mistakenly closed
SetOfVarargsandStringJoinDelimiterVarargsRefaster rules #2083As discussed with @rickie