Introduce Mono{Empty,Just,JustOrEmpty} Refaster rules
#385
Conversation
|
Looks good. No mutations were possible for these changes. |
| static final class MonoEmpty<T> { | ||
| @BeforeTemplate | ||
| Mono<T> before() { | ||
| return Refaster.anyOf(Mono.just(null), Mono.justOrEmpty(null)); |
There was a problem hiding this comment.
I'm admittedly not fully sure whether we should actually cover Mono.just(null), so I added it as part of an extra commit. It's such an obvious mistake that a Bug Check may make more sense.
There was a problem hiding this comment.
Hmm, I think we should indeed omit this one, since Mono.just(null) is actually a bug, triggering an NPE. Ideally NullAway would flag it (and we should look into enabling NullAway internally).
OTOH, the suggested fix for Mono.just(null) is likely Mono.empty(), so in that sense its inclusion may make sense 🤷.
For now I'll push a commit which replaces it with another applicable case: Mono.justOrEmpty(Optional.empty()).
| static final class MonoEmpty<T> { | ||
| @BeforeTemplate | ||
| Mono<T> before() { | ||
| return Refaster.anyOf(Mono.just(null), Mono.justOrEmpty(null)); |
There was a problem hiding this comment.
Hmm, I think we should indeed omit this one, since Mono.just(null) is actually a bug, triggering an NPE. Ideally NullAway would flag it (and we should look into enabling NullAway internally).
OTOH, the suggested fix for Mono.just(null) is likely Mono.empty(), so in that sense its inclusion may make sense 🤷.
For now I'll push a commit which replaces it with another applicable case: Mono.justOrEmpty(Optional.empty()).
| @BeforeTemplate | ||
| @SuppressWarnings("NullAway") | ||
| Mono<T> before(@Nullable T value) { | ||
| return Mono.justOrEmpty(Refaster.anyOf(Optional.of(value), Optional.ofNullable(value))); | ||
| } |
There was a problem hiding this comment.
It seems like we can avoid @SuppressWarnings("NullAway") two ways:
- Moving the
Optional.of(value)to a separate@BeforeTemplatemethod. - Just omit the
@Nullableannotations, since they don't impact Refaster anyway.
Right now (2) seems simplest. If later we flag Optional.ofNullable(v) for non-@Nullable values v, then we should use (1).
There was a problem hiding this comment.
But... actually we should have a separate rule for Mono.just, which gives us (1) for free :)
error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java
Outdated
Show resolved
Hide resolved
|
Looks good. No mutations were possible for these changes. |
rickie
force-pushed
the
werli/mono-empty-rules
branch
from
0c9d9c3 to
27f5312
Compare
|
Rebased. Will merge once 🟢. |
Take two! |
Should never happen in practice, but it's not a compile time error.
rickie
force-pushed
the
werli/mono-empty-rules
branch
from
27f5312 to
7f9ec03
Compare
|
Looks good. No mutations were possible for these changes. |
rickie
changed the title
Mono{JustOr}Empty Refaster rulesMono{Empty,Just,JustOrEmpty} Refaster rules
After #384 I noticed a few more interactions between the Optional and Mono API we can simplify.
Pretty simple cases, but why not cover them?
Suggested commit message: