error-prone-support icon indicating copy to clipboard operation
error-prone-support copied to clipboard

BugChecker for Reactor imposter methods

Open werli opened this issue 7 months ago • 0 comments

Problem

Methods returning reactive types but in reality are implemented synchronously give callers a wrong impression of what's going on in a method. See this section of the "Avoiding Reactor Meltdown" talk describing such methods.

The following is such example. imposterMethod() has a reactive method declaration but isn't doing anything that warrants this. This will result in unnecessary upstream subscriptions.

class Example {
  record SetHolder(Set<Integer> values) {}

  static Mono<SetHolder> imposterMethod(Set<Integer> values) {
    return Mono.just(values)
        .filter(Predicate.not(Set::isEmpty))
        .doOnNext(set -> LOG.debug("Found example set {}", set))
        .map(SetHolder::new);
  }
}

Description of the proposed new feature

  • [ ] Support a stylistic preference.
  • [x] Avoid a common gotcha, or potential problem.
  • [x] Improve performance.

I'd like to have a BugChecker that identifies such a construct as a common mistake and expects changes.

Considerations

If the method has a reactive chain that starts with {Mono,Flux}#just and calls no method that results in an inner subscription (e.g. Mono#flatMap()), then we should have a Reactor imposter method and such should be flagged.

Participation

  • [ ] I am willing to submit a pull request to implement this improvement.

I'm opening this issue to start a discussion (a) whether this is desired, and (b) how to achieve such a BugChecker.

I can see it guide new-comers to better usage of reactive operators.

werli avatar Nov 13 '23 14:11 werli