-
Notifications
You must be signed in to change notification settings - Fork 51
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
Support fluent chains in RemoveMethodCallVisitor #340
Support fluent chains in RemoveMethodCallVisitor #340
Conversation
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.
Some suggestions could not be made:
- src/main/java/org/openrewrite/staticanalysis/RemoveMethodCallVisitor.java
- lines 25-25
Thanks for the measured improvement here! Indeed would say that this is worthwhile to add into the visitor. Could you let me know when you're ready for review? |
@timtebeek Cool. Feel free to have a look, I was just waiting for the tests to go through in CI. |
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 for the controlled extension to what the visitor can cover! I've added some small polishing commits, but other than that good to go.
What's changed?
RemoveMethodCallVisitor
can now remove method invocations in a fluent chain, assuming that the select is also a method invocation and of the same return type. In particular, this works for the builder pattern.What's your motivation?
Anything in particular you'd like reviewers to focus on?
Is partial support for this use-case worth the added complexity?
Anyone you would like to review specifically?
@timtebeek
Have you considered any alternatives or workarounds?
Any additional context
Checklist