Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite Optional.of(A).orElse(B) to requireNonNull(A) #436

Open
1 of 3 tasks
rickie opened this issue Jan 2, 2023 · 3 comments
Open
1 of 3 tasks

Rewrite Optional.of(A).orElse(B) to requireNonNull(A) #436

rickie opened this issue Jan 2, 2023 · 3 comments
Labels

Comments

@rickie
Copy link
Member

rickie commented Jan 2, 2023

Problem

As @EnricSala mentioned in #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

  • 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 rickie added good first issue Good for newcomers new feature labels Jan 2, 2023
@Stephan202
Copy link
Member

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.

@BLasan
Copy link
Contributor

BLasan commented Mar 24, 2024

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

@Stephan202
Copy link
Member

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? 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants