BestFirstSearch: relax cost check w/ optimal token

scalafmt-core/shared/src/main/scala/org/scalafmt/internal/BestFirstSearch.scala

@@ -154,10 +154,14 @@ private class BestFirstSearch private (range: Set[Range])(implicit
             if (cost <= maxCost) {
               val stateToQueue = split.optimalAt match {
                 case Some(opt) if handleOptimalTokens =>
-                  if (cost > 0) killOnFail(opt)
-                  else processOptimalToken(opt) match {
-                    case Left(x) => x
-                    case Right(x) => optimalFound = true; x
+                  val killOnFail = willKillOnFail(opt.killOnFail, opt.token)
+                  def overNonPolicyCost = split.costWithoutPenalty > 0 ||
+                    nextState.appliedPenalty > curr.appliedPenalty
+                  if (killOnFail && overNonPolicyCost) null
+                  else processOptimalToken(opt, killOnFail) match {
+                    case Left(x) if x == null || cost <= 0 => x
+                    case Left(_) => if (killOnFail) null else nextState
+                    case Right(x) => if (cost <= 0) optimalFound = true; x
                   }
                 case _ => nextState
               }
@@ -198,10 +202,8 @@ private class BestFirstSearch private (range: Set[Range])(implicit
       nextState: State,
   ): Boolean = kill || nextState.hasSlbUntil(end)
 
-  private def killOnFail(opt: OptimalToken)(implicit nextState: State): State =
-    if (willKillOnFail(opt.killOnFail, opt.token)) null else nextState
-
-  private def processOptimalToken(opt: OptimalToken)(implicit
+  private def processOptimalToken(opt: OptimalToken, killOnFail: Boolean)(
+      implicit
       nextState: State,
       queue: StateQueue,
       style: ScalafmtConfig,
@@ -210,7 +212,7 @@ private class BestFirstSearch private (range: Set[Range])(implicit
     val nextNextState =
       if (optIdx <= nextState.depth) nextState
       else if (tokens.width(nextState.depth, optIdx) > 3 * style.maxColumn)
-        return Left(killOnFail(opt))
+        return Left(if (killOnFail) null else nextState)
       else {
         val res = shortestPath(
           nextState,

scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Split.scala

@@ -65,6 +65,8 @@ case class Split(
     case _ => this
   }
 
+  @inline
+  def costWithoutPenalty: Int = cost
   @inline
   def costWithPenalty: Int = cost + penalty.max(0)
 

scalafmt-tests/shared/src/test/resources/default/Advanced.stat

@@ -377,7 +377,7 @@ private def withNewLocalDefs = {
             }))
         }))
   }
->>> { stateVisits = 5621, stateVisits2 = 5621 }
+>>> { stateVisits = 5554 }
 val createIsArrayOfStat = {
   envFieldDef(
       "isArrayOf",

scalafmt-tests/shared/src/test/resources/default/Apply.stat

@@ -273,7 +273,7 @@ class ResolutionCopier(x: Int) {
             toNode.rightBracket));
   }
 }
->>> { stateVisits = 57387, stateVisits2 = 57387 }
+>>> { stateVisits = 18173 }
 class ResolutionCopier(x: Int) {
 
   def visitClassDeclaration(node: ClassDeclaration): Boolean = {

scalafmt-tests/shared/src/test/resources/default/String.stat

@@ -723,11 +723,10 @@ println("""|
   |""".stripMargin
 )
 >>>
-println(
-    """|
-       |A very long string
-       |with multiple lines
-       |""".stripMargin)
+println("""|
+           |A very long string
+           |with multiple lines
+           |""".stripMargin)
 <<< #4067 avoid classic
 preset = default
 maxColumn = 120

scalafmt-tests/shared/src/test/resources/newlines/source.source

@@ -69,7 +69,7 @@ private[parser] trait CacheControlHeader { this: Parser with CommonRules with Co
     clearSB() ~ zeroOrMore(!'"' ~ !',' ~ qdtext ~ appendSB() | `quoted-pair`) ~ push(sb.toString)
   }
 }
->>> { stateVisits = 35486, stateVisits2 = 35486 }
+>>> { stateVisits = 35487 }
 /** Copyright (C) 2009-2016 Lightbend Inc. <http://www.lightbend.com>
   */
 

scalafmt-tests/shared/src/test/resources/newlines/source_classic.stat

@@ -8897,7 +8897,7 @@ object a {
     )
   )
 }
->>> { stateVisits = 4176, stateVisits2 = 3915 }
+>>> { stateVisits = 4348, stateVisits2 = 4087 }
 object a {
   div(cls := "cover")(
     div(cls := "doc")(bodyContents),
@@ -9399,7 +9399,7 @@ class UDFRegistration private[sql] (functionRegistry: FunctionRegistry) extends
     val inputEncoders: Seq[Option[ExpressionEncoder[_]]] = Try(ExpressionEncoder[A1]()).toOption :: Try(ExpressionEncoder[A2]()).toOption :: Try(ExpressionEncoder[A3]()).toOption :: Try(ExpressionEncoder[A4]()).toOption :: Try(ExpressionEncoder[A5]()).toOption :: Try(ExpressionEncoder[A6]()).toOption :: Try(ExpressionEncoder[A7]()).toOption :: Try(ExpressionEncoder[A8]()).toOption :: Try(ExpressionEncoder[A9]()).toOption :: Try(ExpressionEncoder[A10]()).toOption :: Try(ExpressionEncoder[A11]()).toOption :: Try(ExpressionEncoder[A12]()).toOption :: Try(ExpressionEncoder[A13]()).toOption :: Try(ExpressionEncoder[A14]()).toOption :: Try(ExpressionEncoder[A15]()).toOption :: Try(ExpressionEncoder[A16]()).toOption :: Try(ExpressionEncoder[A17]()).toOption :: Try(ExpressionEncoder[A18]()).toOption :: Try(ExpressionEncoder[A19]()).toOption :: Try(ExpressionEncoder[A20]()).toOption :: Try(ExpressionEncoder[A21]()).toOption :: Try(ExpressionEncoder[A22]()).toOption :: Nil
   }
 }
->>> { stateVisits = 2626, stateVisits2 = 1038 }
+>>> { stateVisits = 2649, stateVisits2 = 1038 }
 class UDFRegistration private[sql] (
     functionRegistry: FunctionRegistry
 ) extends Logging {
@@ -9630,7 +9630,7 @@ class UDFRegistration {
     val inputEncoders: Seq[Option[ExpressionEncoder[_]]] = Try(ExpressionEncoder[A1]).toOption :: Try(ExpressionEncoder[A2]).toOption :: Try(ExpressionEncoder[A3]).toOption :: Try(ExpressionEncoder[A4]).toOption :: Try(ExpressionEncoder[A5]).toOption :: Try(ExpressionEncoder[A6]).toOption :: Try(ExpressionEncoder[A7]).toOption :: Try(ExpressionEncoder[A8]).toOption :: Try(ExpressionEncoder[A9]).toOption :: Try(ExpressionEncoder[A10]).toOption :: Try(ExpressionEncoder[A11]).toOption :: Try(ExpressionEncoder[A12]).toOption :: Try(ExpressionEncoder[A13]).toOption :: Try(ExpressionEncoder[A14]).toOption :: Try(ExpressionEncoder[A15]).toOption :: Try(ExpressionEncoder[A16]).toOption :: Try(ExpressionEncoder[A17]).toOption :: Try(ExpressionEncoder[A18]).toOption :: Try(ExpressionEncoder[A19]).toOption :: Try(ExpressionEncoder[A20]).toOption :: Try(ExpressionEncoder[A21]).toOption :: Try(ExpressionEncoder[A22]).toOption :: Nil
   }
 }
->>> { stateVisits = 2164, stateVisits2 = 748 }
+>>> { stateVisits = 2186, stateVisits2 = 748 }
 class UDFRegistration {
   def foo = {
     val inputEncoders: Seq[
@@ -10472,7 +10472,7 @@ object A {
         (Seq(), Seq())
   ): Unit = ???
 }
-<<< #4219
+<<< #4219 !avoidForSimpleOverflow
 object a {
   val expression = for (
     x <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu;
@@ -10488,6 +10488,23 @@ object a {
       y <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu
     ) yield x + y
 }
+<<< #4219 avoidForSimpleOverflow
+newlines.avoidForSimpleOverflow = all
+===
+object a {
+  val expression = for (
+    x <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu;
+    y <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu
+  ) yield x + y
+}
+>>>
+object a {
+  val expression =
+    for (
+      x <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu;
+      y <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu
+    ) yield x + y
+}
 <<< #4133 select chain with no-break comment before dot, short
 maxColumn = 37
 ===
@@ -10905,7 +10922,9 @@ system.dynamicAccess.createInstanceFor[Serializer](fqn, Nil).recoverWith {
                 Failure(
                   new NoSuchMethodException(
                     s"The serializer [$fqn] for binding [$bindingName] doesn't have a matching " +
-                      s"constructor, see API documentation of ${classOf[Serializer].getName}"
+                      s"constructor, see API documentation of ${classOf[
+                          Serializer
+                        ].getName}"
                   )
                 )
               }

scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat

@@ -6822,7 +6822,7 @@ object a {
     }
   ))
 }
->>> { stateVisits = 2070, stateVisits2 = 2070 }
+>>> { stateVisits = 2077 }
 object a {
   buffer.append((
     if (!fetchContent) {
@@ -8341,7 +8341,7 @@ object a {
     )
   )
 }
->>> { stateVisits = 4702, stateVisits2 = 4702 }
+>>> { stateVisits = 4736 }
 object a {
   div(cls := "cover")(
     div(cls := "doc")(bodyContents),
@@ -8428,7 +8428,7 @@ object a {
     )
   )
 }
->>> { stateVisits = 3882, stateVisits2 = 3882 }
+>>> { stateVisits = 3882 }
 object a {
   div(cls := "cover")(
     div(cls := "doc")(bodyContents),
@@ -9603,7 +9603,7 @@ object a {
       .map(_.filterNot(_.getCanonicalPath.contains("SSLOptions")))
   }
 }
->>> { stateVisits = 52849, stateVisits2 = 52849 }
+>>> { stateVisits = 52896 }
 object a {
   private def ignoreUndocumentedPackages(
       packages: Seq[Seq[File]]
@@ -9762,7 +9762,24 @@ object A {
         (Seq(), Seq())
   ): Unit = ???
 }
-<<< #4219
+<<< #4219 !avoidForSimpleOverflow
+object a {
+  val expression = for (
+    x <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu;
+    y <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu
+  ) yield x + y
+}
+>>>
+object a {
+  val expression = for (
+    x <-
+      loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu;
+    y <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu
+  ) yield x + y
+}
+<<< #4219 avoidForSimpleOverflow
+newlines.avoidForSimpleOverflow = all
+===
 object a {
   val expression = for (
     x <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu;
@@ -10151,7 +10168,7 @@ system.dynamicAccess.createInstanceFor[Serializer](fqn, Nil).recoverWith {
             }
       }
 }
->>> { stateVisits = 3689, stateVisits2 = 3689 }
+>>> { stateVisits = 3717 }
 system.dynamicAccess.createInstanceFor[Serializer](fqn, Nil).recoverWith {
   case _: NoSuchMethodException => system.dynamicAccess
       .createInstanceFor[Serializer](
@@ -10772,7 +10789,7 @@ object ZStreamSpec extends DefaultRunnableSpec {
     )
 
 }
->>> { stateVisits = 6608, stateVisits2 = 6608 }
+>>> { stateVisits = 6630 }
 object ZStreamSpec extends DefaultRunnableSpec {
   def spec: ZSpec[Environment, Failure] = suite("ZStreamSpec")(
     suite("Combinators")(

scalafmt-tests/shared/src/test/resources/newlines/source_keep.stat

@@ -10228,7 +10228,25 @@ object A {
   def f(a: (Seq[Int], Seq[Int]) =
     (Seq(), Seq())): Unit = ???
 }
-<<< #4219
+<<< #4219 !avoidForSimpleOverflow
+object a {
+  val expression = for (
+    x <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu;
+    y <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu
+  ) yield x + y
+}
+>>>
+object a {
+  val expression = for (
+    x <-
+      loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu;
+    y <-
+      loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu
+  ) yield x + y
+}
+<<< #4219 avoidForSimpleOverflow
+newlines.avoidForSimpleOverflow = all
+===
 object a {
   val expression = for (
     x <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu;

scalafmt-tests/shared/src/test/resources/newlines/source_unfold.stat

@@ -10601,7 +10601,27 @@ object A {
         (Seq(), Seq())
   ): Unit = ???
 }
-<<< #4219
+<<< #4219 !avoidForSimpleOverflow
+object a {
+  val expression = for (
+    x <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu;
+    y <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu
+  ) yield x + y
+}
+>>>
+object a {
+  val expression =
+    for (
+      x <-
+        loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu;
+      y <-
+        loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu
+    )
+      yield x + y
+}
+<<< #4219 avoidForSimpleOverflow
+newlines.avoidForSimpleOverflow = all
+===
 object a {
   val expression = for (
     x <- loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididu;

scalafmt-tests/shared/src/test/resources/scala3/OptionalBraces_fold.stat

@@ -7195,7 +7195,7 @@ final case class UserCheck(
           case b =>
         }
     )
->>> { stateVisits = 3818, stateVisits2 = 3818 }
+>>> { stateVisits = 3831 }
 final case class UserCheck(
     options: PublishSetupOptions,
     configDb: () => ConfigDb,

scalafmt-tests/shared/src/test/resources/unit/DefDef.stat

@@ -245,9 +245,10 @@ object a {
 object a {
   def zipMethodsWithTheirInlinedVersionsFromParent(
       prog: EOProg[EOExprOnly])
-      : EitherNel[String,
-                  Map[EONamedBnd,
-                      ObjectTree[(AnalysisInfo, AnalysisInfo)]]] = {
+      : EitherNel[
+          String,
+          Map[EONamedBnd,
+              ObjectTree[(AnalysisInfo, AnalysisInfo)]]] = {
     ???
   }
 }

scalafmt-tests/shared/src/test/scala/org/scalafmt/FormatTests.scala

@@ -148,7 +148,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, 2536352, "total explored")
+      assertEquals(explored, 2384300, "total explored")
     // TODO(olafur) don't block printing out test results.
     TestPlatformCompat.executeAndWait(PlatformFileOps.writeFile(
       FileOps.getPath("target", "index.html"),