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

) Summary: Fixes #319 Pull Request resolved: #325 Reviewed By: strulovich Differential Revision: D36929348 Pulled By: cgrushko fbshipit-source-id: 48f649a7edf08e58cdea7b6a620132c62807c0e4
facebook · Jun 7, 2022 · 44bf14c · 44bf14c
1 parent 24e6942
commit 44bf14c
Show file tree

Hide file tree

Showing 2 changed files with 114 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
@@ -1754,18 +1754,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 +1827,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 +2022,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 +2036,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 */
@@ -2436,4 +2407,28 @@ class KotlinInputAstVisitor(
   private fun visit(element: PsiElement?) {
     element?.accept(this)
   }
+
+  /** Emits a key word followed by a condition, e.g. `if (b)` or `while (c < d )` */
+  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(")")
+  }
 }
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(