-
Notifications
You must be signed in to change notification settings - Fork 482
Init of Java 25 parser #5942
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
Init of Java 25 parser #5942
Conversation
rewrite-java/src/main/java/org/openrewrite/java/JavaParser.java
Outdated
Show resolved
Hide resolved
14f83d7 to
898041a
Compare
rewrite-java-25/src/test/java/org/openrewrite/java/Java25JavadocTest.java
Outdated
Show resolved
Hide resolved
rewrite-java-25/src/test/resources/META-INF/rewrite/classpath/jackson-annotations-2.17.1.jar
Outdated
Show resolved
Hide resolved
7e228f8 to
e55442c
Compare
|
I think the rewrite-gradle-plugin can't handle Java 25 just yet, hence why it's spewing out so many diffs, there's a concurrency issue. I can reproduce it locally |
|
There's a bit of a hack still in the Java 21 parser, which we wouldn't need there anymore following the new matching of Java versions with LTS parsers. Can we remove that handling from the Java 21 parser? Lines 1791 to 1794 in 8ce5308
And I say that as the one that added it; ideally we push folks to the next LTS parser as opposed to back porting based on what folks happen to have on the classpath. |
| // The AST contained unnecessary whitespace for Javadoc, and they got rid of this with Java 25. | ||
| // So now have to manually account for this. | ||
| if (i+1 <= node.length() -1 && node.charAt(i+1) != source.charAt(cursor) && Character.isWhitespace(source.charAt(cursor))) { | ||
| text.append(whitespaceBeforeAsString()); | ||
| } |
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.
These lines are new as compared to the Java 21 parser; they look fine to me; figured call these out for wider review as the rest of the file is identical between 21 and 25.
|
Similarly to above, we can revert this ward that was added to the Java 21 parser to support Java 24: rewrite/rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21TypeMapping.java Lines 780 to 785 in 8ce5308
We'd want to revert to the simple |
.github/workflows/receive-pr.yml
Outdated
| uses: openrewrite/gh-automation/.github/workflows/receive-pr.yml@main | ||
| uses: openrewrite/gh-automation/.github/workflows/receive-pr.yml@java25 | ||
| with: | ||
| java_version: | | ||
| 25-ea | ||
| 21 | ||
| recipe: 'org.openrewrite.recipes.rewrite.OpenRewriteRecipeBestPracticesSubset' | ||
| additional_gradle_args: '-Drewrite.exclusion=**/rewrite-java-25/**' |
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.
Wondering if we should temporarily disable this workflow completely, such that we can merge the Java 25 parser and aim to fix the runtime issues downstream that cause the excessive suggstions to be reported.
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.
That seems like the only way forward, it is gonna be a challenge though, since the rewrite-gradle-plugin doesn't seem immediately fit for Gradle 9, but I can continue looking into that one :)
|
Now seeing the same suspicious code suggestions in an unrelated PR, so perhaps we don't have to trouble shoot that here: |
I was explicitly able to reproduce by changing my system jdk to 25 explicitly and running the plugin as we do in the pipeline though |
timtebeek
left a comment
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.
Great to see, thanks for taking the time to remove some oddities from our older parsers, ensure going forward we pick the right next parser, and can now move on to making the other parts work for Java 25+.
What's changed?
Initial setup of Java 25 parser based on the Java 21 variant
Checklist