-
Notifications
You must be signed in to change notification settings - Fork 90
Omit migration to @Setter annotated field of other annotations are present #672
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
Conversation
List<J.Annotation> annotations = method.getLeadingAnnotations(); | ||
return !annotations.isEmpty() && | ||
!(annotations.size() == 1 && overwriteMatcher.matches(annotations.get(0))); |
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.
Best to use the annotation service here as there might also be annotations on the return type for instance. Leading annotations only has a limited view on annotations.
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.
We don't have a cursor here, it is a util outside a recipe.
2 options I see:
- accept and note for final fix
- move the check into recipe as additional check
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.
As LombokUtils is private it should be possible to add a Cursor parameter, right?
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.
Refactored it in, I don't know if it is the right thing to do considering it's just a fix not a final solution ;)
@timtebeek @knutwannheden The CI is failing because of missing type information at
|
Indeed that's due to Lombok changes in openrewrite/rewrite. I hadn't yet made adjustments here as it seems we're close to a fix upstream. 🙏🏻 |
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, thanks! I've added some slight polish: we tend to retrieve the AnnotationService through a service method defined on the visitor, to prevent repeated instantiation and to allow for instance the Kotlin visitor to swap in an alternative implementation.
What's changed?
What's your motivation?
prevent breaking changes
Anything in particular you'd like reviewers to focus on?
Anyone you would like to review specifically?
Have you considered any alternatives or workarounds?
Any additional context
Checklist