For --google_style, break between ( and long condition expressions

Fixes #319
facebook · Jun 2, 2022 · 4ce0ae6 · 4ce0ae6
1 parent 24e6942
commit 4ce0ae6
Show file tree

Hide file tree

Showing 2 changed files with 115 additions and 38 deletions.
diff --git a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt
@@ -698,6 +698,31 @@ class KotlinInputAstVisitor(
     return extractCallExpression(this)?.lambdaArguments?.isNotEmpty() ?: false
   }
 
+  private fun emitKeywordWithCondition(keyword: String, condition: KtExpression?) {
+    if (condition == null) {
+      builder.token(keyword)
+      return
+    }
+
+    builder.block(ZERO) {
+      builder.token(keyword)
+      builder.space()
+      builder.token("(")
+      if (isGoogleStyle) {
+        builder.block(expressionBreakIndent) {
+          builder.breakOp(Doc.FillMode.UNIFIED, "", ZERO)
+          visit(condition)
+          builder.breakOp(Doc.FillMode.UNIFIED, "", expressionBreakNegativeIndent)
+        }
+      } else {
+        builder.block(ZERO) {
+          visit(condition)
+        }
+      }
+    }
+    builder.token(")")
+  }
+
   /**
    * emitQualifiedExpression formats call expressions that are either part of a qualified
    * expression, or standing alone. This method makes it easier to handle both cases uniformly.
@@ -1754,18 +1779,8 @@ class KotlinInputAstVisitor(
   override fun visitWhenExpression(expression: KtWhenExpression) {
     builder.sync(expression)
     builder.block(ZERO) {
-      builder.token("when")
-      expression.subjectExpression?.let { subjectExp ->
-        builder.space()
-        builder.block(ZERO) {
-          builder.token("(")
-          builder.block(if (isGoogleStyle) expressionBreakIndent else ZERO) { visit(subjectExp) }
-          if (isGoogleStyle) {
-            builder.breakOp(Doc.FillMode.UNIFIED, "", ZERO)
-          }
-        }
-        builder.token(")")
-      }
+      emitKeywordWithCondition("when", expression.subjectExpression)
+
       builder.space()
       builder.token("{", Doc.Token.RealOrImaginary.REAL, blockIndent, Optional.of(blockIndent))
 
@@ -1837,18 +1852,7 @@ class KotlinInputAstVisitor(
   override fun visitIfExpression(expression: KtIfExpression) {
     builder.sync(expression)
     builder.block(ZERO) {
-      builder.block(ZERO) {
-        builder.token("if")
-        builder.space()
-        builder.token("(")
-        builder.block(if (isGoogleStyle) expressionBreakIndent else ZERO) {
-          visit(expression.condition)
-        }
-        if (isGoogleStyle) {
-          builder.breakOp(Doc.FillMode.UNIFIED, "", ZERO)
-        }
-      }
-      builder.token(")")
+      emitKeywordWithCondition("if", expression.condition)
 
       if (expression.then is KtBlockExpression) {
         builder.space()
@@ -2043,11 +2047,7 @@ class KotlinInputAstVisitor(
   /** Example `while (a < b) { ... }` */
   override fun visitWhileExpression(expression: KtWhileExpression) {
     builder.sync(expression)
-    builder.token("while")
-    builder.space()
-    builder.token("(")
-    visit(expression.condition)
-    builder.token(")")
+    emitKeywordWithCondition("while", expression.condition)
     builder.space()
     visit(expression.body)
   }
@@ -2061,11 +2061,7 @@ class KotlinInputAstVisitor(
       visit(expression.body)
       builder.space()
     }
-    builder.token("while")
-    builder.space()
-    builder.token("(")
-    visit(expression.condition)
-    builder.token(")")
+    emitKeywordWithCondition("while", expression.condition)
   }
 
   /** Example `break` or `break@foo` in a loop */

diff --git a/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt b/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt
@@ -823,14 +823,16 @@ class GoogleStyleFormatterKtTest {
           """
       |----------------------------
       |fun foo() {
-      |  if (expressions1 &&
+      |  if (
+      |    expressions1 &&
       |      expression2 &&
       |      expression3
       |  ) {
       |    bar()
       |  }
       |
-      |  if (foo(
+      |  if (
+      |    foo(
       |      expressions1 &&
       |        expression2 &&
       |        expression3
@@ -843,21 +845,39 @@ class GoogleStyleFormatterKtTest {
           formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
+  @Test
+  fun `if expression with condition that exactly fits to line`() =
+      assertFormatted(
+          """
+      |-------------------------
+      |fun foo() {
+      |  if (
+      |    e1 && e2 && e3 = e4
+      |  ) {
+      |    bar()
+      |  }
+      |}
+      |""".trimMargin(),
+          formattingOptions = Formatter.GOOGLE_FORMAT,
+          deduceMaxWidth = true)
+
   @Test
   fun `when() expression with multiline condition`() =
       assertFormatted(
           """
       |-----------------------
       |fun foo() {
-      |  when (expressions1 +
+      |  when (
+      |    expressions1 +
       |      expression2 +
       |      expression3
       |  ) {
       |    1 -> print(1)
       |    2 -> print(2)
       |  }
       |
-      |  when (foo(
+      |  when (
+      |    foo(
       |      expressions1 &&
       |        expression2 &&
       |        expression3
@@ -871,6 +891,67 @@ class GoogleStyleFormatterKtTest {
           formattingOptions = Formatter.GOOGLE_FORMAT,
           deduceMaxWidth = true)
 
+  @Test
+  fun `when expression with condition that exactly fits to line`() =
+      assertFormatted(
+          """
+      |---------------------------
+      |fun foo() {
+      |  when (
+      |    e1 && e2 && e3 = e4
+      |  ) {
+      |    1 -> print(1)
+      |    2 -> print(2)
+      |  }
+      |}
+      |""".trimMargin(),
+          formattingOptions = Formatter.GOOGLE_FORMAT,
+          deduceMaxWidth = true)
+
+    @Test
+  fun `while expression with multiline condition`() =
+      assertFormatted(
+          """
+      |----------------------------
+      |fun foo() {
+      |  while (
+      |    expressions1 &&
+      |      expression2 &&
+      |      expression3
+      |  ) {
+      |    bar()
+      |  }
+      |
+      |  while (
+      |    foo(
+      |      expressions1 &&
+      |        expression2 &&
+      |        expression3
+      |    )
+      |  ) {
+      |    bar()
+      |  }
+      |}
+      |""".trimMargin(),
+          formattingOptions = Formatter.GOOGLE_FORMAT,
+          deduceMaxWidth = true)
+
+  @Test
+  fun `while expression with condition that exactly fits to line`() =
+      assertFormatted(
+          """
+      |----------------------------
+      |fun foo() {
+      |  while (
+      |    e1 && e2 && e3 = e4
+      |  ) {
+      |    bar()
+      |  }
+      |}
+      |""".trimMargin(),
+          formattingOptions = Formatter.GOOGLE_FORMAT,
+          deduceMaxWidth = true)
+
   @Test
   fun `handle destructuring declaration`() =
       assertFormatted(