-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
feat: JDK 22 unnamed variables & patterns #620
feat: JDK 22 unnamed variables & patterns #620
Conversation
@clementdessoude Let me know what you think about the formatting choices I made. I went back and forth over a few different styles for formatting switch labels with multiple patterns, and ended up deciding to go with this one, but I'd certainly be open to a different style. |
8432aa8
to
c56a8b8
Compare
For this one, I think the review will have to wait for next weekend, as it is less trivial than #603 😅 |
No worries! Initially, I tried to keep my changes to a minimum, but realized that some of the spec deviations were causing a lot of trouble for me, so I ended up aligning things a bit more closely to the spec which helped keep the formatting code a lot simpler. |
c56a8b8
to
9678640
Compare
// https://docs.oracle.com/javase/specs/jls/se16/html/jls-14.html#jls-Resource | ||
// https://docs.oracle.com/javase/specs/jls/se21/html/jls-14.html#jls-Resource | ||
$.RULE("resource", () => { | ||
$.OR([ | ||
{ | ||
GATE: $.BACKTRACK($.resourceInit), | ||
// Spec Deviation: extracted this alternative to "resourceInit" | ||
// to enable backtracking. | ||
ALT: () => $.SUBRULE($.resourceInit) | ||
GATE: () => $.BACKTRACK_LOOKAHEAD($.isLocalVariableDeclaration), | ||
ALT: () => $.SUBRULE($.localVariableDeclaration) | ||
}, | ||
{ ALT: () => $.SUBRULE($.variableAccess) } | ||
]); | ||
}); | ||
|
||
// Spec Deviation: extracted from "resource" | ||
$.RULE("resourceInit", () => { | ||
$.MANY(() => { | ||
$.SUBRULE($.variableModifier); | ||
}); | ||
$.SUBRULE($.localVariableType); | ||
$.CONSUME(t.Identifier); | ||
$.CONSUME(t.Equals); | ||
$.SUBRULE($.expression); | ||
}); |
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.
Thanks for the update ! If I may, it would have been even better if you split the commit in two, one for handling the new JDK 22 unnamed variables & patterns feature, and one other for updating this: it is easier to review as it does not mix up two different things :)
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.
Good point, I probably should have made this change as the first commit, proving that no changes occurred after making this change, and then have my JDK 22 unnamed variables & patterns feature be the second commit, which would effectively leverage the change from the first. I'll keep this kind of thing in mind for future contributions!
What changed with this PR:
JDK 22 unnamed variables & patterns, and switch labels with multiple patterns, are now supported.
Example
Input
Output
Relative issues or prs:
Closes #612