Fix line breaks in switch expression (#394)

Fix line breaks in switch expression
palantir · Jan 6, 2021 · 216d051 · 216d051
1 parent 6686a27
commit 216d051
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 8 deletions.
diff --git a/changelog/@unreleased/pr-394.v2.yml b/changelog/@unreleased/pr-394.v2.yml
@@ -0,0 +1,5 @@
+type: fix
+fix:
+  description: Fix line breaks in switch expression
+  links:
+  - https://github.com/palantir/palantir-java-format/pull/394
diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java
@@ -187,7 +187,7 @@ boolean isYes() {
     }
 
     /** Whether to collapse empty blocks. */
-    enum CollapseEmptyOrNot {
+    protected enum CollapseEmptyOrNot {
         YES,
         NO;
 
@@ -201,7 +201,7 @@ boolean isYes() {
     }
 
     /** Whether to allow leading blank lines in blocks. */
-    enum AllowLeadingBlankLine {
+    protected enum AllowLeadingBlankLine {
         YES,
         NO;
 
@@ -211,7 +211,7 @@ static AllowLeadingBlankLine valueOf(boolean b) {
     }
 
     /** Whether to allow trailing blank lines in blocks. */
-    enum AllowTrailingBlankLine {
+    protected enum AllowTrailingBlankLine {
         YES,
         NO;
 
@@ -1218,6 +1218,11 @@ public Void visitLabeledStatement(LabeledStatementTree node, Void unused) {
     public Void visitLambdaExpression(LambdaExpressionTree node, Void unused) {
         sync(node);
         boolean statementBody = node.getBodyKind() == LambdaExpressionTree.BodyKind.STATEMENT;
+        visitLambdaExpression(node, statementBody);
+        return null;
+    }
+
+    protected void visitLambdaExpression(LambdaExpressionTree node, boolean statementBody) {
         boolean parens = builder.peekToken().equals(Optional.of("("));
         builder.open("lambda arguments", parens ? plusFour : ZERO);
         if (parens) {
@@ -1266,7 +1271,6 @@ public Void visitLambdaExpression(LambdaExpressionTree node, Void unused) {
             scan(node.getBody(), null);
         }
         builder.close();
-        return null;
     }
 
     @Override
@@ -2108,7 +2112,7 @@ void visitAnnotations(List<? extends AnnotationTree> annotations, BreakOrNot bre
     }
 
     /** Helper method for blocks. */
-    private void visitBlock(
+    protected void visitBlock(
             BlockTree node,
             CollapseEmptyOrNot collapseEmptyOrNot,
             AllowLeadingBlankLine allowLeadingBlankLine,

diff --git a/...-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java b/...-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java
@@ -26,12 +26,15 @@
 import com.palantir.javaformat.OpsBuilder;
 import com.palantir.javaformat.java.JavaInputAstVisitor;
 import com.sun.source.tree.BindingPatternTree;
+import com.sun.source.tree.BlockTree;
 import com.sun.source.tree.CaseTree;
 import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.InstanceOfTree;
+import com.sun.source.tree.LambdaExpressionTree;
 import com.sun.source.tree.SwitchExpressionTree;
 import com.sun.source.tree.Tree;
+import com.sun.source.tree.Tree.Kind;
 import com.sun.source.tree.YieldTree;
 import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.tree.JCTree;
@@ -199,15 +202,17 @@ public Void visitCase(CaseTree node, Void unused) {
         } else {
             token("case", plusTwo);
             builder.space();
+            builder.open(plusTwo);
             boolean first = true;
             for (ExpressionTree expression : node.getExpressions()) {
                 if (!first) {
                     token(",");
-                    builder.space();
+                    builder.breakOp(" ");
                 }
                 scan(expression, null);
                 first = false;
             }
+            builder.close();
         }
         switch (node.getCaseKind()) {
             case STATEMENT:
@@ -226,12 +231,36 @@ public Void visitCase(CaseTree node, Void unused) {
                 token("-");
                 token(">");
                 builder.space();
-                scan(node.getBody(), null);
+                if (node.getBody().getKind() == 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;
     }
+
+    /**
+     * TODO(fwindheuser): Collapse with
+     * {@link JavaInputAstVisitor#visitLambdaExpression(LambdaExpressionTree, Void)}} after dropping Java 11
+     * compatibility.
+     */
+    @Override
+    public Void visitLambdaExpression(LambdaExpressionTree node, Void unused) {
+        sync(node);
+        // Also format switch expressions as statement body instead of inlining them
+        boolean statementBody = node.getBodyKind() == LambdaExpressionTree.BodyKind.STATEMENT
+                || node.getBody().getKind() == Kind.SWITCH_EXPRESSION;
+        visitLambdaExpression(node, statementBody);
+        return null;
+    }
 }
diff --git a/...va-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.input b/...va-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.input
@@ -22,4 +22,23 @@ class ExpressionSwitch {
     };
     return val;
   }
-}
+
+  int wrapping(Wrapping w) {
+    switch (w) {
+        case THIS_IS_A_VERY_LONG_ENUM_VALUE_ONE,
+            THIS_IS_A_VERY_LONG_ENUM_VALUE_TWO,
+            THIS_IS_A_VERY_LONG_ENUM_VALUE_THREE -> {}
+    }
+
+    switch (w) {
+        case CASE_A, CASE_B, CASE_C -> {}
+    }
+  }
+
+  Supplier<Integer> lambda(int k) {
+    return () -> switch (k) {
+      case 0 -> true;
+      default -> false;
+    };
+  }
+}
diff --git a/...a-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.output b/...a-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.output
@@ -28,4 +28,23 @@ class ExpressionSwitch {
                 };
         return val;
     }
+
+    int wrapping(Wrapping w) {
+        switch (w) {
+            case THIS_IS_A_VERY_LONG_ENUM_VALUE_ONE,
+                THIS_IS_A_VERY_LONG_ENUM_VALUE_TWO,
+                THIS_IS_A_VERY_LONG_ENUM_VALUE_THREE -> {}
+        }
+
+        switch (w) {
+            case CASE_A, CASE_B, CASE_C -> {}
+        }
+    }
+
+    Supplier<Integer> lambda(int k) {
+        return () -> switch (k) {
+            case 0 -> true;
+            default -> false;
+        };
+    }
 }