From 3dfe0c5609cdb7de23c804a8f5333a5775070a7e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= <olafurpg@gmail.com>
Date: Sat, 17 Dec 2016 15:54:58 +0000
Subject: [PATCH] Bump penalty for select chains of length 1.

Although the change itself is small, this commit changes thej
formatted output for a significant amount of code. An example
diff can be seen here: https://github.com/olafurpg/scala-repos/pull/14

Fixes #599.
---
 .../scala/org/scalafmt/config/Newlines.scala  | 21 +++++++++++++++++++
 .../scala/org/scalafmt/internal/Router.scala  | 21 ++++++++++++++-----
 core/src/test/resources/default/Select.stat   | 19 +++++++++++++++++
 3 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/core/src/main/scala/org/scalafmt/config/Newlines.scala b/core/src/main/scala/org/scalafmt/config/Newlines.scala
index e349df0fec..570d1514ca 100644
--- a/core/src/main/scala/org/scalafmt/config/Newlines.scala
+++ b/core/src/main/scala/org/scalafmt/config/Newlines.scala
@@ -15,6 +15,26 @@ import metaconfig.ConfigReader
   *       def foo = 2
   *     }
   *   )
+  *
+  * @param penalizeSingleSelectMultiArgList
+  *   If true, adds a penalty to newlines before a dot starting a select
+  *   chain of length one and argument list. The penalty matches the number
+  *   of arguments to the select chain application.
+  *   {{{
+  *     // If true, favor
+  *     logger.elem(a,
+  *                 b,
+  *                 c)
+  *     // instead of
+  *     logger
+  *       .elem(a, b, c)
+  *
+  *     // penalty is proportional to argument count, example:
+  *     logger.elem(a, b, c)    // penalty 2
+  *     logger.elem(a, b, c, d) // penalty 3, etc.
+  *   }}}
+  *
+  *   If false, matches <0.5 behavior. Note. this option may be removed in 1.0.
   */
 @ConfigReader
 case class Newlines(
@@ -22,5 +42,6 @@ case class Newlines(
     neverInResultType: Boolean = false,
     neverBeforeJsNative: Boolean = false,
     sometimesBeforeColonInMethodReturnType: Boolean = true,
+    penalizeSingleSelectMultiArgList: Boolean = true,
     alwaysBeforeCurlyBraceLambdaParams: Boolean = false
 )
diff --git a/core/src/main/scala/org/scalafmt/internal/Router.scala b/core/src/main/scala/org/scalafmt/internal/Router.scala
index f69eaca53c..9dfd681ec1 100644
--- a/core/src/main/scala/org/scalafmt/internal/Router.scala
+++ b/core/src/main/scala/org/scalafmt/internal/Router.scala
@@ -863,12 +863,23 @@ class Router(formatOps: FormatOps) {
         val newlinePolicy = breakOnEveryDot
           .andThen(penalizeNewlinesInApply.f)
           .copy(expire = lastToken.end)
-        Seq(
-          Split(NoSplit,
-                0,
-                ignoreIf = style.breakChainOnFirstMethodDot && newlines > 0)
+        val ignoreNoSplit = style.breakChainOnFirstMethodDot && newlines > 0
+        val chainLengthPenalty =
+          if (!style.bestEffortInDeeplyNestedCode && // breaks tests.
+              style.newlines.penalizeSingleSelectMultiArgList &&
+              chain.length < 2) {
+            // penalize by the number of arguments in the rhs open apply.
+            // I know, it's a bit arbitrary, but my manual experiments seem
+            // to show that it produces OK output.
+            splitApplyIntoLhsAndArgsLifted(owners(next(next(tok)).right)).map {
+              case (_, lst) => Math.max(0, lst.length - 1)
+            }.getOrElse(0)
+          } else 0
+        Seq(
+          Split(NoSplit, 0, ignoreIf = ignoreNoSplit)
             .withPolicy(noSplitPolicy),
-          Split(Newline.copy(acceptNoSplit = true), 2 + nestedPenalty)
+          Split(Newline.copy(acceptNoSplit = true),
+                2 + nestedPenalty + chainLengthPenalty)
             .withPolicy(newlinePolicy)
             .withIndent(2, optimalToken, Left)
         )
diff --git a/core/src/test/resources/default/Select.stat b/core/src/test/resources/default/Select.stat
index 43eabed092..257261e825 100644
--- a/core/src/test/resources/default/Select.stat
+++ b/core/src/test/resources/default/Select.stat
@@ -271,3 +271,22 @@ object a {
       computeOrReadCheckpoint(partition, context)
     })
 }
+<<< #599
+{{
+ logger
+      .elem(threp1, threp2, threp3, tax.total.value, tax.usedPersonalDiscount)
+}}
+>>>
+{
+  {
+    logger.elem(threp1,
+                threp2,
+                threp3,
+                tax.total.value,
+                tax.usedPersonalDiscount)
+  }
+}
+<<< No args #599
+  keys.clear() // we need to remove the selected keys from the set, otherwise they remain selected
+>>>
+keys.clear() // we need to remove the selected keys from the set, otherwise they remain selected