From 8b528f09b35f21267d01f72ffa6c81b1047ce0e8 Mon Sep 17 00:00:00 2001 From: Karsten Knuth Date: Fri, 10 Nov 2023 15:34:35 +0100 Subject: [PATCH 1/2] Add support for guard clauses in Java 21 switch expressions --- core/pom.xml | 29 ++++++- .../googlejavaformat/java/Formatter.java | 12 ++- .../java/java21/Java21InputAstVisitor.java | 76 +++++++++++++++++++ 3 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 core/src/main/java/com/google/googlejavaformat/java/java21/Java21InputAstVisitor.java diff --git a/core/pom.xml b/core/pom.xml index 038e4eda6..d1363fed1 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -226,7 +226,7 @@ jdk11 - (,17) + [11,17) @@ -236,6 +236,7 @@ **/Java17InputAstVisitor.java + **/Java21InputAstVisitor.java @@ -243,6 +244,32 @@ maven-javadoc-plugin com.google.googlejavaformat.java.java17 + com.google.googlejavaformat.java.java21 + + + + + + + jdk17 + + [17,21) + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + **/Java21InputAstVisitor.java + + + + + maven-javadoc-plugin + + com.google.googlejavaformat.java.java21 diff --git a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java index 9ff702d5a..831a07d90 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java +++ b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java @@ -151,7 +151,17 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept OpsBuilder builder = new OpsBuilder(javaInput, javaOutput); // Output the compilation unit. JavaInputAstVisitor visitor; - if (Runtime.version().feature() >= 17) { + if (Runtime.version().feature() >= 21) { + try { + visitor = + Class.forName("com.google.googlejavaformat.java.java21.Java21InputAstVisitor") + .asSubclass(JavaInputAstVisitor.class) + .getConstructor(OpsBuilder.class, int.class) + .newInstance(builder, options.indentationMultiplier()); + } catch (ReflectiveOperationException e) { + throw new LinkageError(e.getMessage(), e); + } + } else if (Runtime.version().feature() >= 17) { try { visitor = Class.forName("com.google.googlejavaformat.java.java17.Java17InputAstVisitor") diff --git a/core/src/main/java/com/google/googlejavaformat/java/java21/Java21InputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/java21/Java21InputAstVisitor.java new file mode 100644 index 000000000..f78984938 --- /dev/null +++ b/core/src/main/java/com/google/googlejavaformat/java/java21/Java21InputAstVisitor.java @@ -0,0 +1,76 @@ +package com.google.googlejavaformat.java.java21; + +import static com.google.common.collect.Iterables.getOnlyElement; + +import com.google.googlejavaformat.OpsBuilder; +import com.google.googlejavaformat.java.java17.Java17InputAstVisitor; +import com.sun.source.tree.*; +import java.util.List; + +public class Java21InputAstVisitor extends Java17InputAstVisitor { + + public Java21InputAstVisitor(OpsBuilder builder, int indentMultiplier) { + super(builder, indentMultiplier); + } + + @Override + public Void visitCase(CaseTree node, Void unused) { + sync(node); + markForPartialFormat(); + builder.forcedBreak(); + List labels = node.getLabels(); + boolean isDefault = + labels.size() == 1 && getOnlyElement(labels).getKind().name().equals("DEFAULT_CASE_LABEL"); + if (isDefault) { + token("default", plusTwo); + } else { + token("case", plusTwo); + builder.open(labels.size() > 1 ? plusFour : ZERO); + builder.space(); + boolean first = true; + for (Tree expression : labels) { + if (!first) { + token(","); + builder.breakOp(" "); + } + scan(expression, null); + first = false; + } + builder.close(); + } + if (node.getGuard() != null) { + builder.space(); + token("when"); + builder.space(); + scan(node.getGuard(), null); + } + switch (node.getCaseKind()) { + case STATEMENT: + token(":"); + builder.open(plusTwo); + visitStatements(node.getStatements()); + builder.close(); + break; + case RULE: + builder.space(); + token("-"); + token(">"); + builder.space(); + if (node.getBody().getKind() == Tree.Kind.BLOCK) { + // Explicit call with {@link CollapseEmptyOrNot.YES} to handle empty case blocks. + visitBlock( + (BlockTree) node.getBody(), + CollapseEmptyOrNot.YES, + AllowLeadingBlankLine.NO, + AllowTrailingBlankLine.NO); + } else { + scan(node.getBody(), null); + } + builder.guessToken(";"); + break; + default: + throw new AssertionError(node.getCaseKind()); + } + return null; + } +} From 628fd63e298381cd655c2e542215267beeb873e0 Mon Sep 17 00:00:00 2001 From: TheCK Date: Thu, 7 Dec 2023 22:24:34 +0100 Subject: [PATCH 2/2] refactor guard clause PR and add a test --- .../googlejavaformat/java/Formatter.java | 36 +++++----- .../java/java17/Java17InputAstVisitor.java | 28 ++++---- .../java/java21/Java21InputAstVisitor.java | 66 ++----------------- .../java/FormatterIntegrationTest.java | 1 + .../java/testdata/SwitchGuardClause.input | 9 +++ .../java/testdata/SwitchGuardClause.output | 9 +++ 6 files changed, 55 insertions(+), 94 deletions(-) create mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchGuardClause.input create mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchGuardClause.output diff --git a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java index 831a07d90..5aa7a1233 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java +++ b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java @@ -152,25 +152,13 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept // Output the compilation unit. JavaInputAstVisitor visitor; if (Runtime.version().feature() >= 21) { - try { - visitor = - Class.forName("com.google.googlejavaformat.java.java21.Java21InputAstVisitor") - .asSubclass(JavaInputAstVisitor.class) - .getConstructor(OpsBuilder.class, int.class) - .newInstance(builder, options.indentationMultiplier()); - } catch (ReflectiveOperationException e) { - throw new LinkageError(e.getMessage(), e); - } + visitor = + createVisitor( + "com.google.googlejavaformat.java.java21.Java21InputAstVisitor", builder, options); } else if (Runtime.version().feature() >= 17) { - try { - visitor = - Class.forName("com.google.googlejavaformat.java.java17.Java17InputAstVisitor") - .asSubclass(JavaInputAstVisitor.class) - .getConstructor(OpsBuilder.class, int.class) - .newInstance(builder, options.indentationMultiplier()); - } catch (ReflectiveOperationException e) { - throw new LinkageError(e.getMessage(), e); - } + visitor = + createVisitor( + "com.google.googlejavaformat.java.java17.Java17InputAstVisitor", builder, options); } else { visitor = new JavaInputAstVisitor(builder, options.indentationMultiplier()); } @@ -183,6 +171,18 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept javaOutput.flush(); } + private static JavaInputAstVisitor createVisitor( + final String className, final OpsBuilder builder, final JavaFormatterOptions options) { + try { + return Class.forName(className) + .asSubclass(JavaInputAstVisitor.class) + .getConstructor(OpsBuilder.class, int.class) + .newInstance(builder, options.indentationMultiplier()); + } catch (ReflectiveOperationException e) { + throw new LinkageError(e.getMessage(), e); + } + } + static boolean errorDiagnostic(Diagnostic input) { if (input.getKind() != Diagnostic.Kind.ERROR) { return false; diff --git a/core/src/main/java/com/google/googlejavaformat/java/java17/Java17InputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/java17/Java17InputAstVisitor.java index a0561e2fe..9fa6b6ab7 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/java17/Java17InputAstVisitor.java +++ b/core/src/main/java/com/google/googlejavaformat/java/java17/Java17InputAstVisitor.java @@ -22,20 +22,7 @@ import com.google.googlejavaformat.OpsBuilder; import com.google.googlejavaformat.OpsBuilder.BlankLineWanted; import com.google.googlejavaformat.java.JavaInputAstVisitor; -import com.sun.source.tree.AnnotationTree; -import com.sun.source.tree.BindingPatternTree; -import com.sun.source.tree.BlockTree; -import com.sun.source.tree.CaseLabelTree; -import com.sun.source.tree.CaseTree; -import com.sun.source.tree.ClassTree; -import com.sun.source.tree.CompilationUnitTree; -import com.sun.source.tree.InstanceOfTree; -import com.sun.source.tree.ModifiersTree; -import com.sun.source.tree.ModuleTree; -import com.sun.source.tree.SwitchExpressionTree; -import com.sun.source.tree.Tree; -import com.sun.source.tree.VariableTree; -import com.sun.source.tree.YieldTree; +import com.sun.source.tree.*; import com.sun.tools.javac.code.Flags; import com.sun.tools.javac.tree.JCTree; import com.sun.tools.javac.tree.JCTree.JCVariableDecl; @@ -238,6 +225,15 @@ public Void visitCase(CaseTree node, Void unused) { } builder.close(); } + + final ExpressionTree guard = getGuard(node); + if (guard != null) { + builder.space(); + token("when"); + builder.space(); + scan(guard, null); + } + switch (node.getCaseKind()) { case STATEMENT: token(":"); @@ -267,4 +263,8 @@ public Void visitCase(CaseTree node, Void unused) { } return null; } + + protected ExpressionTree getGuard(final CaseTree node) { + return null; + } } diff --git a/core/src/main/java/com/google/googlejavaformat/java/java21/Java21InputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/java21/Java21InputAstVisitor.java index f78984938..983c8814a 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/java21/Java21InputAstVisitor.java +++ b/core/src/main/java/com/google/googlejavaformat/java/java21/Java21InputAstVisitor.java @@ -1,11 +1,9 @@ package com.google.googlejavaformat.java.java21; -import static com.google.common.collect.Iterables.getOnlyElement; - import com.google.googlejavaformat.OpsBuilder; import com.google.googlejavaformat.java.java17.Java17InputAstVisitor; -import com.sun.source.tree.*; -import java.util.List; +import com.sun.source.tree.CaseTree; +import com.sun.source.tree.ExpressionTree; public class Java21InputAstVisitor extends Java17InputAstVisitor { @@ -14,63 +12,7 @@ public Java21InputAstVisitor(OpsBuilder builder, int indentMultiplier) { } @Override - public Void visitCase(CaseTree node, Void unused) { - sync(node); - markForPartialFormat(); - builder.forcedBreak(); - List labels = node.getLabels(); - boolean isDefault = - labels.size() == 1 && getOnlyElement(labels).getKind().name().equals("DEFAULT_CASE_LABEL"); - if (isDefault) { - token("default", plusTwo); - } else { - token("case", plusTwo); - builder.open(labels.size() > 1 ? plusFour : ZERO); - builder.space(); - boolean first = true; - for (Tree expression : labels) { - if (!first) { - token(","); - builder.breakOp(" "); - } - scan(expression, null); - first = false; - } - builder.close(); - } - if (node.getGuard() != null) { - builder.space(); - token("when"); - builder.space(); - scan(node.getGuard(), null); - } - switch (node.getCaseKind()) { - case STATEMENT: - token(":"); - builder.open(plusTwo); - visitStatements(node.getStatements()); - builder.close(); - break; - case RULE: - builder.space(); - token("-"); - token(">"); - builder.space(); - if (node.getBody().getKind() == Tree.Kind.BLOCK) { - // Explicit call with {@link CollapseEmptyOrNot.YES} to handle empty case blocks. - visitBlock( - (BlockTree) node.getBody(), - CollapseEmptyOrNot.YES, - AllowLeadingBlankLine.NO, - AllowTrailingBlankLine.NO); - } else { - scan(node.getBody(), null); - } - builder.guessToken(";"); - break; - default: - throw new AssertionError(node.getCaseKind()); - } - return null; + protected ExpressionTree getGuard(final CaseTree node) { + return node.getGuard(); } } diff --git a/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java b/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java index 61a43468a..688b24d8e 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java @@ -52,6 +52,7 @@ public class FormatterIntegrationTest { .putAll(15, "I603") .putAll(16, "I588") .putAll(17, "I683", "I684", "I696") + .putAll(21, "SwitchGuardClause") .build(); @Parameters(name = "{index}: {0}") diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchGuardClause.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchGuardClause.input new file mode 100644 index 000000000..25df58096 --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchGuardClause.input @@ -0,0 +1,9 @@ +class SwitchGuardClause { + boolean test(Object x) { + return switch (x) { + case String s when s.length() < 5 -> true; + case Integer i -> false; + default -> true; + }; + } +} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchGuardClause.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchGuardClause.output new file mode 100644 index 000000000..25df58096 --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchGuardClause.output @@ -0,0 +1,9 @@ +class SwitchGuardClause { + boolean test(Object x) { + return switch (x) { + case String s when s.length() < 5 -> true; + case Integer i -> false; + default -> true; + }; + } +}