-
Notifications
You must be signed in to change notification settings - Fork 77
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
Optional<Stream>.filter(Optional::isPresent).map(Optional::get)
to Java 11 .flatMap(Optional::stream)
#315
Optional<Stream>.filter(Optional::isPresent).map(Optional::get)
to Java 11 .flatMap(Optional::stream)
#315
Conversation
Hi @timtebeek! What happens now? Will someone come and review? |
src/main/java/org/openrewrite/java/migrate/util/OptionalStreamRecipe.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Impressed with the work you've done here! Really like the attention to detail in handling edge cases, and the suite of tests to show what's correctly handled. I've added some comments that I think would improve this PR still, and bring it closer to a merge.
Let me know if you'd be open to making those changes still!
src/main/java/org/openrewrite/java/migrate/util/OptionalStreamRecipe.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/migrate/util/OptionalStreamRecipe.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/migrate/util/OptionalStreamRecipe.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/migrate/util/OptionalStreamRecipe.java
Outdated
Show resolved
Hide resolved
Optional<Stream>.filter(Optional::isPresent).map(Optional::get)
to Java 11 .flatMap(Optional::stream)
src/main/java/org/openrewrite/java/migrate/util/OptionalStreamRecipe.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/migrate/util/OptionalStreamRecipeTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this addition @aslaski2 ! Let us know how this works for you; you can try it out through app.moderne.io soon after the merge.
Fixes #313
Problem
Java 8 Optional class didn't provide
::stream
method. As a result of this unfortunate mistake a popular idiom was created to flatten a Stream of Optionals:.filter(Optional::isPresent).map(Optional::get)
.This was corrected in Java 9 and the idiom may be replaced with
.flatMap(Optional::stream)
Anything in particular you'd like reviewers to focus on?
This is my first adventure with OpenRewrite, please check everything carefully.
Checklist
./gradlew licenseFormat