Introduce {Mono,Flux}OnErrorComplete Refaster rules
#273
Conversation
chamil-prabodha
requested review from
Stephan202,
ibabiankou,
Badbond,
oxkitsune,
rickie and
nathankooij
rickie
changed the title
.onErrorResume(e -> Mono.empty()) to .onErrorComplete().onErrorResume(e -> Mono.empty()) to .onErrorComplete()
There was a problem hiding this comment.
Congrats on the first EPS PR @chamil-prabodha ! 🚀
Added a commit and left some comments.
Suggested commit message:
Prefer `{Mono,Flux}#onErrorComplete` over `{Mono,Flux}#onErrorResume(Function)` (#273)
Alternative:
Prefer `{Mono,Flux}#onErrorComplete` over `onErrorResume` with `{Mono,Flux#empty}` (#273)
One thing:
- Usually we sort lexicographical but I feel Mono and Flux are a bit like min and max, which means we order like proposed.
| @@ -271,6 +271,32 @@ Flux<S> after(Flux<T> flux) { | |||
| } | |||
| } | |||
|
|
|||
| /** Prefer {@link Flux#onErrorComplete()} over more contrived alternatives */ | |||
| static final class FluxOnErrorComplete<T> { | |||
There was a problem hiding this comment.
Although the added templates are sorted lexicographical, I see that within this class we usually list the Mono before the Flux, so I'm swapping them around.
| @@ -271,6 +271,32 @@ Flux<S> after(Flux<T> flux) { | |||
| } | |||
| } | |||
|
|
|||
| /** Prefer {@link Flux#onErrorComplete()} over more contrived alternatives */ | |||
There was a problem hiding this comment.
| /** Prefer {@link Flux#onErrorComplete()} over more contrived alternatives */ | |
| /** Prefer {@link Flux#onErrorComplete()} over more contrived alternatives. */ |
Javadoc should end with a dot 😄.,
| @@ -95,6 +95,14 @@ Flux<Number> testFluxCast() { | |||
| return Flux.just(1).map(Number.class::cast); | |||
| } | |||
|
|
|||
| Flux<Object> testFluxOnErrorComplete() { | |||
| return Flux.error(new IllegalStateException()).onErrorResume(e -> Flux.empty()); | |||
There was a problem hiding this comment.
We can simplify the example and as a result we don't need to use Flux<Object>, we can go with Flux<Integer>.
| } | ||
| } | ||
|
|
||
| /** Prefer {@link Mono#onErrorComplete()} over more contrived alternatives */ |
There was a problem hiding this comment.
I was doubting to make the description more specific. However, it's now the same as the Javadoc of surrounding templates so decided to leave it as is.
…o.empty())` to `.onErrorComplete()` Add a new refaster template rule that rewrites `.onErrorResume(e -> Mono.empty())` to `.onErrorComplete()`. This is applied to `.onErrorResume(e -> Flux.empty())` as well.
Stephan202
force-pushed
the
chamil-prabodha/PSM-1627
branch
from
990bc0f to
a8e8c58
Compare
| /** Prefer {@link Mono#onErrorComplete()} over more contrived alternatives. */ | ||
| static final class MonoOnErrorComplete<T> { | ||
| @BeforeTemplate | ||
| Mono<T> before(Mono<T> mono) { | ||
| return mono.onErrorResume(e -> Mono.empty()); | ||
| } | ||
|
|
||
| @AfterTemplate | ||
| Mono<T> after(Mono<T> mono) { | ||
| return mono.onErrorComplete(); | ||
| } | ||
| } | ||
|
|
||
| /** Prefer {@link Flux#onErrorComplete()} over more contrived alternatives. */ | ||
| static final class FluxOnErrorComplete<T> { | ||
| @BeforeTemplate | ||
| Flux<T> before(Flux<T> flux) { | ||
| return flux.onErrorResume(e -> Flux.empty()); | ||
| } | ||
|
|
||
| @AfterTemplate | ||
| Flux<T> after(Flux<T> flux) { | ||
| return flux.onErrorComplete(); | ||
| } | ||
| } |
There was a problem hiding this comment.
There are also templates (such as EqualityTemplates.Negation) in which we combine @BeforeTemplates with incompatible types. We could do the same here. It feels kinda hacky, so I think it's good to keep this code as-is, but that does raise questions about the other templates. (Something to revisit once we add a check that validates the "sanity" of a Refaster rule.)
| static final class FluxOnErrorComplete<T> { | ||
| @BeforeTemplate | ||
| Flux<T> before(Flux<T> flux) { | ||
| return flux.onErrorResume(e -> Flux.empty()); |
There was a problem hiding this comment.
We can also add a case for Mono.empty() here.
| @@ -301,7 +301,7 @@ Flux<T> after(Flux<T> flux) { | |||
| static final class PublisherProbeEmpty<T> { | |||
| @BeforeTemplate | |||
| PublisherProbe<T> before() { | |||
| return Refaster.anyOf(PublisherProbe.of(Mono.empty()), PublisherProbe.of(Flux.empty())); | |||
There was a problem hiding this comment.
Aiii, not sure how I misses this 😓.
rickie
changed the title
.onErrorResume(e -> Mono.empty()) to .onErrorComplete(){Mono,Flux}OnErrorComplete Refaster rules
Add a new refaster template rule that rewrites
.onErrorResume(e -> Mono.empty())to.onErrorComplete(). This is applied to.onErrorResume(e -> Flux.empty())as well.