From 38409133cb847273ec215df384d4ae0ab3bfc96e Mon Sep 17 00:00:00 2001 From: Patrick Koenig Date: Thu, 17 Dec 2020 22:39:28 -0800 Subject: [PATCH 1/6] Format tests for switch expressions --- .../java/testdata/ExpressionSwitch.input | 23 ++++++++++++++++++- .../java/testdata/ExpressionSwitch.output | 21 +++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.input b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.input index 82bc25bfa..9cd4afc5b 100644 --- a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.input +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.input @@ -22,4 +22,25 @@ class ExpressionSwitch { }; return val; } -} \ No newline at end of file + + enum Wrapping { + THIS_IS_A_VERY_LONG_ENUM_VALUE_ONE, + THIS_IS_A_VERY_LONG_ENUM_VALUE_TWO, + THIS_IS_A_VERY_LONG_ENUM_VALUE_THREE, + } + + 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 -> {} + } + } + + Supplier lambda(int k) { + return () -> switch (k) { + case 0 -> true; + default -> false; + }; + } +} diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.output b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.output index 348b651c9..7f8180067 100644 --- a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.output +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.output @@ -28,4 +28,25 @@ class ExpressionSwitch { }; return val; } + + enum Wrapping { + THIS_IS_A_VERY_LONG_ENUM_VALUE_ONE, + THIS_IS_A_VERY_LONG_ENUM_VALUE_TWO, + THIS_IS_A_VERY_LONG_ENUM_VALUE_THREE, + } + + 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 -> {} + } + } + + Supplier closeDelimiter(int k) { + return () -> switch (k) { + case 0 -> true; + default -> false; + }; + } } From 23b7c124a4380593a705abfc4875936820a5deb5 Mon Sep 17 00:00:00 2001 From: fwindheuser Date: Wed, 6 Jan 2021 15:12:07 +0100 Subject: [PATCH 2/6] Handle switch expression lambda bodies as block statements --- .../com/palantir/javaformat/java/JavaInputAstVisitor.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 index eca68c4e8..648a3fb03 100644 --- 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 @@ -127,6 +127,7 @@ import com.sun.source.tree.SynchronizedTree; import com.sun.source.tree.ThrowTree; import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; import com.sun.source.tree.TryTree; import com.sun.source.tree.TypeCastTree; import com.sun.source.tree.TypeParameterTree; @@ -1217,7 +1218,9 @@ public Void visitLabeledStatement(LabeledStatementTree node, Void unused) { @Override public Void visitLambdaExpression(LambdaExpressionTree node, Void unused) { sync(node); - boolean statementBody = node.getBodyKind() == LambdaExpressionTree.BodyKind.STATEMENT; + // Also format switch expressions as statement body instead of inlining them + boolean statementBody = node.getBodyKind() == LambdaExpressionTree.BodyKind.STATEMENT + || node.getBody().getKind() == Kind.SWITCH_EXPRESSION; boolean parens = builder.peekToken().equals(Optional.of("(")); builder.open("lambda arguments", parens ? plusFour : ZERO); if (parens) { From 4b0fc94a00b2c9c9ca8e519645e7e697ea1e22cb Mon Sep 17 00:00:00 2001 From: fwindheuser Date: Wed, 6 Jan 2021 16:06:21 +0100 Subject: [PATCH 3/6] Break switch expressions with multiple long case parameters --- .../javaformat/java/JavaInputAstVisitor.java | 8 ++++---- .../java/java14/Java14InputAstVisitor.java | 15 +++++++++++++-- .../java/testdata/ExpressionSwitch.input | 10 ++++------ .../java/testdata/ExpressionSwitch.output | 12 +++++------- 4 files changed, 26 insertions(+), 19 deletions(-) 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 index 648a3fb03..bf3b6bcbb 100644 --- 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 @@ -188,7 +188,7 @@ boolean isYes() { } /** Whether to collapse empty blocks. */ - enum CollapseEmptyOrNot { + protected enum CollapseEmptyOrNot { YES, NO; @@ -202,7 +202,7 @@ boolean isYes() { } /** Whether to allow leading blank lines in blocks. */ - enum AllowLeadingBlankLine { + protected enum AllowLeadingBlankLine { YES, NO; @@ -212,7 +212,7 @@ static AllowLeadingBlankLine valueOf(boolean b) { } /** Whether to allow trailing blank lines in blocks. */ - enum AllowTrailingBlankLine { + protected enum AllowTrailingBlankLine { YES, NO; @@ -2111,7 +2111,7 @@ void visitAnnotations(List annotations, BreakOrNot bre } /** Helper method for blocks. */ - private void visitBlock( + protected void visitBlock( BlockTree node, CollapseEmptyOrNot collapseEmptyOrNot, AllowLeadingBlankLine allowLeadingBlankLine, diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java index 913a504f0..9cc1b44be 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java @@ -26,6 +26,7 @@ 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; @@ -199,15 +200,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,7 +229,15 @@ public Void visitCase(CaseTree node, Void unused) { token("-"); token(">"); builder.space(); - scan(node.getBody(), null); + if (node.getBody().getKind() == BLOCK) { + visitBlock( + (BlockTree) node.getBody(), + CollapseEmptyOrNot.YES, + AllowLeadingBlankLine.NO, + AllowTrailingBlankLine.NO); + } else { + scan(node.getBody(), null); + } builder.guessToken(";"); break; default: diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.input b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.input index 9cd4afc5b..bdc4d2b8c 100644 --- a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.input +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.input @@ -23,18 +23,16 @@ class ExpressionSwitch { return val; } - enum Wrapping { - THIS_IS_A_VERY_LONG_ENUM_VALUE_ONE, - THIS_IS_A_VERY_LONG_ENUM_VALUE_TWO, - THIS_IS_A_VERY_LONG_ENUM_VALUE_THREE, - } - 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 lambda(int k) { diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.output b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.output index 7f8180067..a85ed870e 100644 --- a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.output +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ExpressionSwitch.output @@ -29,21 +29,19 @@ class ExpressionSwitch { return val; } - enum Wrapping { - THIS_IS_A_VERY_LONG_ENUM_VALUE_ONE, - THIS_IS_A_VERY_LONG_ENUM_VALUE_TWO, - THIS_IS_A_VERY_LONG_ENUM_VALUE_THREE, - } - 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 closeDelimiter(int k) { + Supplier lambda(int k) { return () -> switch (k) { case 0 -> true; default -> false; From bdd5fcd50b40b15b264cdf08afb5775228b5e5ed Mon Sep 17 00:00:00 2001 From: fwindheuser Date: Wed, 6 Jan 2021 16:19:31 +0100 Subject: [PATCH 4/6] Fix test on java 11 --- .../javaformat/java/JavaInputAstVisitor.java | 11 ++++++----- .../java/java14/Java14InputAstVisitor.java | 12 ++++++++++++ 2 files changed, 18 insertions(+), 5 deletions(-) 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 index bf3b6bcbb..bb9519eb9 100644 --- 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 @@ -127,7 +127,6 @@ import com.sun.source.tree.SynchronizedTree; import com.sun.source.tree.ThrowTree; import com.sun.source.tree.Tree; -import com.sun.source.tree.Tree.Kind; import com.sun.source.tree.TryTree; import com.sun.source.tree.TypeCastTree; import com.sun.source.tree.TypeParameterTree; @@ -1218,9 +1217,12 @@ public Void visitLabeledStatement(LabeledStatementTree node, Void unused) { @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; + 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) { @@ -1269,7 +1271,6 @@ public Void visitLambdaExpression(LambdaExpressionTree node, Void unused) { scan(node.getBody(), null); } builder.close(); - return null; } @Override diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java index 9cc1b44be..06676767e 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java @@ -31,8 +31,10 @@ 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; @@ -245,4 +247,14 @@ public Void visitCase(CaseTree node, Void unused) { } return null; } + + @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; + } } From 0e0a0f61985008416058c94e096a59a706332e13 Mon Sep 17 00:00:00 2001 From: fwindheuser Date: Wed, 6 Jan 2021 15:19:31 +0000 Subject: [PATCH 5/6] Add generated changelog entries --- changelog/@unreleased/pr-394.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-394.v2.yml diff --git a/changelog/@unreleased/pr-394.v2.yml b/changelog/@unreleased/pr-394.v2.yml new file mode 100644 index 000000000..b9f8ec05a --- /dev/null +++ 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 From e7eb7171d7a8e839335a4c1e93c4717ee9f151fe Mon Sep 17 00:00:00 2001 From: fwindheuser Date: Wed, 6 Jan 2021 17:05:31 +0100 Subject: [PATCH 6/6] Add comments --- .../javaformat/java/java14/Java14InputAstVisitor.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java index 06676767e..a14fc6cc3 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java @@ -232,6 +232,7 @@ public Void visitCase(CaseTree node, Void unused) { token(">"); builder.space(); if (node.getBody().getKind() == BLOCK) { + // Explicit call with {@link CollapseEmptyOrNot.YES} to handle empty case blocks. visitBlock( (BlockTree) node.getBody(), CollapseEmptyOrNot.YES, @@ -248,6 +249,11 @@ public Void visitCase(CaseTree node, Void unused) { 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);