Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BestFirstSearch: stop using State.allAltAreNL #4369

Merged
merged 3 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand All @@ -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 &&
Expand All @@ -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)
}
}
Expand All @@ -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)(
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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

Expand Down Expand Up @@ -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),
)
}
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,30 +191,10 @@ object TreeOps {
addDefn[KwDef](t.mods, t)
addAll(t.stats)
// special handling for rewritten blocks
case t @ Term.Block(arg :: Nil) // single-stat block
if t.tokens.headOption // see if opening brace was removed
.exists(x => x.is[Token.LeftBrace] && ftoks(x).left.ne(x)) =>
if (arg.is[Term.FunctionTerm]) {
// handle rewritten apply { { x => b } } to a { x => b }
val parentApply = findTreeWithParent(t) {
case _: Term.Block => None
case _: Term.FunctionTerm => None
case p @ Term.ArgClause(_ :: Nil, _) => Some(isParentAnApply(p))
case _ => Some(false)
}
if (parentApply.isDefined) addOne(arg)
}
// special handling for rewritten apply(x => { b }) to a { x => b }
case t: Term.Apply =>
val ac = t.argClause
ac.values match {
case (f: Term.FunctionTerm) :: Nil if ac.tokens.lastOption.exists {
x => // see if closing paren is now brace
x.is[Token.RightParen] && ftoks.prevNonComment(ftoks(x))
.left.is[Token.RightBrace]
} => addOne(f)
case _ =>
}
case t @ Term.Block(_ :: Nil) if t.tokens.headOption.exists { x =>
// ignore single-stat block if opening brace was removed
x.is[Token.LeftBrace] && ftoks(x).left.ne(x)
} =>
case t => // Nothing
addAll(extractStatementsIfAny(t))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9787,3 +9787,36 @@ val finalProperties =
loc
))
}
<<< #4133 lambda in parens rewritten to braces
maxColumn = 80
rewrite.rules = [RedundantBraces]
===
object Build {
lazy val scaladoc = project.in(file("scaladoc")).
settings(
generateScalaDocumentation := Def.inputTaskDyn {
val outputDirOverride = extraArgs.headOption.fold(identity[GenerationConfig](_))(newDir => {
config: GenerationConfig => config.add(OutputDir(newDir))
})
val justAPI = justAPIArg.fold(identity[GenerationConfig](_))(_ => {
config: GenerationConfig => config.remove[SiteRoot]
})
}.evaluated,
)
}
>>>
object Build {
lazy val scaladoc = project
.in(file("scaladoc"))
.settings(
generateScalaDocumentation := Def.inputTaskDyn {
val outputDirOverride =
extraArgs.headOption.fold(identity[GenerationConfig](_)) {
newDir => config: GenerationConfig => config.add(OutputDir(newDir))
}
val justAPI = justAPIArg.fold(identity[GenerationConfig](_)) {
_ => config: GenerationConfig => config.remove[SiteRoot]
}
}.evaluated
)
}
31 changes: 31 additions & 0 deletions scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat
Original file line number Diff line number Diff line change
Expand Up @@ -9184,3 +9184,34 @@ val finalProperties = properties.get(SupportsNamespaces.PROP_LOCATION)
properties +
(SupportsNamespaces.PROP_LOCATION -> makeQualifiedDBObjectPath(loc))
}
<<< #4133 lambda in parens rewritten to braces
maxColumn = 80
rewrite.rules = [RedundantBraces]
===
object Build {
lazy val scaladoc = project.in(file("scaladoc")).
settings(
generateScalaDocumentation := Def.inputTaskDyn {
val outputDirOverride = extraArgs.headOption.fold(identity[GenerationConfig](_))(newDir => {
config: GenerationConfig => config.add(OutputDir(newDir))
})
val justAPI = justAPIArg.fold(identity[GenerationConfig](_))(_ => {
config: GenerationConfig => config.remove[SiteRoot]
})
}.evaluated,
)
}
>>>
object Build {
lazy val scaladoc = project.in(file("scaladoc"))
.settings(generateScalaDocumentation := Def.inputTaskDyn {
val outputDirOverride = extraArgs.headOption
.fold(identity[GenerationConfig](_)) {
newDir => config: GenerationConfig => config.add(OutputDir(newDir))
}
val justAPI = justAPIArg
.fold(identity[GenerationConfig](_)) { _ => config: GenerationConfig =>
config.remove[SiteRoot]
}
}.evaluated)
}
32 changes: 32 additions & 0 deletions scalafmt-tests/shared/src/test/resources/newlines/source_keep.stat
Original file line number Diff line number Diff line change
Expand Up @@ -9585,3 +9585,35 @@ val finalProperties =
loc
))
}
<<< #4133 lambda in parens rewritten to braces
maxColumn = 80
rewrite.rules = [RedundantBraces]
===
object Build {
lazy val scaladoc = project.in(file("scaladoc")).
settings(
generateScalaDocumentation := Def.inputTaskDyn {
val outputDirOverride = extraArgs.headOption.fold(identity[GenerationConfig](_))(newDir => {
config: GenerationConfig => config.add(OutputDir(newDir))
})
val justAPI = justAPIArg.fold(identity[GenerationConfig](_))(_ => {
config: GenerationConfig => config.remove[SiteRoot]
})
}.evaluated,
)
}
>>>
object Build {
lazy val scaladoc = project.in(file("scaladoc")).
settings(
generateScalaDocumentation := Def.inputTaskDyn {
val outputDirOverride =
extraArgs.headOption.fold(identity[GenerationConfig](_)) {
newDir => config: GenerationConfig => config.add(OutputDir(newDir))
}
val justAPI = justAPIArg.fold(identity[GenerationConfig](_)) {
_ => config: GenerationConfig => config.remove[SiteRoot]
}
}.evaluated
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -9871,3 +9871,44 @@ val finalProperties = properties
properties +
(SupportsNamespaces.PROP_LOCATION -> makeQualifiedDBObjectPath(loc))
}
<<< #4133 lambda in parens rewritten to braces
maxColumn = 80
rewrite.rules = [RedundantBraces]
===
object Build {
lazy val scaladoc = project.in(file("scaladoc")).
settings(
generateScalaDocumentation := Def.inputTaskDyn {
val outputDirOverride = extraArgs.headOption.fold(identity[GenerationConfig](_))(newDir => {
config: GenerationConfig => config.add(OutputDir(newDir))
})
val justAPI = justAPIArg.fold(identity[GenerationConfig](_))(_ => {
config: GenerationConfig => config.remove[SiteRoot]
})
}.evaluated,
)
}
>>>
object Build {
lazy val scaladoc = project
.in(file("scaladoc"))
.settings(
generateScalaDocumentation :=
Def
.inputTaskDyn {
val outputDirOverride =
extraArgs
.headOption
.fold(identity[GenerationConfig](_)) {
newDir => config: GenerationConfig =>
config.add(OutputDir(newDir))
}
val justAPI =
justAPIArg.fold(identity[GenerationConfig](_)) {
_ => config: GenerationConfig =>
config.remove[SiteRoot]
}
}
.evaluated
)
}
Loading