For annotated expressions, break after annotations iff it is block-le…

…vel expression. For some block-level expressions (e.g. binary operators, infix functions) the line break changes the target of the annotation. When on the previous line, the annotation targets the entire expression. When on the same line, it targets only the left-most sub-expression. In order to always preserve semantics, and guarantee consistent formatting, we always break only for this case.
facebook · Apr 17, 2022 · 60452a3 · 60452a3
1 parent 8a5af6c
commit 60452a3
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 18 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
@@ -1557,20 +1557,31 @@ class KotlinInputAstVisitor(
   override fun visitAnnotatedExpression(expression: KtAnnotatedExpression) {
     builder.sync(expression)
     builder.block(ZERO) {
-      loop@ for (child in expression.node.children()) {
-        when (val psi = child.psi) {
-          is PsiWhiteSpace -> continue@loop
-          is KtAnnotationEntry -> {
-            visit(psi)
-            if (expression.annotationEntries.size != 1) {
-              builder.forcedBreak()
-            } else {
-              builder.breakOp(Doc.FillMode.UNIFIED, " ", ZERO)
-            }
-          }
-          else -> visit(psi)
+      builder.block(ZERO) {
+        visit(expression.annotationEntries[0])
+        expression.annotationEntries.drop(1).forEach {
+          builder.breakOp(Doc.FillMode.UNIFIED, " ", ZERO)
+          visit(it)
         }
       }
+
+      /**
+       * Break after annotations iff this is a block-level expression.
+       *
+       * For some block-level expressions (e.g. binary operators, infix functions) the line break
+       * changes the target of the annotation. When on the previous line, the annotation targets the
+       * entire expression. When on the same line, it targets only the left-most sub-expression.
+       *
+       * In order to always preserve semantics, and guarantee consistent formatting, we always break
+       * only for this case.
+       */
+      if (expression.parent is KtBlockExpression) {
+        builder.forcedBreak()
+      } else {
+        builder.space()
+      }
+
+      visit(expression.baseExpression)
     }
   }
 

diff --git a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt
@@ -4021,7 +4021,8 @@ class FormatterTest {
       |class MyClass {}
       |
       |fun f() {
-      |  @Suppress("MagicNumber") add(10)
+      |  @Suppress("MagicNumber")
+      |  add(10)
       |
       |  @Annotation // test a comment after annotations
       |  return 5
@@ -4033,16 +4034,38 @@ class FormatterTest {
   fun `annotated expressions`() =
       assertFormatted(
           """
+      |------------------------------------------------
       |fun f() {
-      |  @Suppress("MagicNumber") add(10)
+      |  @Suppress("MagicNumber") add(10) && add(20)
+      |
+      |  @Suppress("MagicNumber")
+      |  add(10) && add(20)
+      |
+      |  @Anno1 @Anno2(param = Param1::class)
+      |  add(10) && add(20)
       |
       |  @Anno1
       |  @Anno2(param = Param1::class)
       |  @Anno3
       |  @Anno4(param = Param2::class)
-      |  add(10)
+      |  add(10) && add(20)
+      |
+      |  @Anno1
+      |  @Anno2(param = Param1::class)
+      |  @Anno3
+      |  @Anno4(param = Param2::class) add(10) &&
+      |      add(20)
+      |
+      |  @Suppress("MagicNumber") add(10) &&
+      |      add(20) &&
+      |      add(30)
+      |
+      |  add(@Suppress("MagicNumber") 10) &&
+      |      add(20) &&
+      |      add(30)
       |}
-      |""".trimMargin())
+      |""".trimMargin(),
+          deduceMaxWidth = true)
 
   @Test
   fun `annotated function declarations`() =
@@ -4686,8 +4709,7 @@ class FormatterTest {
       |    }
       |
       |fun foo() =
-      |    Runnable @Px
-      |    {
+      |    Runnable @Px {
       |      foo()
       |      //
       |    }