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

Add support for guard clauses in Java 21 switch expressions #988

Closed
wants to merge 3 commits into from

Conversation

TheCK
Copy link
Contributor

@TheCK TheCK commented Nov 10, 2023

This PR adds support for switch statements where a case has a guard clause.

See Issue #983

Copy link

google-cla bot commented Nov 10, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Collaborator

@cushon cushon left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

Could you add a test? There are some examples of tests that are only run on newer Java versions here:

private static final ImmutableMultimap<Integer, String> VERSIONED_TESTS =

}
builder.close();
}
if (node.getGuard() != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this block is the only part that needs to be different between 17 and 21, I would like to avoid duplicating the rest of the method.

An alternative would be to do something like this:

protected List<? extends Tree> getPermitsClause(ClassTree node) {

There could be a getGuard method that returns null in the base class, and Java21InputAstVisitor could override it

Class.forName("com.google.googlejavaformat.java.java21.Java21InputAstVisitor")
.asSubclass(JavaInputAstVisitor.class)
.getConstructor(OpsBuilder.class, int.class)
.newInstance(builder, options.indentationMultiplier());
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be worth creating a shared helper to reflectively instantiate the visitor at this point:

private static JavaInputAstVisitor createVisitor(String className) { .. }

@TheCK
Copy link
Contributor Author

TheCK commented Dec 7, 2023

@cushon I've implemented your requested changes. Hope this works for you. If you think it needs more attention still, let me know.

copybara-service bot pushed a commit that referenced this pull request Dec 7, 2023
This PR adds support for `switch` statements where a `case` has a guard clause.

See Issue #983

Fixes #988

FUTURE_COPYBARA_INTEGRATE_REVIEW=#988 from TheCK:master 4771486
PiperOrigin-RevId: 588901660
@copybara-service copybara-service bot closed this in b86c508 Dec 7, 2023
eholmer pushed a commit to palantir/palantir-java-format that referenced this pull request Oct 21, 2024
This PR adds support for `switch` statements where a `case` has a guard clause.

See Issue #983

Fixes #988

COPYBARA_INTEGRATE_REVIEW=google/google-java-format#988 from TheCK:master 4771486db7d8aab84eb4ecf8e68af2612d0c2b5c
PiperOrigin-RevId: 588913297
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants