Disabling grouping of fields after the first group in qualified expre…

…ssions Summary: This addresses the request to not try and pack fields in a qualified expression after the first grouping. Reviewed By: hick209 Differential Revision: D36202571 fbshipit-source-id: 4e342ce1917a9a7a3d3fcdb229b613752d02b941
facebook · May 6, 2022 · 63eef56 · 63eef56
1 parent b7ff8ba
commit 63eef56
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 17 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
@@ -622,15 +622,17 @@ class KotlinInputAstVisitor(
               (receiverExpression as? KtQualifiedExpression)?.selectorExpression
                   ?: receiverExpression
           val current = checkNotNull(part.selectorExpression)
-          if (shouldGroupPartWithPrevious(parts, part, index, previous, current)) {
+          if (lastIndexToOpen == 0 &&
+              shouldGroupPartWithPrevious(parts, part, index, previous, current)) {
             // this and the previous items should be grouped for better style
-            // we add another group to open in the current index we have been using
-            groupingInfos[lastIndexToOpen].groupOpenCount++
+            // we add another group to open in index 0
+            groupingInfos[0].groupOpenCount++
             // we don't always close a group when emitting this node, so we need this flag to
             // mark if we need to close a group
             groupingInfos[index].shouldCloseGroup = true
           } else {
-            // use this index in to open future groups
+            // use this index in to open future groups for arrays and postfixes
+            // we will also stop grouping field access to the beginning of the expression
             lastIndexToOpen = index
           }
         }

diff --git a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt
@@ -585,7 +585,8 @@ class FormatterTest {
       |
       |  // Similar to above.
       |  abcdefghijkl.abcdefghijkl
-      |      ?.methodName3?.abcdefghijkl()
+      |      ?.methodName3
+      |      ?.abcdefghijkl()
       |
       |  // Multiple call expressions cause each part of the expression
       |  // to be placed on its own line.
@@ -2218,7 +2219,8 @@ class FormatterTest {
       |          .getInternalMutablePackageInfo(context.packageName)
       |          .someItems[0]
       |          .getInternalMutablePackageInfo(context.packageName)
-      |          .someItems[0].doIt()
+      |          .someItems[0]
+      |          .doIt()
       |}
       |""".trimMargin(),
           deduceMaxWidth = true)
@@ -5034,7 +5036,9 @@ class FormatterTest {
       |    .green
       |    .blue
       |    .shine()
-      |    .indigo.violet.cyan
+      |    .indigo
+      |    .violet
+      |    .cyan
       |    .magenta
       |    .key
       |""".trimMargin(),
@@ -5051,7 +5055,9 @@ class FormatterTest {
       |    .indigo
       |    .shine()
       |    .bright()
-      |    .violet.cyan.magenta
+      |    .violet
+      |    .cyan
+      |    .magenta
       |    .key
       |""".trimMargin(),
           deduceMaxWidth = true)
@@ -5119,7 +5125,9 @@ class FormatterTest {
       |    .green
       |    .blue
       |    .z { it }
-      |    .indigo.violet.cyan
+      |    .indigo
+      |    .violet
+      |    .cyan
       |    .magenta
       |    .key
       |""".trimMargin(),
@@ -5137,7 +5145,9 @@ class FormatterTest {
       |      it
       |      it
       |    }
-      |    .indigo.violet.cyan
+      |    .indigo
+      |    .violet
+      |    .cyan
       |    .magenta
       |    .key
       |""".trimMargin(),
@@ -5153,7 +5163,11 @@ class FormatterTest {
       |      it
       |      it
       |    }
-      |    .indigo.violet.cyan.magenta.key
+      |    .indigo
+      |    .violet
+      |    .cyan
+      |    .magenta
+      |    .key
       |""".trimMargin(),
           deduceMaxWidth = true)
 
@@ -5169,7 +5183,11 @@ class FormatterTest {
       |    }
       |    .shine()
       |    .bright()
-      |    .indigo.violet.cyan.magenta.key
+      |    .indigo
+      |    .violet
+      |    .cyan
+      |    .magenta
+      |    .key
       |""".trimMargin(),
           deduceMaxWidth = true)
 
@@ -5241,7 +5259,9 @@ class FormatterTest {
       |    .indigo
       |    .z { it }
       |    .shine()
-      |    .violet.cyan.magenta
+      |    .violet
+      |    .cyan
+      |    .magenta
       |    .key
       |""".trimMargin(),
           deduceMaxWidth = true)
@@ -5257,7 +5277,9 @@ class FormatterTest {
       |    .indigo
       |    .shine()
       |    .z { it }
-      |    .violet.cyan.magenta
+      |    .violet
+      |    .cyan
+      |    .magenta
       |    .key
       |""".trimMargin(),
           deduceMaxWidth = true)
@@ -5367,7 +5389,9 @@ class FormatterTest {
           """
       |-------------------------
       |z12.z { it }
-      |    .red.orange.yellow
+      |    .red
+      |    .orange
+      |    .yellow
       |    .green
       |    .blue
       |    .indigo
@@ -5385,7 +5409,9 @@ class FormatterTest {
           """
       |-------------------------
       |this.z { it }
-      |    .red.orange.yellow
+      |    .red
+      |    .orange
+      |    .yellow
       |    .green
       |    .blue
       |    .indigo

diff --git a/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt b/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt
@@ -663,7 +663,8 @@ class GoogleStyleFormatterKtTest {
       |
       |  // Similar to above.
       |  abcdefghijkl.abcdefghijkl
-      |    ?.methodName3?.abcdefghijkl()
+      |    ?.methodName3
+      |    ?.abcdefghijkl()
       |
       |  // Multiple call expressions cause each part of the expression
       |  // to be placed on its own line.