Introduce MonoIdentity and MonoThen Refaster rules
#405
Conversation
|
Looks good. No mutations were possible for these changes. |
| } | ||
|
|
||
| /** Don't unnecessarily call {@link Mono#then()} on a {@code Mono<Void>} . */ | ||
| @SuppressWarnings("VoidMissingNullable") |
There was a problem hiding this comment.
Introduced this because I got into a fight with NullAway:
[NullAway] dereferenced expression mono is @Nullable
"mono" was nullable:
@BeforeTemplate
Mono<@Nullable Void> before(Mono<@Nullable Void> mono) {
return mono.then();
}
regardless, I wasn't really sure why we need @Nullable on the Void type in this context
There was a problem hiding this comment.
IIUC this is a bug in NullAway; indeed their support for generics is limited. Let's add the @Nullable annotations (they are correct) but also keep the suppression (only on this class).
| } | ||
|
|
||
| Mono<Void> testMonoVoidThen() { | ||
| return Mono.just("foo").then().then(); |
There was a problem hiding this comment.
The most concise way I could think of creating a Mono<Void>, not the prettiest
There was a problem hiding this comment.
How about Mono.<Void>empty()? :)
pacbeckh
force-pushed
the
pacbeckh/mono-voidthen-fluxthen
branch
from
7c22188 to
d4145a5
Compare
|
Looks good. No mutations were possible for these changes. |
There was a problem hiding this comment.
Thanks for your contribution @pacbeckh!
I rebased and added a commit. Perhaps the MonoIdentity rule should now be moved (we have some "fuzzy logic" for that), but as-is the diff is easier to review.
Suggested commit message:
Introduce `MonoIdentity` and `MonoThen` Refaster rules (#405)
The `MonoIdentity` rule is a generalization of the existing
`MonoSwitchIfEmptyOfEmptyPublisher` rule.
| } | ||
|
|
||
| /** Don't unnecessarily call {@link Mono#then()} on a {@code Mono<Void>} . */ | ||
| @SuppressWarnings("VoidMissingNullable") |
There was a problem hiding this comment.
IIUC this is a bug in NullAway; indeed their support for generics is limited. Let's add the @Nullable annotations (they are correct) but also keep the suppression (only on this class).
| @AfterTemplate | ||
| Mono<Void> after(Mono<Void> mono) { | ||
| return mono; | ||
| } |
There was a problem hiding this comment.
We can combine this rule with MonoSwitchIfEmptyOfEmptyPublisher above, as we generally try to have one rule for each "target shape" in an @AfterTemplate.
| @@ -675,6 +675,34 @@ Flux<T> after(Mono<T> mono) { | |||
| } | |||
| } | |||
|
|
|||
| /** Don't unnecessarily convert {@link Mono} to {@link Flux}. */ | |||
| @SuppressWarnings("VoidMissingNullable") | |||
| static final class MonoFluxThen<T> { | |||
There was a problem hiding this comment.
We haven't fully migrated yet, but the naming standard we're thinking of settling on would have this rule be named MonoThen, derived from the code in the @AfterTemplate.
| } | ||
|
|
||
| Mono<Void> testMonoVoidThen() { | ||
| return Mono.just("foo").then().then(); |
There was a problem hiding this comment.
How about Mono.<Void>empty()? :)
|
Looks good. No mutations were possible for these changes. |
rickie
changed the title
MonoFluxThen and MonoVoidThen Refaster rulesMonoIdentity and MonoThen Refaster rules
We came across this pattern in our code base after some (automatic) refactorings.
.flux().then()doesn't add anything, similarly to calling.thenon aMono<Void>.Introduced two rules for both steps
Suggested commit message: