Skip to content
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

rewrite-java-21 (8.36.0) fails on Java 23 #4540

Closed
Col-E opened this issue Oct 2, 2024 · 2 comments · Fixed by #4555
Closed

rewrite-java-21 (8.36.0) fails on Java 23 #4540

Col-E opened this issue Oct 2, 2024 · 2 comments · Fixed by #4555
Assignees
Labels
bug Something isn't working parser-java

Comments

@Col-E
Copy link

Col-E commented Oct 2, 2024

Overview

#4534 was closed under the assumption that the reporter's problem was their project was picking up the Java 17 parser and not the patched Java 21 parser (with #4516))

Unfortunately, even with the provided patched rewrite-java-21 parsing code on Java 23 fails with an exception:

java.lang.NoSuchMethodError: 'com.sun.tools.javac.tree.DCTree$DCDocComment com.sun.tools.javac.tree.DocCommentTable.getCommentTree(com.sun.tools.javac.tree.JCTree)'
  org.openrewrite.java.isolated.ReloadableJava21ParserVisitor.convert(ReloadableJava21ParserVisitor.java:1667)
  org.openrewrite.java.isolated.ReloadableJava21ParserVisitor.visitCompilationUnit(ReloadableJava21ParserVisitor.java:538)
  org.openrewrite.java.isolated.ReloadableJava21ParserVisitor.visitCompilationUnit(ReloadableJava21ParserVisitor.java:74)
  com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:625)
  com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:92)

The problem

The line in question is here: ReloadableJava21ParserVisitor.java#L1667

  • The rewrite-java-21 module is sensibly compiled against JDK 21
  • This means the reference to DocCommentTable.getCommentTree is expecting the return type to be what was defined in the JDK 21 code
  • Running rewrite-java-21 on JDK 23 crashes because the getCommentTree changed the return type in a non-binary compatible way
    • JDK 22 - DocCommentTable.java#L69 - return type is com.sun.tools.javac.tree.DCTree.DCDocComment
    • JDK 23 - DocCommentTable.java#L69 - return type is com.sun.source.doctree.DocCommentTree
    • Compiling against 22 or below makes method reference with a return type DCDocComment which cannot be coerced to be the looser DocCommentTree that would be compatible on newer JDK's

Solutions

  • Create a rewrite-java-23 module
  • Compile rewrite-java-21 against JDK 23 but with a target version of Java 21
    • This should emit a method reference to the method with a looser return type, and since the older Java code's return value implements the interface type of the new methods return type it should be compatible
    • Assuming the rewrite-java-21 is thoroughly covered, any other possible changes from targeting the newer API (with a compatible bytecode output though) that are incompatible should be caught by the tests
  • Use reflection to support use from both <23 and >=23

Reflection solution:

// Lookup by name does not require strict contract with return type
Method getCommentTreeMethod = DocCommentTable.class.getMethod("getCommentTree", JCTree.class);

// DCTree.DCDocComment implements DocCommentTree so the cast is safe in both 22- and 23+
DocCommentTree value = (DocCommentTree) getCommentTreeMethod.invoke(table, (Object) null);

Workarounds

  • Run your application on Java 22 or lower
@timtebeek
Copy link
Contributor

Thanks a lot for pulling together all the relevant context here @Col-E , and outlining possible next steps. I'll pass this along to colleagues as I'm traveling, but if you want to push up the reflection fix that ought to be manageable to review on the go. I don't see us adding a Java 23 specific parser for now, as it's quite a bit to maintain for a non-LTS version.

@timtebeek
Copy link
Contributor

I've pushed up this proposal based on your last suggestion above

I hope I understood you correctly @Col-E ! :)

@timtebeek timtebeek self-assigned this Oct 6, 2024
@timtebeek timtebeek moved this from Backlog to Ready to Review in OpenRewrite Oct 6, 2024
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Oct 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working parser-java
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants