-
Notifications
You must be signed in to change notification settings - Fork 332
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
Update model for interfaces extending interfaces #4663
Conversation
rewrite-java/src/test/java/org/openrewrite/java/ExtendInterfaceTest.java
Outdated
Show resolved
Hide resolved
Saving others a click: we fail on line 50 with
|
Thanks for diving in here @rlsanders4 ! Surprised no one else had stumbled upon this before; Now I'm wondering if anyone had worked around this, although recipes for interfaces that extend interfaces might be rare. As a note to myself we should likely add the same changes in the other Java parsers, just so this is consistent when parsed and targeted there on Java 8, 11 and 21 as well. |
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.
Capturing some of what was discussed internally here as well: Yes it's confusing that extends/implements are intuitively the wrong way round, but it matches how these are modeled in the Java compiler as well. In part this might be due to interfaces being able to extend multiple other interfaces, whereas classes can only extend one class, but can implement many interfaces. Changing any part of that now it likely to be painful given that we serialize LSTs. Perhaps the best way forward is to document this seeming inversion for interfaces on the getExtends and getImplements methods for class declarations.
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 working through this with us to improve the experience for folks going forward!
What's changed?
Update the parser model to correctly represent an interface extending another interface.
What's your motivation?
Currently, when a Java interface extends another interface it is represented in the resulting
J.ClassDeclaration
as an 'implements' rather than an 'extends'. This can cause confusion when authoring custom recipes, because instead of accessing the extended interface usingJ.ClassDeclaration.getExtends()
it must instead be accesed usingJ.ClassDeclaration.getImplements()
.For example:
Parent interface
Child interface
Once the child interface has been parsed, in a
visitClassDeclaration()
visitor we would expect to be able to access the extended parent interface withcd.getExtends()
. However, that currently returns null, and you must access it instead withcd.getImplements()
.Checklist