Router: penalize func arrow NL further

Increase arrow penalty on break after left brace, and also penalize it on break before preceding dot. Also, simplify the code a little bit, as nlPenalty computation was in completely superseded by the lambda penalty execution path.
scalameta · Nov 26, 2024 · 8b81ac1 · 8b81ac1
1 parent f0975ba
commit 8b81ac1
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 50 deletions.
diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala
@@ -439,23 +439,12 @@ class Router(formatOps: FormatOps) {
             }
           case _ => NoPolicy
         }
-        val nlPenalty = leftOwner match {
-          case _ if !style.newlines.fold => 0
-          case t: Term.ArgClause if (t.values match {
-                case x :: Nil if !x.is[Term.FunctionTerm] => true
-                case _ => false
-              }) => 1
-          case t @ Tree.Block(x :: Nil)
-              if !x.is[Term.FunctionTerm] && t.parent.is[Term.ArgClause] => 1
-          case _ => 0
-        }
         val (nlCost, nlArrowPenalty) =
           if (!nl.isNL) (0, 0)
-          else if (slbMod eq noSplitMod) (1 + nlPenalty, nlPenalty)
-          else getLambdaPenaltiesOnLeftBraceOnLeft(ft)
-            .fold((1, nlPenalty)) { case (sharedPenalty, herePenalty) =>
-              (sharedPenalty + herePenalty, sharedPenalty)
-            }
+          else {
+            if (slbMod eq noSplitMod) None
+            else getLambdaPenaltiesOnLeftBraceOnLeft(ft)
+          }.fold((1, 0)) { case (shared, here) => (shared + here, shared + 2) }
         val newlineBeforeClosingCurly = decideNewlinesOnlyBeforeClose(close)
         val nlPolicy = lambdaNLPolicy ==> newlineBeforeClosingCurly
         val nlSplit = Split(nl, nlCost, policy = nlPolicy)
@@ -476,7 +465,8 @@ class Router(formatOps: FormatOps) {
             val (mod, policy) = singleLineSplitOpt match {
               case Some(slbSplit) if singleLineSplit.isIgnored =>
                 val arrSplit = slbSplit.withMod(Space)
-                slbMod -> Policy.onRight(lambdaExpire, s"FNARR($arrSplit)") {
+                val fnarrDesc = s"FNARR($nlArrowPenalty;$arrSplit)"
+                slbMod -> Policy.onRight(lambdaExpire, fnarrDesc) {
                   case Decision(`lambdaExpire`, ss) =>
                     var hadNoSplit = false
                     val nlSplits = ss.flatMap { s =>
@@ -1929,6 +1919,17 @@ class Router(formatOps: FormatOps) {
                 val singleArg =
                   if (!ftAfterRight.right.is[T.OpenDelim]) None
                   else getSingleArgOnLeftBraceOnLeft(ftNextAfterRight).map(_._2)
+                val ftArrowOpt = singleArg match {
+                  case Some(t: Term.FunctionTerm) => getFuncArrow(t)
+                  case _ => None
+                }
+                val nlPolicy = ftArrowOpt.map { ftArrow =>
+                  val xft = nextNonCommentSameLine(ftArrow)
+                  Policy.End <= xft ==>
+                    Policy.onRight(xft, "PNL+ARR") { case Decision(`xft`, ss) =>
+                      ss.penalizeNL(1)
+                    }
+                }
                 val bracesToParens = singleArg.isDefined && {
                   implicit val ft: FT = ftNextAfterRight
                   val rb = matchingRight(ftAfterRight)
@@ -1937,7 +1938,9 @@ class Router(formatOps: FormatOps) {
                 }
                 val noSplit = Split(modSpace, 0)
                   .withSingleLine(end, exclude = exclude)
-                Seq(noSplit, nlSplitBase(if (bracesToParens) 0 else 1))
+                val nlCost = if (bracesToParens) 0 else 1
+                val nlSplit = nlSplitBase(nlCost, nlPolicy)
+                Seq(noSplit, nlSplit)
               }
             else {
               val policy: Policy = forcedBreakOnNextDotPolicy

diff --git a/scalafmt-tests/shared/src/test/resources/newlines/source_classic.stat b/scalafmt-tests/shared/src/test/resources/newlines/source_classic.stat
@@ -11079,9 +11079,9 @@ object a {
 >>>
 object a {
   def encodeCookieHeader(cookies: Seq[Cookie]): String =
-    encoder.encode(cookies.map { cookie =>
-      new DefaultCookie(cookie.name, cookie.value)
-    }.asJava)
+    encoder.encode(
+      cookies.map(cookie => new DefaultCookie(cookie.name, cookie.value)).asJava
+    )
 }
 <<< #4133 braces-to-parens with select, apply of nested match
 maxColumn = 76

diff --git a/scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat b/scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat
@@ -3099,17 +3099,17 @@ object a {
   def a(b: Int, d: Range): Set[Int] =
     // comment 1
     d.map(x => Foo.bar.baz(Bar.baz(x)))
-      .bar.baz.qux(foo(b, c))
-      .flatMap { y =>
-        // comment 2
-        val a: Int = y.getOrElse(
-          foo.bar,
-          throw new Exception(
-            "missing " + foo(y)
-          )
-        ).toInt
-        // comment 3
-        Some(a).filter(d.contains)
+      .bar.baz.qux(foo(b, c)).flatMap {
+        y =>
+          // comment 2
+          val a: Int = y.getOrElse(
+            foo.bar,
+            throw new Exception(
+              "missing " + foo(y)
+            )
+          ).toInt
+          // comment 3
+          Some(a).filter(d.contains)
       }
 }
 <<< #2113 if-apply 1
@@ -9155,14 +9155,14 @@ validateStructuralIntegrityWithReasonImpl(row, expectedSchema).map {
       s"UnsafeRow status: ${getStructuralIntegrityStatus(row, expectedSchema)}"
 }
 >>>
-validateStructuralIntegrityWithReasonImpl(row, expectedSchema)
-  .map { errorMessage =>
+validateStructuralIntegrityWithReasonImpl(row, expectedSchema).map {
+  errorMessage =>
     s"Error message is: $errorMessage, " +
       s"UnsafeRow status: ${getStructuralIntegrityStatus(
           row,
           expectedSchema
         )}"
-  }
+}
 <<< #4133 lambda in braces, overflow after brace 2
 maxColumn = 74
 ===
@@ -9440,7 +9440,7 @@ object a {
         }
   }
 }
->>> { stateVisits = 2250, stateVisits2 = 2250 }
+>>> { stateVisits = 2183, stateVisits2 = 2183 }
 object a {
   private object MemoMap {
     def make(implicit trace: Trace): UIO[MemoMap] = Ref.Synchronized
@@ -10333,10 +10333,9 @@ object a {
 }
 >>>
 object a {
-  def encodeCookieHeader(cookies: Seq[Cookie]): String = encoder
-    .encode(cookies.map { cookie =>
-      new DefaultCookie(cookie.name, cookie.value)
-    }.asJava)
+  def encodeCookieHeader(cookies: Seq[Cookie]): String = encoder.encode(
+    cookies.map(cookie => new DefaultCookie(cookie.name, cookie.value)).asJava
+  )
 }
 <<< #4133 braces-to-parens with select, apply of nested match
 maxColumn = 76
@@ -10561,7 +10560,6 @@ rewrite.redundantBraces.parensForOneLineApply = true
     (x.minBreaks, x.maxNest, -x.minNest, x.regex.fold(0)(-_.length))
   }
 >>>
-topLevelStatementBlankLines.filter(x => x.minNest <= x.maxNest)
-  .sortBy { x =>
-    (x.minBreaks, x.maxNest, -x.minNest, x.regex.fold(0)(-_.length))
-  }
+topLevelStatementBlankLines.filter(x => x.minNest <= x.maxNest).sortBy {
+  x => (x.minBreaks, x.maxNest, -x.minNest, x.regex.fold(0)(-_.length))
+}
diff --git a/scalafmt-tests/shared/src/test/resources/scala3/OptionalBraces_fold.stat b/scalafmt-tests/shared/src/test/resources/scala3/OptionalBraces_fold.stat
@@ -5931,8 +5931,8 @@ object a:
        .AnyRefMap[String, collection.mutable.ListBuffer[Path]]()
      val isJava12OrHigher = scala.util.Properties.isJavaAtLeast("12")
      rootsForRelease.foreach(root =>
-       Files.walk(root).iterator().asScala.filter(Files.isDirectory(_))
-         .foreach { p =>
+       Files.walk(root).iterator().asScala.filter(Files.isDirectory(_)).foreach {
+         p =>
             val moduleNamePathElementCount = if (isJava12OrHigher) 1 else 0
             if (
               p.getNameCount > root.getNameCount + moduleNamePathElementCount
@@ -5946,7 +5946,7 @@ object a:
                 new collection.mutable.ListBuffer
               ) += p
             }
-         }
+       }
      )
      index
    }
@@ -5964,8 +5964,8 @@ object a:
 >>>
 object a:
    rootsForRelease_foreach(root =>
-     Files.walkingIterator().useScalaFilter(Files__Is__Directory)
-       .foreach { p =>
+     Files.walkingIterator().useScalaFilter(Files__Is__Directory).foreach {
+       p =>
           val_moduleNamePathElementCount_е_0
           if (
             p_getNameCount_g_root_getNameCount_p_moduleNamePathElementCount
@@ -5975,7 +5975,7 @@ object a:
               packageDotted__new_collectionMutableListBuffer
             )
           }
-       }
+     }
    )
 <<< braces to parens in one-line apply: overflow with braces, fit with parens
 maxColumn = 32

diff --git a/scalafmt-tests/shared/src/test/scala/org/scalafmt/FormatTests.scala b/scalafmt-tests/shared/src/test/scala/org/scalafmt/FormatTests.scala
@@ -144,7 +144,7 @@ class FormatTests extends FunSuite with CanRunTests with FormatAssertions {
     val explored = Debug.explored.get()
     logger.debug(s"Total explored: $explored")
     if (!onlyUnit && !onlyManual)
-      assertEquals(explored, 1184830, "total explored")
+      assertEquals(explored, 1184446, "total explored")
     val results = debugResults.result()
     // TODO(olafur) don't block printing out test results.
     // I don't want to deal with scalaz's Tasks :'(