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

Map trailing commas as TrailingComma Marker, to fix J.Erroneous issues seen #4869

Merged
merged 14 commits into from
Jan 9, 2025

Conversation

@timtebeek timtebeek added the bug Something isn't working label Jan 8, 2025
@timtebeek timtebeek requested a review from sambsnyd January 8, 2025 22:24
@timtebeek timtebeek self-assigned this Jan 8, 2025
timtebeek and others added 2 commits January 9, 2025 00:02
Add an overload for `ReloadableJava17ParserVisitor#convert()` which allows supplying a `Markers` function, so that we can capture trailing commas using a `TrailingComma` marker and then also avoid the issue with the `J.Erroneous` getting constructed.
The last enum constant should only be right-padded if it has a trailing comma or semicolon. This is important because the whitespace after it belongs to the prefix of the next statement or the `J.Block#end`.
Comment on lines +481 to +483
if (t != lastConstant) {
return whitespace();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if-block is actually not necessary, as we are here inside the suffix function which is only ever applied to the last element.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree but on the other hand it also guarantees that we don't accidentally do harm when it is called on an element that is not the last one

@Laurens-W Laurens-W marked this pull request as ready for review January 9, 2025 14:04
Copy link
Contributor Author

@timtebeek timtebeek left a 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 the help here guys! Great to have this sorted before the release.

@timtebeek timtebeek changed the title Do not allow J.Erroneous LST nodes by default in tests Map trailing commas as TrailingComma Marker, to fix J.Erroneous issues seen Jan 9, 2025
@timtebeek timtebeek merged commit 82b61de into main Jan 9, 2025
2 checks passed
@timtebeek timtebeek deleted the do-not-allow-erroneous-lst-nodes-by-default branch January 9, 2025 16:06
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
3 participants