-
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
Recipe to convert explicit getters to the Lombok annotation #623
Conversation
src/main/java/org/openrewrite/java/migrate/lombok/ConvertGetter.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/migrate/lombok/ConvertGetter.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/migrate/lombok/ConvertGetter.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/migrate/lombok/LombokUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/migrate/lombok/LombokUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/migrate/lombok/LombokUtils.java
Outdated
Show resolved
Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
src/main/java/org/openrewrite/java/migrate/lombok/ConvertGetter.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/migrate/lombok/ConvertGetter.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.
Great to see this already @timo-a ! I've added some polish to move the field/method name check to utils as well, and added a failing test for the inner class case not yet covered. Figured you might like the puzzle of solving that one, but do let me know if you'd like some more guidance, or me stepping in for the fix there.
Could you step in, please @timtebeek ? I don't have a lot of time at the moment. BTW, you included a name check in |
had copy-pasted from the example recipe
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 the work done here @timo-a ! The tests made it easy to simplify the logic. I quite like what we've arrived at now: just a single pass through method declarations, with doAfterVisit to annotate the field based on type, not simple name.
Thanks, I had no idea you could stack |
What's changed?
Adds new recipe to convert explicit getter methods to the lombok annotation
What's your motivation?
Anything in particular you'd like reviewers to focus on?
Please check especially the naming.
Anyone you would like to review specifically?
@timtebeek as discussed this is the first of several of my lombok recipes to be moved to
rewrite-migrate-java
Have you considered any alternatives or workarounds?
Any additional context
This recipe was written before the introduction of traits. I guess the methods of the utils class could be traits nowadays but I'd need really specific hints to use traits here.
Checklist