From fd042ed517645a2cbe0be0fd49e48204888fe501 Mon Sep 17 00:00:00 2001
From: David Torosyan <dtoro@fb.com>
Date: Thu, 13 Jan 2022 15:09:48 -0800
Subject: [PATCH] Don't break for short variables names (even for simple
 chains)

Summary:
## Context

After the changes in D33399531 (https://github.com/facebookincubator/ktfmt/commit/5164ad9c90a851b99c8b76dcd6fb33fcc40264b7), we were left with a few hard to understand variables which would get in the way of changing the newline logic. This aims to remove the `simple` boolean.

## Change

WARNING: This isn't a refactor - the final formatting is different (but for the better).

As described in the docs for `textLength`, we don't break if the first reference is short enough:
```
// shorter than 4 characters
z123.red
    .orange
    .yellow
    .green
    .blue
    .indigo
    .violet
    .cyan
    .magenta
    .key
```

However, this logic wasn't used whenever there were multiple invocations:
```
// shorter than 4 characters...
// but ends with a lambda and has more 2 invocations
// why should that matter?
z123
    .shine()
    .bright()
    .z { it }
```

Now it's done uniformly:
```
// shorter than 4 characters
z123.shine()
    .bright()
    .z { it }
```

Reviewed By: strulovich

Differential Revision: D33488790

fbshipit-source-id: 5eb47fd952d976bd0899a85e112fb4fa08eee379
---
 .../facebook/ktfmt/format/KotlinInputAstVisitor.kt   | 12 +-----------
 .../java/com/facebook/ktfmt/format/FormatterTest.kt  |  3 +--
 2 files changed, 2 insertions(+), 13 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
index ff304124..ee30e888 100644
--- a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt
+++ b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt
@@ -669,16 +669,6 @@ class KotlinInputAstVisitor(
     // is the last expression a lambda? e.g.`foo.bar.apply { ... }`
     val hasTrailingLambda = chunks.last().expressions.last().isLambda()
 
-    // Do we have any expressions that should be kept on the same line?
-    // True example: `rainbow.red.orange.shine()`
-    // False example: `rainbow.red.orange`
-    // Note that in this example the function invocation is what causes the chunk to be a "prefix",
-    // but there are other ways defined in [breakIntoChunks].
-    val hasPrefixes = chunks.first().shouldKeepOnSameLine
-
-    // simple chain with no meaningful groups or trailing lambda, e.g `rainbow.red.orange`
-    val simple = !hasPrefixes && !hasTrailingLambda
-
     // When the last chunk is meant to be on one line, reduce the indentation of arguments:
     // ```
     // rainbow.shine(
@@ -754,7 +744,7 @@ class KotlinInputAstVisitor(
           if (chunkIndex > 0 || itemIndex > 0) {
 
             // break if there's a lambda, or the line is long enough
-            if (!simple || textLength > options.continuationIndent || item.isLambda()) {
+            if (textLength > options.continuationIndent || item.isLambda()) {
               val fillMode =
                   if (chunk.shouldKeepOnSameLine) Doc.FillMode.INDEPENDENT else Doc.FillMode.UNIFIED
               builder.breakOp(fillMode, "", ZERO, Optional.of(nameTag))
diff --git a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt
index 57c5ec80..281dcdcb 100644
--- a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt
+++ b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt
@@ -4945,8 +4945,7 @@ class FormatterTest {
       assertFormatted(
           """
       |-------------
-      |z123
-      |    .shine()
+      |z123.shine()
       |    .bright()
       |    .z { it }
       |""".trimMargin(),