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

Accommodate Java 23 in ReloadableJava21ParserVisitor #4516

Merged
merged 1 commit into from
Sep 23, 2024

Conversation

timtebeek
Copy link
Contributor

What's changed?

com.sun.tools.javac.tree.DocCommentTable#getCommentTree used to return com.sun.tools.javac.tree.DCTree.DCDocComment in Java 21.
As of Java 23 it returns com.sun.source.doctree.DocCommentTree, an interface that was already present and implemented by DCDocComment.

@SuppressWarnings("unchecked") J2 j = (J2) scan(t, formatWithCommentTree(prefix, (JCTree) t, docCommentTable.getCommentTree((JCTree) t)));

This PR changes the usage downstream in formatWithCommentTree to use the interface instead.

What's your motivation?

Have you considered any alternatives or workarounds?

A separate Java 23 parser; decided against for now as we still have work to do on our Java 21 parser, and will likely only target LST releases going forward. That said if support is easy to add, as seen here, we will try to do so.

Any additional context

@Stephan202
Copy link

Hey @timtebeek! Have you considered backporting this fix to rewrite-java-17, as alluded to here? 🙏

@timtebeek
Copy link
Contributor Author

Hey @timtebeek! Have you considered backporting this fix to rewrite-java-17, as alluded to here? 🙏

I didn't think that'd be necessary if rewrite-java-21 is available, but it looks like EPS only uses rewrite-java-17: would it be possible to change that in EPS only? Or would you still prefer the backport to 17?

@timtebeek
Copy link
Contributor Author

timtebeek commented Oct 27, 2024

Pending your response I've already pushed up a PR here in case this is indeed needed; tell me it is and we'll merge:

@Stephan202
Copy link

Hey @timtebeek! Have you considered backporting this fix to rewrite-java-17, as alluded to here? 🙏

I didn't think that'd be necessary if rewrite-java-21 is available, but it looks like EPS only uses rewrite-java-17: would it be possible to change that in EPS only? Or would you still prefer the backport to 17?

Fair question. When I upgrade EPS to rewrite-java-21 and then build with JDK 17.0.13 (which we currently still support), the build fails for two reasons:

  1. A complaint by the Maven Enforcer plugin (can perhaps be worked around, would have to check):
    [WARNING] Rule 6: org.codehaus.mojo.extraenforcer.dependencies.EnforceBytecodeVersion failed with message:
    Found Banned Dependency: org.openrewrite:rewrite-java-21:jar:8.38.0
    Use 'mvn dependency:tree' to locate the source of the banned dependencies.
    
  2. A test failure in StringRulesRecipesTest.stringValueOf:28 ExceptionInInitializer:
    Caused by: java.lang.IllegalStateException: Unable to create a Java parser instance. `rewrite-java-8`, `rewrite-java-11`, `rewrite-java-17`, or `rewrite-java-21` must be on the classpath.
    	at org.openrewrite.java.JavaParser.fromJavaVersion(JavaParser.java:250)
    	at org.openrewrite.java.Assertions.<clinit>(Assertions.java:104)
    ... 66 more
    

(That IllegalStateException message is a bit misleading 😅.)

I could look into having our build require JDK 21, while still targeting and testing against JDK 17 using toolchain support, but a backport would be easier 😅.

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 this pull request may close these issues.

OpenRewrite fails to parse java files when using Java 23
2 participants