-
Notifications
You must be signed in to change notification settings - Fork 49
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
Fix line breaks in switch expression #394
Changes from all commits
3840913
23b7c12
4b0fc94
bdd5fcd
0e0a0f6
e7eb717
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
type: fix | ||
fix: | ||
description: Fix line breaks in switch expression | ||
links: | ||
- https://github.com/palantir/palantir-java-format/pull/394 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For collapsing empty case blocks: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mind adding this as a source comment? :-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the comment 👍 |
||
(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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix for case 1. This has to be in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we add some kind of todo to collapse once we drop 11 (probably several years out) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the todo. But tbh, once we drop java 11 compat, we would collapse the whole |
||
visitLambdaExpression(node, statementBody); | ||
return null; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix for case 2.