Improve record parsing #5916

Laurens-W · 2025-08-14T17:16:58Z

What's changed?

Improve the parsing of records to match closer with the Java compiler output

What's your motivation?

We've seen annotations with different targets end up on different elements in the compiler, but this is not reflected in the LST

Any additional context

~~Remaining TODOs~~
~~- [ ] Expand the logic around mapping the members, as currently we only consider VariableTree when parsing a record~~
~~- [ ] Add a marker to the LST so we can preserve the original order of the annotations~~
~~- [ ] Adjust the JavaPrinter so that we become print idempotent again~~
Based on Knut's guidance I've simplified the approach to not require drastic changes to the printer

Closes added ConfigurationPropertiesDefaultValueTest for LST contains invalid nodes rewrite-spring#713

Checklist

I've added unit tests to cover both positive and negative cases
I've read and applied the recipe conventions and best practices
I've used the IntelliJ IDEA auto-formatter on affected files

rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21ParserVisitor.java

rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17ParserVisitor.java

Laurens-W · 2025-08-20T12:39:45Z

rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17ParserVisitor.java

    public J visitLiteral(LiteralTree node, Space fmt) {
-        cursor(endPos(node));
+        int endPos = endPos(node);
        Object value = node.getValue();
-        String valueSource = source.substring(((JCLiteral) node).getStartPosition(), endPos(node));
+
+        if (endPos == Position.NOPOS) {
+            if (typeMapping.primitive(((JCLiteral) node).typetag) == JavaType.Primitive.String) {
+                int quote = source.substring(cursor).startsWith("\"\"\"") ? 3 : 1;
+                if (quote == 3) {
+                    endPos = cursor + quote + source.indexOf("\"\"\"", cursor + quote) + quote;
+                } else {
+                    endPos = cursor + quote + value.toString().length() + quote;
+                }
+            } else {
+                endPos = cursor + indexOf(source.substring(cursor),
+                        ch -> Character.isWhitespace(ch) || ",;)]}+-*/%=!<>&|^?:.".indexOf(ch) != -1
+                );
+            }
+        }
+
+        cursor(endPos);
+        String valueSource = source.substring(((JCLiteral) node).getStartPosition(), endPos);


The reason this had to change at all is because the endpos table is computed in this step

rewrite/rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17Parser.java

Line 224 in 04628ec

com.sun.tools.javac.util.List<JCTree.JCCompilationUnit> jcCompilationUnits = compiler.parseFiles((List<JavaFileObject>) (List<?>) inputFileObjects, true);

and the removal of unapplicable annotations is done here

rewrite/rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17Parser.java

Line 236 in 04628ec

annotate.unblockAnnotations(); // also flushes once unblocked

without updating that endpos table, so we can no longer reliable get that information

rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21ParserVisitor.java

rewrite-core/src/main/java/org/openrewrite/internal/StringUtils.java

rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17ParserVisitor.java

timtebeek · 2025-09-02T20:48:17Z

@Laurens-W should any of the improvements here now be replicated into the Java 25 parser since this wasn't merged yet?

Laurens-W · 2025-09-03T10:29:31Z

@Laurens-W should any of the improvements here now be replicated into the Java 25 parser since this wasn't merged yet?

Yes, they probably should, I'll update the PR!

tubbynl · 2025-09-04T09:18:14Z

tnx for the hard work; i'll retest openrewrite/rewrite-spring#714 when this get's released :)

tubbynl · 2025-10-06T11:47:22Z

re-tested with rewrite-maven-plugin:6.19.0 and rewrite-spring:6.15.0 and can confirm it now parses this correctly and does not yield errors :)

Laurens-W · 2025-10-06T13:01:51Z

Thanks for reporting back @tubbynl!
If you find any other issues feel free to report them 👍🏻

Laurens-W added 4 commits August 13, 2025 19:41

WIP

92b9ee0

Step 1; print idempotent, but not yet resembling AST

658eb7d

Step 2; LST matches closer to AST

3581c47

FMT

fa0754e

Laurens-W self-assigned this Aug 14, 2025

Laurens-W added bug Something isn't working parser labels Aug 14, 2025

github-project-automation bot moved this to In Progress in OpenRewrite Aug 14, 2025

github-project-automation bot added this to OpenRewrite Aug 14, 2025

github-actions bot reviewed Aug 14, 2025

View reviewed changes

rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21ParserVisitor.java Outdated Show resolved Hide resolved

Self review

1eb0700

Laurens-W commented Aug 14, 2025

View reviewed changes

rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21ParserVisitor.java Outdated Show resolved Hide resolved

Bot review

c8631fd

Laurens-W commented Aug 14, 2025

View reviewed changes

rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21ParserVisitor.java Outdated Show resolved Hide resolved

Laurens-W added 2 commits August 18, 2025 11:46

Apply Knut's suggestions

a644b3a

Further simplify

d04c724

Laurens-W requested a review from knutwannheden August 18, 2025 11:10

Laurens-W marked this pull request as ready for review August 18, 2025 11:10

This comment was marked as resolved.

Sign in to view

Laurens-W added 4 commits August 18, 2025 13:16

Backport to Java 17 as well

4c7988a

Fix tests and imports

a2f5b90

Limit scope of recordParam map

8129e36

Fix final test

749c8b4

Laurens-W requested review from jevanlingen and timtebeek August 18, 2025 14:13

jevanlingen reviewed Aug 18, 2025

View reviewed changes

rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17ParserVisitor.java Outdated Show resolved Hide resolved

jevanlingen reviewed Aug 18, 2025

View reviewed changes

rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17ParserVisitor.java Outdated Show resolved Hide resolved

jevanlingen reviewed Aug 18, 2025

View reviewed changes

rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17ParserVisitor.java Outdated Show resolved Hide resolved

Laurens-W added 2 commits August 19, 2025 10:49

First few review comments

4e10143

Port First few review comments to Java21 parser

4dff439

jevanlingen reviewed Aug 20, 2025

View reviewed changes

rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17ParserVisitor.java Outdated Show resolved Hide resolved

jevanlingen reviewed Aug 20, 2025

View reviewed changes

rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17ParserVisitor.java Outdated Show resolved Hide resolved

Apply feedback

525fc4a

Laurens-W commented Aug 20, 2025

View reviewed changes

knutwannheden reviewed Aug 20, 2025

View reviewed changes

rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21ParserVisitor.java Outdated Show resolved Hide resolved

Laurens-W and others added 4 commits August 20, 2025 16:51

Add override with offset for indexOf

2252de8

Merge branch 'main' into record-annotation

abbb350

Remove redundant space

c1fe5f9

Apply to both parsers

77b0f1e

jevanlingen approved these changes Aug 20, 2025

View reviewed changes

github-project-automation bot moved this from In Progress to Ready to Review in OpenRewrite Aug 20, 2025

knutwannheden reviewed Aug 20, 2025

View reviewed changes

rewrite-core/src/main/java/org/openrewrite/internal/StringUtils.java Outdated Show resolved Hide resolved

knutwannheden reviewed Aug 20, 2025

View reviewed changes

rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17ParserVisitor.java Outdated Show resolved Hide resolved

Apply feedback

ece1965

knutwannheden approved these changes Aug 20, 2025

View reviewed changes

Merge branch 'main' into record-annotation

d110149

Update Java 25 parser

55dd366

timtebeek added the java label Sep 3, 2025

jkschneider approved these changes Sep 3, 2025

View reviewed changes

timtebeek merged commit 87e9a68 into main Sep 3, 2025
2 checks passed

timtebeek deleted the record-annotation branch September 3, 2025 23:00

github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Sep 3, 2025

timtebeek mentioned this pull request Sep 22, 2025

DefaultValue annotation on a record seems to fail openrewrite/rewrite-spring#714

Closed

This was referenced Nov 3, 2025

Don't remove referenced annotations from record parameters #4474

Closed

RemoveUnusedImports removes used imports for annotated record constructor parameters #4473

Closed

Improve record parsing #5916

Improve record parsing #5916

Uh oh!

Conversation

Laurens-W commented Aug 14, 2025 • edited by timtebeek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's changed?

What's your motivation?

Any additional context

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Laurens-W Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

timtebeek commented Sep 2, 2025

Uh oh!

Laurens-W commented Sep 3, 2025

Uh oh!

Uh oh!

tubbynl commented Sep 4, 2025

Uh oh!

tubbynl commented Oct 6, 2025

Uh oh!

Laurens-W commented Oct 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Laurens-W commented Aug 14, 2025 •

edited by timtebeek

Loading

Laurens-W Aug 20, 2025 •

edited

Loading