Router: penalize func arrow NL further

Increase arrow penalty on break after left brace or paren, and penalize arrows on break before preceding dot.
scalameta · Nov 27, 2024 · d38fa85 · d38fa85
1 parent 8c6e447
commit d38fa85
Show file tree

Hide file tree

Showing 12 changed files with 72 additions and 63 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,7 +439,7 @@ class Router(formatOps: FormatOps) {
           else {
             if (slbMod eq noSplitMod) None
             else getLambdaPenaltiesOnLeftBraceOnLeft(ft)
-          }.fold((1, 0)) { case (shared, here) => (shared + here, shared) }
+          }.fold((1, 0)) { case (shared, here) => (shared + here, shared + 1) }
         val newlineBeforeClosingCurly = decideNewlinesOnlyBeforeClose(close)
         val nlPolicy = lambdaNLPolicy ==> newlineBeforeClosingCurly
         val nlSplit = Split(nl, nlCost, policy = nlPolicy)
@@ -895,9 +895,9 @@ class Router(formatOps: FormatOps) {
               else Seq(Split(Space(style.spaces.inParentheses), 0))
             }
             Policy ? lambdaIsABlock ||
-            Policy.RelayOnSplit.by(Policy.End <= lambdaLeft.getOrElse(close))(
-              (s, _) => s.isNL,
-            )(before)(newlinePolicy)
+            penalizeOneNewline(lambdaToken, 1) & Policy.RelayOnSplit.by(
+              Policy.End <= lambdaLeft.getOrElse(close),
+            )((s, _) => s.isNL)(before)(newlinePolicy)
           }
           Split(noSplitMod, 0, policy = spacePolicy)
             .withOptimalToken(lambdaToken, killOnFail = true)
@@ -1906,6 +1906,21 @@ class Router(formatOps: FormatOps) {
                 }
                 val exclude = insideBracesBlock(ft, end, parensToo = true)
                   .excludeCloseDelim
+                val arrowPolicy = exclude.ranges.map { tr =>
+                  Policy.End <= tr.lt ==> Policy.onRight(tr.rt, "PNL+DOTARR") {
+                    case Decision(FT(_: T.FunctionArrow, r, m), ss)
+                        if !r.is[T.Comment] &&
+                          m.leftOwner.is[Term.FunctionTerm] &&
+                          findTreeWithParent(m.leftOwner) {
+                            case _: Member.Apply => Some(true)
+                            case p: Term.ArgClause
+                                if !isSeqSingle(p.values) => Some(false)
+                            case p: Tree.Block
+                                if !isSeqSingle(p.stats) => Some(false)
+                            case _ => None
+                          }.isDefined => ss.penalizeNL(1)
+                  }
+                }.foldLeft(Policy.noPolicy) { case (res, pol) => pol ==> res }
                 val ftNextAfterRight = next(ftAfterRight)
                 val singleArg =
                   if (!ftAfterRight.right.is[T.OpenDelim]) None
@@ -1918,7 +1933,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, arrowPolicy)
+                Seq(noSplit, nlSplit)
               }
             else {
               val policy: Policy = forcedBreakOnNextDotPolicy

diff --git a/...intellij/src/test/scala/org/scalafmt/community/intellij/CommunityIntellijScalaSuite.scala b/...intellij/src/test/scala/org/scalafmt/community/intellij/CommunityIntellijScalaSuite.scala
@@ -13,7 +13,7 @@ abstract class CommunityIntellijScalaSuite(name: String)
 class CommunityIntellijScala_2024_2_Suite
     extends CommunityIntellijScalaSuite("intellij-scala-2024.2") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(59317488)
+  override protected def totalStatesVisited: Option[Int] = Some(59317379)
 
   override protected def builds = Seq(getBuild(
     "2024.2.28",
@@ -51,7 +51,7 @@ class CommunityIntellijScala_2024_2_Suite
 class CommunityIntellijScala_2024_3_Suite
     extends CommunityIntellijScalaSuite("intellij-scala-2024.3") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(59533781)
+  override protected def totalStatesVisited: Option[Int] = Some(59534082)
 
   override protected def builds = Seq(getBuild(
     "2024.3.4",

diff --git a/...-community/scala2/src/test/scala/org/scalafmt/community/scala2/CommunityScala2Suite.scala b/...-community/scala2/src/test/scala/org/scalafmt/community/scala2/CommunityScala2Suite.scala
@@ -9,7 +9,7 @@ abstract class CommunityScala2Suite(name: String)
 
 class CommunityScala2_12Suite extends CommunityScala2Suite("scala-2.12") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(42542831)
+  override protected def totalStatesVisited: Option[Int] = Some(42550468)
 
   override protected def builds =
     Seq(getBuild("v2.12.20", dialects.Scala212, 1277))
@@ -18,7 +18,7 @@ class CommunityScala2_12Suite extends CommunityScala2Suite("scala-2.12") {
 
 class CommunityScala2_13Suite extends CommunityScala2Suite("scala-2.13") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(53024660)
+  override protected def totalStatesVisited: Option[Int] = Some(53037336)
 
   override protected def builds =
     Seq(getBuild("v2.13.14", dialects.Scala213, 1287))

diff --git a/...-community/scala3/src/test/scala/org/scalafmt/community/scala3/CommunityScala3Suite.scala b/...-community/scala3/src/test/scala/org/scalafmt/community/scala3/CommunityScala3Suite.scala
@@ -9,15 +9,15 @@ abstract class CommunityScala3Suite(name: String)
 
 class CommunityScala3_2Suite extends CommunityScala3Suite("scala-3.2") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(39220760)
+  override protected def totalStatesVisited: Option[Int] = Some(39222286)
 
   override protected def builds = Seq(getBuild("3.2.2", dialects.Scala32, 791))
 
 }
 
 class CommunityScala3_3Suite extends CommunityScala3Suite("scala-3.3") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(42405381)
+  override protected def totalStatesVisited: Option[Int] = Some(42407030)
 
   override protected def builds = Seq(getBuild("3.3.3", dialects.Scala33, 861))
 

diff --git a/...sts-community/spark/src/test/scala/org/scalafmt/community/spark/CommunitySparkSuite.scala b/...sts-community/spark/src/test/scala/org/scalafmt/community/spark/CommunitySparkSuite.scala
@@ -9,7 +9,7 @@ abstract class CommunitySparkSuite(name: String)
 
 class CommunitySpark3_4Suite extends CommunitySparkSuite("spark-3.4") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(86363674)
+  override protected def totalStatesVisited: Option[Int] = Some(86349533)
 
   override protected def builds =
     Seq(getBuild("v3.4.1", dialects.Scala213, 2585))
@@ -18,7 +18,7 @@ class CommunitySpark3_4Suite extends CommunitySparkSuite("spark-3.4") {
 
 class CommunitySpark3_5Suite extends CommunitySparkSuite("spark-3.5") {
 
-  override protected def totalStatesVisited: Option[Int] = Some(91365411)
+  override protected def totalStatesVisited: Option[Int] = Some(91349741)
 
   override protected def builds =
     Seq(getBuild("v3.5.3", dialects.Scala213, 2756))

diff --git a/scalafmt-tests/shared/src/test/resources/newlines/source_classic.stat b/scalafmt-tests/shared/src/test/resources/newlines/source_classic.stat
@@ -7919,9 +7919,8 @@ class a {
 }
 >>>
 class a {
-  def this(settings: Settings, eventStream: EventStream) = this(() ⇒
-    eventStream.logLevel
-  )
+  def this(settings: Settings, eventStream: EventStream) =
+    this(() ⇒ eventStream.logLevel)
 }
 <<< SM 7.4.0: 47.2
 maxColumn = 76
@@ -10096,9 +10095,8 @@ object a {
                       observers <- Ref.make(0)
                       promise <- Promise
                         .make[E, (FiberRefs.Patch, ZEnvironment[B])]
-                      finalizerRef <- Ref.make[Exit[Any, Any] => UIO[Any]](_ =>
-                        ZIO.unit
-                      )
+                      finalizerRef <- Ref
+                        .make[Exit[Any, Any] => UIO[Any]](_ => ZIO.unit)
 
                       resource = ZIO.uninterruptibleMask { restore =>
                         for {
@@ -11079,9 +11077,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
@@ -7380,9 +7380,8 @@ class a {
 }
 >>>
 class a {
-  def this(settings: Settings, eventStream: EventStream) = this(() ⇒
-    eventStream.logLevel
-  )
+  def this(settings: Settings, eventStream: EventStream) =
+    this(() ⇒ eventStream.logLevel)
 }
 <<< SM 7.4.0: 47.2
 maxColumn = 76
@@ -9155,14 +9154,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
 ===
@@ -9200,10 +9199,9 @@ object Build {
         .fold(identity[GenerationConfig](_)) {
           newDir => config: GenerationConfig => config.add(OutputDir(newDir))
         }
-      val justAPI = justAPIArg
-        .fold(identity[GenerationConfig](_)) { _ => config: GenerationConfig =>
-          config.remove[SiteRoot]
-        }
+      val justAPI = justAPIArg.fold(identity[GenerationConfig](_)) {
+        _ => config: GenerationConfig => config.remove[SiteRoot]
+      }
     }.evaluated)
 }
 <<< #4133 monads with binpack and no newlines in types
@@ -9440,7 +9438,7 @@ object a {
         }
   }
 }
->>> { stateVisits = 2229, stateVisits2 = 2229 }
+>>> { stateVisits = 2151, stateVisits2 = 2151 }
 object a {
   private object MemoMap {
     def make(implicit trace: Trace): UIO[MemoMap] = Ref.Synchronized
@@ -10201,12 +10199,12 @@ val offsets = Utils.tryWithResource {
   buffer.asLongBuffer
 }
 >>>
-val offsets = Utils
-  .tryWithResource(new DataInputStream(Files.newInputStream(indexFile.toPath))) {
-    dis =>
-      dis.readFully(buffer.array)
-      buffer.asLongBuffer
-  }
+val offsets = Utils.tryWithResource {
+  new DataInputStream(Files.newInputStream(indexFile.toPath))
+} { dis =>
+  dis.readFully(buffer.array)
+  buffer.asLongBuffer
+}
 <<< #4133 braces to parens with two curried params, second func and comment
 maxColumn = 76
 newlines.avoidForSimpleOverflow = all
@@ -10333,10 +10331,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 +10558,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/resources/scalajs/Apply.stat b/scalafmt-tests/shared/src/test/resources/scalajs/Apply.stat
@@ -1399,9 +1399,9 @@ object a {
     captures = Nil)(prevArgsCount => varDefs.take(prevArgsCount).toList.map(_.ref))
   val rhs = genScalaArg(sym, index, formalArgsRegistry, param, static)(
     prevArgsCount => varDefs.take(prevArgsCount).toList.map(_.ref))
-  val rhs = genScalaArg(prevArgsCount =>
-    varDefs.take(prevArgsCount).toList.map(_.ref))(
-    sym, index, formalArgsRegistry, param, static, captures = Nil)
+  val rhs =
+    genScalaArg(prevArgsCount => varDefs.take(prevArgsCount).toList.map(_.ref))(
+      sym, index, formalArgsRegistry, param, static, captures = Nil)
 }
 <<< #4133 overflow with nested lambda and braces 1
 Block(norm.tail.map(sym => DefDef(sym, { vparamss: List[List[Symbol]] => EmptyTree })), ddef)

diff --git a/...t-tests/shared/src/test/resources/trailing-commas/trailingCommasAlwaysDanglingParens.stat b/...t-tests/shared/src/test/resources/trailing-commas/trailingCommasAlwaysDanglingParens.stat
@@ -265,9 +265,8 @@ object Foo {
     aaaaaaaaaaaaaaa(c, ddddddd).map { zzzzzzzzzz =>
       val ccccc = zz.bbbbbb(EEEEE.xxx, Schema.String).toOption.flatMap {
         case c: Seq[String] =>
-          xxxx(c, (v: String) => s"""\\"$v\\"""")(Schema.String).map(s =>
-            Yyyyyyyyyyyyyyyy("SSSSSSS", s),
-          )
+          xxxx(c, (v: String) => s"""\\"$v\\"""")(Schema.String)
+            .map(s => Yyyyyyyyyyyyyyyy("SSSSSSS", s))
       }
     }.get
   }

diff --git a/...tests/shared/src/test/resources/trailing-commas/trailingCommasMultipleDanglingParens.stat b/...tests/shared/src/test/resources/trailing-commas/trailingCommasMultipleDanglingParens.stat
@@ -265,9 +265,8 @@ object Foo {
     aaaaaaaaaaaaaaa(c, ddddddd).map { zzzzzzzzzz =>
       val ccccc = zz.bbbbbb(EEEEE.xxx, Schema.String).toOption.flatMap {
         case c: Seq[String] =>
-          xxxx(c, (v: String) => s"""\\"$v\\"""")(Schema.String).map(s =>
-            Yyyyyyyyyyyyyyyy("SSSSSSS", s)
-          )
+          xxxx(c, (v: String) => s"""\\"$v\\"""")(Schema.String)
+            .map(s => Yyyyyyyyyyyyyyyy("SSSSSSS", s))
       }
     }.get
   }

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, 1186090, "total explored")
+      assertEquals(explored, 1187430, "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 :'(