error-prone-support
error-prone-support copied to clipboard
Rewrite `Optional.of(A).orElse(B)` to `requireNonNull(A)`
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.
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
.
Hi @rickie @Stephan202 , Is this issue still valid?
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: