From c92e4d3aeca12e97ea90995fda663223eeecdb26 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Sun, 29 Sep 2024 11:47:15 -0700 Subject: [PATCH] BestFirstSearch: stop using State.allAltAreNL The behaviour is non-idempotent since splits depend on source formatting but might produce different breaks. --- .../scalafmt/internal/BestFirstSearch.scala | 18 +++++------ .../scala/org/scalafmt/internal/State.scala | 31 +++++++++---------- 2 files changed, 21 insertions(+), 28 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 f2a96455a..0525c03eb 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 @@ -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 index dbc56bc59..c57dc01ab 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 @@ -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 {