From 0ec390ef2341329cb156f22ce20143508cd520ca Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Tue, 28 Dec 2021 13:59:30 -0800 Subject: [PATCH] State: keep explicit penalty to detect overflow Using cost and delayed penalty proved to be insufficient. --- .../main/scala/org/scalafmt/internal/BestFirstSearch.scala | 5 +++-- .../src/main/scala/org/scalafmt/internal/PolicySummary.scala | 2 +- .../shared/src/main/scala/org/scalafmt/internal/State.scala | 4 +++- scalafmt-tests/src/test/resources/default/String.stat | 3 ++- scalafmt-tests/src/test/resources/scalajs/Apply.stat | 4 ++-- 5 files changed, 11 insertions(+), 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 index 544adc64e3..06cd546b81 100644 --- 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 index a2978e30af..f575f4a068 100644 --- 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 index 5aaa8dc09e..64a17e9545 100644 --- 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 index be398ee66d..176694d393 100644 --- 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 index 22172e0b85..e1d95dc9fe 100644 --- 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