Skip to content

Introduce FluxCollectToImmutableList Refaster rule #570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Apr 13, 2023
Merged

Introduce FluxCollectToImmutableList Refaster rule #570

merged 4 commits into from
Apr 13, 2023

Conversation

mlrprananta
Copy link
Contributor

@mlrprananta mlrprananta commented Apr 7, 2023 •
edited by rickie
Loading

Based on internal inquiry

Suggested commit message:

Introduce `FluxCollectToImmutableList` Refaster rule (#570)

@github-actions
Copy link

github-actions bot commented Apr 7, 2023

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@mlrprananta mlrprananta requested review from Stephan202 and rickie April 7, 2023 09:57
@mlrprananta mlrprananta mentioned this pull request Apr 7, 2023
Merged
@rickie rickie added this to the 0.10.0 milestone Apr 7, 2023
rickie
rickie approved these changes Apr 7, 2023
View reviewed changes
Copy link
Member

@rickie rickie left a comment

Choose a reason for hiding this comment

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

Added a commit with an extra rule :). Changes LGTM!

Slightly tweaked the suggested commit message.

error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java
*/
static final class FluxCollectListToCollectImmutableList<T> {
@BeforeTemplate
Mono<List<T>> before(Flux<T> flux) {
Copy link
Member

Choose a reason for hiding this comment

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

We can add: flux.collect(toImmutableList()).map(ImmutableList::copyOf);.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, the ((Mono<ImmutableList<T>>) flux).map(ImmutableList::copyOf) simplification should be in a rule of its own. (And is in fact a special case of a large range of rules.)

@github-actions
Copy link

github-actions bot commented Apr 7, 2023

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@github-actions
Copy link

github-actions bot commented Apr 7, 2023

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 force-pushed the mlrprananta/flux-collectlist-to-collect branch from eb80bb2 to 013ba14 Compare April 7, 2023 18:21
Stephan202
Stephan202 approved these changes Apr 7, 2023
View reviewed changes
Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

Rebased and added a commit.

Suggested commit message:

Introduce `FluxCollectToImmutableList` Refaster rule (#570)

And extend `MonoIdentity` to simplify `mono.map(ImmutableList::copyOf)`
expressions where possible, as the new rule may introduce such cases.

error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java Outdated
Comment on lines 1183 to 1192
Mono<List<T>> after(Flux<T> flux) {
return flux.collect(toImmutableList());
Copy link
Member

Choose a reason for hiding this comment

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

Let's make explicit that strictly speaking this expression has a slightly different return type.

error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java
*/
static final class FluxCollectListToCollectImmutableList<T> {
@BeforeTemplate
Mono<List<T>> before(Flux<T> flux) {
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, the ((Mono<ImmutableList<T>>) flux).map(ImmutableList::copyOf) simplification should be in a rule of its own. (And is in fact a special case of a large range of rules.)

@github-actions
Copy link

github-actions bot commented Apr 7, 2023

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mlrprananta mlrprananta force-pushed the mlrprananta/flux-collectlist-to-collect branch from 013ba14 to 432ba2d Compare April 11, 2023 07:54
@mlrprananta
Copy link
Contributor Author

Rebased and resolved ✅

@github-actions
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@rickie rickie force-pushed the mlrprananta/flux-collectlist-to-collect branch from 432ba2d to 1ef57fb Compare April 13, 2023 06:04
@rickie
Copy link
Member

rickie commented Apr 13, 2023

Changes LGTM! Will merge once 🟢.

@github-actions
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@rickie rickie changed the title Prefer Flux#collect(toImmutableList()) over Flux#collectList() Introduce FluxCollectToImmutableList Refaster rule Apr 13, 2023
@rickie rickie merged commit 6514236 into master Apr 13, 2023
@rickie rickie deleted the mlrprananta/flux-collectlist-to-collect branch April 13, 2023 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rickie rickie rickie approved these changes

@Stephan202 Stephan202 Stephan202 approved these changes

Assignees

No one assigned

Labels

new feature

Milestone

0.10.0

Development

Successfully merging this pull request may close these issues.

3 participants

@mlrprananta @rickie @Stephan202