Skip to content

Commit

Permalink
Router: use SpaceOrNoSplit instead of custom rule
Browse files Browse the repository at this point in the history
This reverts a large part of #4153.
  • Loading branch information
kitbellew committed Oct 20, 2024
1 parent b625e06 commit 89da7a6
Show file tree
Hide file tree
Showing 11 changed files with 56 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import org.scalafmt.config.ScalafmtOptimizer
import org.scalafmt.config.TrailingCommas
import org.scalafmt.internal.Length.Num
import org.scalafmt.internal.Policy.NoPolicy
import org.scalafmt.rewrite.RedundantBraces
import org.scalafmt.util.InfixApp._
import org.scalafmt.util._

Expand Down Expand Up @@ -2958,6 +2959,22 @@ class FormatOps(
@inline
def indentedPackage(pkg: Pkg): Boolean = indentedPackage(pkg.body)

def getBracesToParensModAndPolicy(rb: FormatToken, mod: Modification)(implicit
style: ScalafmtConfig,
): (Modification, Policy) = {
if ((mod eq Space) && initStyle.rewrite.bracesToParensForOneLineApply)
RedundantBraces.noSplitForParensOnRightBrace(rb)
else None
}.fold((mod, Policy.noPolicy)) { rb =>
val beforeClose = prev(rb)
val end = Policy.End < rb.left
val policy = end ==> Policy.on(rb.left, "BracesToParens") {
case Decision(`beforeClose`, ss) => ss
.flatMap(s => if (s.isNL) None else Some(s.withMod(NoSplit)))
}
SpaceOrNoSplit(end) -> policy
}

}

object FormatOps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import org.scalafmt.internal.ExpiresOn.Before
import org.scalafmt.internal.Length.Num
import org.scalafmt.internal.Length.StateColumn
import org.scalafmt.internal.Policy.NoPolicy
import org.scalafmt.rewrite.RedundantBraces
import org.scalafmt.util._

import org.scalameta.FileLine
Expand Down Expand Up @@ -390,34 +389,21 @@ class Router(formatOps: FormatOps) {
}

val noSplitMod = xmlSpace(leftOwner)
val (slbMod, slbParensPolicy) =
if (singleLineDecisionOpt.isEmpty) (noSplitMod, NoPolicy)
else getBracesToParensModAndPolicy(closeFT, noSplitMod)
val singleLineSplitOpt = singleLineDecisionOpt.map { sld =>
val sldPolicy = getSingleLinePolicy
val expire = leftOwner.parent match {
case Some(p: Term.ForYield)
if !sld && sldPolicy.isEmpty && style.newlines.fold &&
leftOwner.is[Term.EnumeratorsBlock] => getLastToken(p)
case _ if style.newlines.isBeforeOpenParenCallSite => close
case _ => endOfSingleLineBlock(closeFT)
}
val toParens = initStyle.rewrite.bracesToParensForOneLineApply &&
RedundantBraces.noSplitForParensOnRightBrace(closeFT).isDefined
if (toParens) Right {
val noCloseSpacePolicy = Policy.End < close ==>
Policy.on(close, "PAREN1LAPPLY") {
case Decision(FormatToken(_, `close`, _), ss) => ss.map { s =>
if (s.isNL) s else s.withMod(NoSplit)
}
}
// copy logic from `( ...`, binpack=never, defining `slbSplit`
val slbEnd =
if (style.newlines.isBeforeOpenParenCallSite) close
else endOfSingleLineBlock(closeFT)
Split(NoSplit, 0).withSingleLine(slbEnd, noSyntaxNL = true)
.andPolicy(sldPolicy & noCloseSpacePolicy)
}
else Left {
Split(noSplitMod, 0).withSingleLine(expire, noSyntaxNL = true)
.andPolicy(sldPolicy)
}
// copy logic from `( ...`, binpack=never, defining `slbSplit`
Split(slbMod, 0).withSingleLine(expire, noSyntaxNL = true)
.andPolicy(sldPolicy & slbParensPolicy)
}

val nlSplit = Split(nl, if (nl.isNL) 1 else 0)
Expand All @@ -428,10 +414,11 @@ class Router(formatOps: FormatOps) {
val lambdaSplit =
if (noLambdaSplit) Split.ignored
else {
val nlPolicy = newlineBeforeClosingCurly
val policy = singleLineSplitOpt match {
case Some(Left(slbSplit)) => Policy.after(lambdaExpire, "FNARR") {
case Some(slbSplit) if slbParensPolicy.isEmpty =>
Policy.after(lambdaExpire, "FNARR") {
case Decision(FormatToken(`lambdaExpire`, _, _), ss) =>
val nlPolicy = newlineBeforeClosingCurly
var hadNoSplit = false
val nlSplits = ss.flatMap { s =>
if (s.isNL) Some(s.andPolicy(nlPolicy))
Expand All @@ -440,16 +427,15 @@ class Router(formatOps: FormatOps) {
if (hadNoSplit) slbSplit.withMod(Space) +: nlSplits
else nlSplits
}
case _ => newlineBeforeClosingCurly &
decideNewlinesOnlyAfterToken(lambdaExpire)
case _ => decideNewlinesOnlyAfterToken(lambdaExpire) ==> nlPolicy
}
Split(noSplitMod, 0).withSingleLine(lambdaExpire).andPolicy(policy)
.withIndent(lambdaIndent, close, Before)
}

val singleLineSplit = singleLineSplitOpt match {
case Some(Right(slbSplit)) => slbSplit
case Some(Left(slbSplit)) if lambdaSplit.isIgnored => slbSplit
case Some(slbSplit)
if slbParensPolicy.nonEmpty || lambdaSplit.isIgnored => slbSplit
case _ => Split.ignored
}
val splits = Seq(singleLineSplit, lambdaSplit, nlSplit)
Expand Down Expand Up @@ -1528,17 +1514,11 @@ class Router(formatOps: FormatOps) {
.exists(x => isTokenLastOrAfter(x.left, roPos))
}
} =>
val slbParensSplit =
if (!initStyle.rewrite.bracesToParensForOneLineApply) None
else RedundantBraces.noSplitForParensOnRightBrace(matching(lb))
.map { rbft =>
// copy logic from `( ...`, binpack=never, defining `slbSplit`
val isBeforeOpenParen = style.newlines.isBeforeOpenParenCallSite
val optimal: T =
if (isBeforeOpenParen) rbft.left else endOfSingleLineBlock(rbft)
Split(NoSplit, 0).withSingleLine(optimal)
}
Seq(slbParensSplit.getOrElse(Split.ignored), Split(Space, 0))
val rb = matching(lb)
val (mod, toParensPolicy) = getBracesToParensModAndPolicy(rb, Space)
val policy = Policy
.RelayOnSplit { case (s, _) => s.isNL }(toParensPolicy, NoPolicy)
Seq(Split(mod, 0, policy = policy))

// Delim
case FormatToken(left, _: T.Comma, _)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ abstract class CommunityIntellijScalaSuite(name: String)
class CommunityIntellijScala_2024_2_Suite
extends CommunityIntellijScalaSuite("intellij-scala-2024.2") {

override protected def totalStatesVisited: Option[Int] = Some(48193019)
override protected def totalStatesVisited: Option[Int] = Some(48163081)

override protected def builds = Seq(getBuild(
"2024.2.28",
Expand Down Expand Up @@ -51,7 +51,7 @@ class CommunityIntellijScala_2024_2_Suite
class CommunityIntellijScala_2024_3_Suite
extends CommunityIntellijScalaSuite("intellij-scala-2024.3") {

override protected def totalStatesVisited: Option[Int] = Some(48372795)
override protected def totalStatesVisited: Option[Int] = Some(48342732)

override protected def builds = Seq(getBuild(
"2024.3.4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ abstract class CommunityScala2Suite(name: String)

class CommunityScala2_12Suite extends CommunityScala2Suite("scala-2.12") {

override protected def totalStatesVisited: Option[Int] = Some(35856745)
override protected def totalStatesVisited: Option[Int] = Some(35837156)

override protected def builds =
Seq(getBuild("v2.12.20", dialects.Scala212, 1277))
Expand All @@ -18,7 +18,7 @@ class CommunityScala2_12Suite extends CommunityScala2Suite("scala-2.12") {

class CommunityScala2_13Suite extends CommunityScala2Suite("scala-2.13") {

override protected def totalStatesVisited: Option[Int] = Some(44618123)
override protected def totalStatesVisited: Option[Int] = Some(44596615)

override protected def builds =
Seq(getBuild("v2.13.14", dialects.Scala213, 1287))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ abstract class CommunityScala3Suite(name: String)

class CommunityScala3_2Suite extends CommunityScala3Suite("scala-3.2") {

override protected def totalStatesVisited: Option[Int] = Some(33385186)
override protected def totalStatesVisited: Option[Int] = Some(33365450)

override protected def builds = Seq(getBuild("3.2.2", dialects.Scala32, 791))

}

class CommunityScala3_3Suite extends CommunityScala3Suite("scala-3.3") {

override protected def totalStatesVisited: Option[Int] = Some(36033019)
override protected def totalStatesVisited: Option[Int] = Some(36011287)

override protected def builds = Seq(getBuild("3.3.3", dialects.Scala33, 861))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ abstract class CommunitySparkSuite(name: String)

class CommunitySpark3_4Suite extends CommunitySparkSuite("spark-3.4") {

override protected def totalStatesVisited: Option[Int] = Some(72481328)
override protected def totalStatesVisited: Option[Int] = Some(72398899)

override protected def builds =
Seq(getBuild("v3.4.1", dialects.Scala213, 2585))
Expand All @@ -18,7 +18,7 @@ class CommunitySpark3_4Suite extends CommunitySparkSuite("spark-3.4") {

class CommunitySpark3_5Suite extends CommunitySparkSuite("spark-3.5") {

override protected def totalStatesVisited: Option[Int] = Some(76650858)
override protected def totalStatesVisited: Option[Int] = Some(76561809)

override protected def builds =
Seq(getBuild("v3.5.3", dialects.Scala213, 2756))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10629,14 +10629,6 @@ convertStream(inputStream, tokenizer, handleHeader, parser.options.charset) { to
safeParser.parse(tokens)
}.flatten
>>>
Idempotency violated
=> Diff (- obtained, + expected)
-convertStream(inputStream, tokenizer, handleHeader, parser.options.charset)(
- tokens => safeParser.parse(tokens)
-).flatten
+convertStream(
+ inputStream,
+ tokenizer,
+ handleHeader,
+ parser.options.charset
+)(tokens => safeParser.parse(tokens)).flatten
convertStream(inputStream, tokenizer, handleHeader, parser.options.charset) {
tokens => safeParser.parse(tokens)
}.flatten
14 changes: 3 additions & 11 deletions scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat
Original file line number Diff line number Diff line change
Expand Up @@ -9931,14 +9931,6 @@ convertStream(inputStream, tokenizer, handleHeader, parser.options.charset) { to
safeParser.parse(tokens)
}.flatten
>>>
Idempotency violated
=> Diff (- obtained, + expected)
-convertStream(inputStream, tokenizer, handleHeader, parser.options.charset)(
- tokens => safeParser.parse(tokens)
-).flatten
+convertStream(
+ inputStream,
+ tokenizer,
+ handleHeader,
+ parser.options.charset
+)(tokens => safeParser.parse(tokens)).flatten
convertStream(inputStream, tokenizer, handleHeader, parser.options.charset) {
tokens => safeParser.parse(tokens)
}.flatten
14 changes: 3 additions & 11 deletions scalafmt-tests/shared/src/test/resources/newlines/source_keep.stat
Original file line number Diff line number Diff line change
Expand Up @@ -10378,14 +10378,6 @@ convertStream(inputStream, tokenizer, handleHeader, parser.options.charset) { to
safeParser.parse(tokens)
}.flatten
>>>
Idempotency violated
=> Diff (- obtained, + expected)
-convertStream(inputStream, tokenizer, handleHeader, parser.options.charset)(
- tokens => safeParser.parse(tokens)
-).flatten
+convertStream(
+ inputStream,
+ tokenizer,
+ handleHeader,
+ parser.options.charset
+)(tokens => safeParser.parse(tokens)).flatten
convertStream(inputStream, tokenizer, handleHeader, parser.options.charset) {
tokens => safeParser.parse(tokens)
}.flatten
Original file line number Diff line number Diff line change
Expand Up @@ -10759,14 +10759,6 @@ convertStream(inputStream, tokenizer, handleHeader, parser.options.charset) { to
safeParser.parse(tokens)
}.flatten
>>>
Idempotency violated
=> Diff (- obtained, + expected)
-convertStream(inputStream, tokenizer, handleHeader, parser.options.charset)(
- tokens => safeParser.parse(tokens)
-).flatten
+convertStream(
+ inputStream,
+ tokenizer,
+ handleHeader,
+ parser.options.charset
+)(tokens => safeParser.parse(tokens)).flatten
convertStream(inputStream, tokenizer, handleHeader, parser.options.charset) {
tokens => safeParser.parse(tokens)
}.flatten
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ class FormatTests extends FunSuite with CanRunTests with FormatAssertions {
val explored = Debug.explored.get()
logger.debug(s"Total explored: $explored")
if (!onlyUnit && !onlyManual)
assertEquals(explored, 1499684, "total explored")
assertEquals(explored, 1498911, "total explored")
val results = debugResults.result()
// TODO(olafur) don't block printing out test results.
// I don't want to deal with scalaz's Tasks :'(
Expand Down

0 comments on commit 89da7a6

Please sign in to comment.