Skip to content
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

Merged
merged 6 commits into from
Jan 6, 2021
Merged

Conversation

fawind
Copy link
Contributor

@fawind fawind commented Jan 6, 2021

Before this PR

Switch expressions were incorrectly formatted in two cases as reported in #383:

  1. When the switch expression is a lambda body:
 Supplier<Integer> closeDelimiter(int k) {
     return () -> switch (k) {
         case 0 -> true;
-        default -> false;};
+        default -> false;
+   };
 }
  1. When multiple case parameters exceed the maximum line length:
  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 -> {}
+         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 -> {}
      }
  }

After this PR

==COMMIT_MSG==
Fix line breaks in switch expression
==COMMIT_MSG==

cc @pkoenig10

@fawind fawind requested a review from CRogers January 6, 2021 15:33
@changelog-app
Copy link

changelog-app bot commented Jan 6, 2021

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Fix line breaks in switch expression

Check the box to generate changelog(s)

  • Generate changelog entry

@fawind fawind changed the title Fix breaks switch expression Fix line breaks in switch expression Jan 6, 2021
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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix for case 1. This has to be in the Java14InputAstVisitor.java class to not break java 11 compatibility.

Copy link
Contributor

Choose a reason for hiding this comment

The 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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 Java14InputAstVisitor with the base class.

boolean first = true;
for (ExpressionTree expression : node.getExpressions()) {
if (!first) {
token(",");
builder.space();
builder.breakOp(" ");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix for case 2.

@@ -226,12 +231,30 @@ public Void visitCase(CaseTree node, Void unused) {
token("-");
token(">");
builder.space();
scan(node.getBody(), null);
if (node.getBody().getKind() == BLOCK) {
visitBlock(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For collapsing empty case blocks: case ENUM -> {}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind adding this as a source comment? :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the comment 👍

Copy link
Contributor

@CRogers CRogers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice, found the comments very helpful in understanding this.

Copy link
Contributor

@carterkozak carterkozak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@bulldozer-bot bulldozer-bot bot merged commit 216d051 into develop Jan 6, 2021
@bulldozer-bot bulldozer-bot bot deleted the fw/switch-expression branch January 6, 2021 16:11
@svc-autorelease
Copy link
Collaborator

Released 2.0.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants