Extend {Flux,Mono}OnErrorComplete Refaster rules
#1939
Conversation
| return Mono.just(1).onErrorComplete().doOnError(e -> {}); | ||
| } | ||
|
|
||
| Mono<Integer> testMonoDoOnErrorClassOnErrorComplete() { |
There was a problem hiding this comment.
I admittedly don't know:
- Why Refaster is not picking this line from the
@BeforeTemplate. - How to fix it 😄
@Stephan202 @rickie , would appreciate the help 🙏
There was a problem hiding this comment.
It's because of the specific signature of this doOnError overload; I'll push a fix. 👍
There was a problem hiding this comment.
Sorry, I still don't quite get it 😅
There was a problem hiding this comment.
This is the diff output:
diff (-expected +actual):
@@ -600,7 +600,7 @@
}
Mono<Integer> testMonoDoOnErrorClassOnErrorComplete() {
- return Mono.just(1).doOnError(IllegalArgumentException.class, e -> {}).onErrorComplete();
+ return Mono.just(1).onErrorComplete().doOnError(IllegalArgumentException.class, e -> {});
}why didn't the @BeforeTemplate match the statement .doOnError(IllegalArgumentException.class, e -> {})
There was a problem hiding this comment.
The signature of that particular overload is:
public final <E extends Throwable> Mono<T> doOnError(Class<E> exceptionType, final Consumer<? super E> onError) {My change introduces a similar type constraint involving E extends Throwable to appease the type matching.
There was a problem hiding this comment.
AHA! Makes sense; thanks a ton 💡
| /** Calling {@link Mono#doOnError(Consumer)} after {@link Mono#onErrorComplete()} is redundant. */ | ||
| static final class MonoDoOnErrorOnErrorComplete<T> { | ||
| @BeforeTemplate | ||
| Mono<T> before(Mono<T> mono, Consumer<? super Throwable> onError) { | ||
| return mono.onErrorComplete().doOnError(onError); | ||
| } | ||
|
|
||
| @AfterTemplate | ||
| Mono<T> after(Mono<T> mono, Consumer<? super Throwable> onError) { | ||
| return mono.doOnError(onError).onErrorComplete(); | ||
| } | ||
| } |
There was a problem hiding this comment.
We're trying to make sure that Refaster rules are behavior preserving. While this rule and the ones below may rewrite the code in a way that was intended, it does change behavior. I'm in favour of doing what we do elsewhere: just drop the redundant operators. Users that meant the other behavior can manually update the code.
There was a problem hiding this comment.
Makes sense 🧠 👍
| return Mono.just(1).onErrorComplete().doOnError(e -> {}); | ||
| } | ||
|
|
||
| Mono<Integer> testMonoDoOnErrorClassOnErrorComplete() { |
There was a problem hiding this comment.
It's because of the specific signature of this doOnError overload; I'll push a fix. 👍
| @BeforeTemplate | ||
| Mono<T> before( | ||
| Mono<T> mono, Class<? extends Throwable> clazz, Consumer<? super Throwable> onError) { | ||
| return mono.onErrorComplete().doOnError(clazz, onError); |
There was a problem hiding this comment.
There's also a third doOnError overload; will add support.
|
Looks good. No mutations were possible for these changes. |
Stephan202
changed the title
{Flux,Mono}OnErrorComplete Refaster rules
|
Thanks @Stephan202 🙏 |
Stephan202
force-pushed
the
mohamedsamehsalah/do-on-error
branch
from
2da97d1 to
0ef65ec
Compare
|
Suggested commit message