State: keep explicit penalty to detect overflow

Using cost and delayed penalty proved to be insufficient.
scalameta · Dec 28, 2021 · 0ec390e · 0ec390e
1 parent 96736fc
commit 0ec390e
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 7 deletions.
diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/BestFirstSearch.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/BestFirstSearch.scala
@@ -197,8 +197,9 @@ private class BestFirstSearch private (
                   if (null == nextNextState) null
                   else traverseSameLine(nextNextState, depth)
                 if (null != furtherState) {
-                  val delta = furtherState.totalCost - nextNextState.totalCost
-                  if (delta > Constants.ExceedColumnPenalty)
+                  val overflow =
+                    furtherState.appliedPenalty > nextNextState.appliedPenalty
+                  if (overflow)
                     Q.enqueue(nextNextState)
                   else {
                     optimalNotFound = false

diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/PolicySummary.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/PolicySummary.scala
@@ -4,7 +4,7 @@ import scala.meta.tokens.Token
 
 import org.scalafmt.util.LoggerOps
 
-class PolicySummary(val policies: Seq[Policy]) {
+class PolicySummary(val policies: Seq[Policy]) extends AnyVal {
   import LoggerOps._
 
   @inline def noDequeue = policies.exists(_.noDequeue)

diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/State.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/State.scala
@@ -23,6 +23,7 @@ final case class State(
     pushes: Seq[ActualIndent],
     column: Int,
     allAltAreNL: Boolean,
+    appliedPenalty: Int, // penalty applied from overflow
     delayedPenalty: Int // apply if positive, ignore otherwise
 ) {
 
@@ -117,6 +118,7 @@ final case class State(
       nextIndents,
       nextStateColumn,
       nextAllAltAreNL,
+      appliedPenalty + penalty,
       nextDelayedPenalty
     )
   }
@@ -258,7 +260,6 @@ final case class State(
     }
   }
 
-  def totalCost: Int = cost + math.max(0, delayedPenalty)
 }
 
 object State {
@@ -273,6 +274,7 @@ object State {
     Seq.empty,
     0,
     false,
+    0,
     0
   )
 

diff --git a/scalafmt-tests/src/test/resources/default/String.stat b/scalafmt-tests/src/test/resources/default/String.stat
@@ -185,7 +185,8 @@ object a {
 >>>
 object a {
   intercept[TestException] {
-    val ct = Thread.currentThread() // Ensure that the thunk is executed in the tests thread
+    val ct =
+      Thread.currentThread() // Ensure that the thunk is executed in the tests thread
     val ct =
       Thread.currentThread() // Ensure that the thunk is executed in the tests thread
   }

diff --git a/scalafmt-tests/src/test/resources/scalajs/Apply.stat b/scalafmt-tests/src/test/resources/scalajs/Apply.stat
@@ -941,8 +941,8 @@ object a {
 >>>
 object a {
   @tailrec
-  def constructOptimized(
-      revAlts: List[(js.Tree, js.Tree)], elsep: js.Tree): js.Tree = {
+  def constructOptimized(revAlts: List[(js.Tree, js.Tree)],
+      elsep: js.Tree): js.Tree = {
     revAlts match {
       case foo =>
         // cannot use flatMap due to tailrec