-
Notifications
You must be signed in to change notification settings - Fork 297
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
ChangePackage fails to migrate annotation argument enum in package-info #4174
Conversation
Thanks a lot! Runnable tests are my favorite way to show a bug; did you already try to step through with a debugger to see if you could pinpoint the source of the issue? |
No, I haven't found the source of the issue. It seems to find that the types are in use, but the changes are not applied. I can maybe check it out later if you haven't already found it! |
The fix was really simple when I finally found what was going on 🙂 The |
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.
Yes makes sense when you see the issue; thanks for digging in and providing the fix!
6b4b3f5
to
b7707bb
Compare
I'm not really sure of why, but some tests were failing if Instead, I updated the proposed fix to only update the annotations in the same way as is done inside the super method. This seems to work with all the existing tests at least. |
dcd7317
to
60c6ca8
Compare
What's changed?
I found this issue describing problems with migrating annotations in
package-info.java
. We seem to still have this problem.In this MR, I have added a (failing) test showing our issue.
Our actual issue is regarding applying jakarta migration for this class:
We have made it work using
org.openrewrite.java.ChangeType
for our migration, but would prefer iforg.openrewrite.java.ChangePackage
worked for this.Anything in particular you'd like reviewers to focus on?
Please note that this does not fix the issue, only adds a test case to it :-)
Any additional context
Checklist