-
Notifications
You must be signed in to change notification settings - Fork 451
AddOrUpdateAnnotationAttribute: Add better support for enums #5709
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
6ee58c7
to
203d1b0
Compare
rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java
Outdated
Show resolved
Hide resolved
rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java
Outdated
Show resolved
Hide resolved
rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java
Outdated
Show resolved
Hide resolved
rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java
Outdated
Show resolved
Hide resolved
2dc4c2a
to
0b5df44
Compare
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.
Appreciate the additional tests; the amount of changes in the recipe is a little larger than I would have liked, but went through it in detail and it seems you've been able to clear out some subtle issues by extracting out dedicated methods and working out how the options should combine. Thanks for that; let's wait for a final review before we merge.
What's changed?
What's your motivation?
Make this recipe usable for all possible cases.
Anything in particular you'd like reviewers to focus on?
Next to the better enum support, more refactoring has been done. The biggest change consists of the introduction of separate functions to update the annotation arguments.
Any additional context
Also contains fixes from
addOnly
andappendArray
options for AddOrUpdateAnnotationAttribute recipe #5698as that PR was reverted, because an enabled
addOnly
option did not make changes when something like@Foo(name = "old")
to@Foo(value = "new", name = "old")
was intended, which caused problems downstream. That's fixed in this PR as well (see cbf2069).Checklist