refactor guard clause PR and add a test

google · Dec 7, 2023 · 628fd63 · 628fd63
1 parent efcc978
commit 628fd63
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 94 deletions.
diff --git 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
@@ -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
@@ -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<? extends CaseLabelTree> 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
@@ -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
@@ -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
@@ -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;
+    };
+  }
+}