BestFirstSearch: stop using State.allAltAreNL

The behaviour is non-idempotent since splits depend on source formatting but might produce different breaks.
scalameta · Sep 30, 2024 · bcfb7b5 · bcfb7b5
1 parent 0c673a0
commit bcfb7b5
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 45 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
@@ -97,9 +97,8 @@ private class BestFirstSearch private (range: Set[Range])(implicit
         if (curr.split != null && curr.split.isNL)
           if (
             emptyQueueSpots.contains(idx) ||
-            optimizer.dequeueOnNewStatements && curr.allAltAreNL &&
-            !(depth == 0 && noOptZone) &&
-            (leftTok.is[Token.KwElse] || statementStarts.contains(idx))
+            optimizer.dequeueOnNewStatements && !(depth == 0 &&
+              noOptZone) && statementStarts.contains(idx)
           ) Q.addGeneration()
 
         val noBlockClose = start == curr && 0 != maxCost || !noOptZone ||
@@ -117,7 +116,6 @@ private class BestFirstSearch private (range: Set[Range])(implicit
             )
 
           val actualSplit = getActiveSplits(splitToken, curr, maxCost)
-          val allAltAreNL = actualSplit.forall(_.isNL)
 
           var optimalNotFound = true
           val handleOptimalTokens = optimizer.acceptOptimalAtHints &&
@@ -144,8 +142,7 @@ private class BestFirstSearch private (range: Set[Range])(implicit
           }
 
           actualSplit.foreach { split =>
-            if (optimalNotFound)
-              processNextState(getNext(curr, split, allAltAreNL))
+            if (optimalNotFound) processNextState(getNext(curr, split))
             else sendEvent(split)
           }
         }
@@ -158,11 +155,11 @@ private class BestFirstSearch private (range: Set[Range])(implicit
   private def sendEvent(split: Split): Unit = initStyle.runner
     .event(FormatEvent.Enqueue(split))
 
-  private def getNext(state: State, split: Split, allAltAreNL: Boolean)(implicit
+  private def getNext(state: State, split: Split)(implicit
       style: ScalafmtConfig,
   ): State = {
     sendEvent(split)
-    state.next(split, nextAllAltAreNL = allAltAreNL)
+    state.next(split)
   }
 
   private def killOnFail(opt: OptimalToken, nextNextState: State = null)(
@@ -244,8 +241,7 @@ private class BestFirstSearch private (range: Set[Range])(implicit
         case Seq(split) =>
           if (split.isNL) Right(state)
           else {
-            implicit val nextState: State =
-              getNext(state, split, allAltAreNL = false)
+            implicit val nextState: State = getNext(state, split)
             traverseSameLine(nextState)
           }
         case ss
@@ -265,7 +261,7 @@ private class BestFirstSearch private (range: Set[Range])(implicit
     def newNlState: Option[State] = stateAsOptimal(state, splits).orElse(nlState)
     splits.filter(_.costWithPenalty == 0) match {
       case Seq(split) if !split.isNL =>
-        val nextState: State = getNext(state, split, allAltAreNL = false)
+        val nextState: State = getNext(state, split)
         if (nextState.split.costWithPenalty > 0) newNlState.toRight(state)
         else if (nextState.depth >= tokens.length) Right(nextState)
         else {

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
@@ -22,7 +22,6 @@ final case class State(
     indentation: Int,
     pushes: Seq[ActualIndent],
     column: Int,
-    allAltAreNL: Boolean,
     appliedPenalty: Int, // penalty applied from overflow
     delayedPenalty: Int, // apply if positive, ignore otherwise
     lineId: Int,
@@ -42,10 +41,9 @@ final case class State(
 
   /** Calculates next State given split at tok.
     */
-  def next(initialNextSplit: Split, nextAllAltAreNL: Boolean)(implicit
-      style: ScalafmtConfig,
-      tokens: FormatTokens,
-  ): State = {
+  def next(
+      initialNextSplit: Split,
+  )(implicit style: ScalafmtConfig, tokens: FormatTokens): State = {
     val tok = tokens(depth)
     val right = tok.right
 
@@ -118,18 +116,17 @@ final case class State(
     val splitWithPenalty = nextSplit.withPenalty(penalty)
 
     State(
-      cost + splitWithPenalty.costWithPenalty,
+      cost = cost + splitWithPenalty.costWithPenalty,
       // TODO(olafur) expire policy, see #18.
-      nextPolicy,
-      splitWithPenalty,
-      depth + 1,
-      this,
-      nextIndent,
-      nextIndents,
-      nextStateColumn,
-      nextAllAltAreNL,
-      appliedPenalty + penalty,
-      nextDelayedPenalty,
+      policy = nextPolicy,
+      split = splitWithPenalty,
+      depth = depth + 1,
+      prev = this,
+      indentation = nextIndent,
+      pushes = nextIndents,
+      column = nextStateColumn,
+      appliedPenalty = appliedPenalty + penalty,
+      delayedPenalty = nextDelayedPenalty,
       lineId = lineId + (if (nextSplit.isNL) 1 else 0),
     )
   }
@@ -318,7 +315,7 @@ final case class State(
 object State {
 
   val start =
-    State(0, PolicySummary.empty, null, 0, null, 0, Seq.empty, 0, false, 0, 0, 0)
+    State(0, PolicySummary.empty, null, 0, null, 0, Seq.empty, 0, 0, 0, 0)
 
   // this is not best state, it's higher priority for search
   object Ordering {

diff --git a/scalafmt-tests/shared/src/test/resources/newlines/source_classic.stat b/scalafmt-tests/shared/src/test/resources/newlines/source_classic.stat
@@ -9770,7 +9770,8 @@ validateStructuralIntegrityWithReasonImpl(row, expectedSchema).map {
       s"UnsafeRow status: ${getStructuralIntegrityStatus(row, expectedSchema)}"
 }
 >>>
-validateStructuralIntegrityWithReasonImpl(row, expectedSchema).map { errorMessage =>
-  s"Error message is: $errorMessage, " +
-    s"UnsafeRow status: ${getStructuralIntegrityStatus(row, expectedSchema)}"
+validateStructuralIntegrityWithReasonImpl(row, expectedSchema).map {
+  errorMessage =>
+    s"Error message is: $errorMessage, " +
+      s"UnsafeRow status: ${getStructuralIntegrityStatus(row, expectedSchema)}"
 }
diff --git a/scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat b/scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat
@@ -3112,17 +3112,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
@@ -9164,11 +9164,11 @@ 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
         )}"
-  }
+}