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

Rewrite `Optional.of(A).orElse(B)` to `requireNonNull(A)`

Open rickie opened this issue 1 year ago • 3 comments

Problem

As @EnricSala mentioned in https://github.com/PicnicSupermarket/error-prone-support/issues/364, we might want to investigate rewriting Optional.of(A).orElse(B) to requireNonNull(A). This will probably require a BugChecker as we probably want to provide two fixes, have to do a more extensive analysis, and are making some non behavior preserving changes.

We should also consider the Optional#orElseGet case.

Description of the proposed new feature

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

I would like to rewrite the following code:

Optional.of(1).orElse(2); // Holds for non-nullable things.
Optional.of(null).orElse(2); // Holds for nullable things.

to:

requireNonNull(1);  // Holds for non-nullable things.
Optional.ofNullable(null).orElse(2); // Holds for nullable things.

rickie avatar Jan 02 '23 09:01 rickie

Since the nullability analysis isn't perfect, perhaps we should emit both suggested fixes in either case, but in a different order. Just like in FluxFlatMapUsage.

Stephan202 avatar Jan 02 '23 13:01 Stephan202

Hi @rickie @Stephan202 , Is this issue still valid?

BLasan avatar Mar 24 '24 05:03 BLasan

Hey @BLasan! I believe so; I don't readily find code that would already cover this.

As mentioned, we worry a bit that Refaster solutions to this issue may produce too many false positives. So that would then argue for a BugChecker, but that's quite a bit more work, and still raises questions about which (if any) nullness analysis to apply, and which suggestion to prefer in which context.

If you're up for this we're happy to review a PR! If you're looking for more of a bite-size OSS contribution snack, perhaps we should look for another ticket. Which reminds me: did you see @rickie's suggestion for you here? :smile:

Stephan202 avatar Mar 24 '24 08:03 Stephan202