Skip to content

Commit

Permalink
Router: check exclude blocks if braces to parens
Browse files Browse the repository at this point in the history
  • Loading branch information
kitbellew committed Nov 4, 2024
1 parent 272bbe1 commit 551a129
Show file tree
Hide file tree
Showing 12 changed files with 88 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2977,20 +2977,45 @@ class FormatOps(
@inline
def indentedPackage(pkg: Pkg): Boolean = indentedPackage(pkg.body)

def insideBracesBlockIfBracesToParens(
rb: FormatToken,
mod: Modification,
)(implicit style: ScalafmtConfig, ft: FormatToken): Option[TokenRanges] =
if (
initStyle.rewrite.bracesToParensForOneLineApply &&
style.rewrite.trailingCommas.isOptional && (mod eq Space)
) {
val isWithinBraces = ft.left.is[T.LeftBrace]
if (
isWithinBraces && style.spaces.inParentheses ||
!RedundantBraces.canRewriteWithParensOnRightBrace(rb)
) None
else {
// the rule for `(...` excludes brace blocks
// check if all of them can be converted to parens
val nft = if (isWithinBraces) ft else next(ft)
def getTokenRanges = {
val tr = insideBracesBlock(nft, rb.left)
.filter(x => !couldHaveBracesConvertedToParens(x.lt.leftOwner))
if (tr.ranges.exists(!_.lt.left.is[T.LeftBrace])) null else Some(tr)
}
nft.leftOwner match {
case Term.Block(arg :: Nil) if style.newlines.fold =>
if (arg.is[Term.FunctionTerm]) Some(TokenRanges.empty)
else if (isTreeEndingInArgumentClause(arg)) Some(parensTuple(arg))
else getTokenRanges
case _ => getTokenRanges
}
}
} else None

def getBracesToParensMod(rb: FormatToken, mod: Modification)(implicit
style: ScalafmtConfig,
ft: FormatToken,
): Modification = {
val isWithinBraces = ft.left.is[T.LeftBrace]
val ok = initStyle.rewrite.bracesToParensForOneLineApply &&
style.rewrite.trailingCommas.isOptional && (mod eq Space) &&
(!isWithinBraces || !style.spaces.inParentheses) &&
RedundantBraces.canRewriteWithParensOnRightBrace(rb) &&
// the rule for `(...` excludes brace blocks
// check if all of them can be converted to parens
insideBracesBlock(if (isWithinBraces) ft else next(ft), rb.left).ranges
.forall(x => couldHaveBracesConvertedToParens(x.lt.leftOwner))
if (ok) SpaceOrNoSplit(Policy.End < rb.left) else mod
): (Modification, Option[TokenRanges]) = {
val tr = insideBracesBlockIfBracesToParens(rb, mod)
if ((tr eq null) || tr.isEmpty) (mod, tr)
else (SpaceOrNoSplit(Policy.End < rb.left), tr)
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,10 +389,12 @@ class Router(formatOps: FormatOps) {
}

val noSplitMod = xmlSpace(leftOwner)
val slbMod =
if (singleLineDecisionOpt.isEmpty) noSplitMod
val (slbMod, slbParensExclude) =
if (singleLineDecisionOpt.isEmpty) (noSplitMod, None)
else getBracesToParensMod(closeFT, noSplitMod)
val singleLineSplitOpt = singleLineDecisionOpt.map { sld =>
val singleLineSplitOpt = {
if (slbParensExclude eq null) None else singleLineDecisionOpt
}.map { sld =>
val sldPolicy = getSingleLinePolicy
val expire = leftOwner.parent match {
case Some(p: Term.ForYield)
Expand All @@ -409,8 +411,20 @@ class Router(formatOps: FormatOps) {
.flatMap(s => if (s.isNL) None else Some(s.withMod(NoSplit)))
}
}
Split(slbMod, 0).withSingleLine(expire, noSyntaxNL = true)
.andPolicy(sldPolicy & slbParensPolicy)
val exclude = slbParensExclude.getOrElse(TokenRanges.empty)
val slbPolicy =
if (exclude.isEmpty) slbParensPolicy
else Policy.RelayOnSplit { case (s, _) => s.isNL }(
slbParensPolicy,
Policy.on(close, "BracesToParensFailed") { case _ => Nil },
)
Split(slbMod, 0).withSingleLine(
expire,
exclude = exclude,
noOptimal = style.newlines.fold && !exclude.isEmpty &&
exclude.ranges.forall(_.lt.left.is[T.LeftParen]),
noSyntaxNL = true,
).andPolicy(sldPolicy & slbPolicy)
}

val lambdaNLPolicy = leftOwner match {
Expand Down Expand Up @@ -1532,7 +1546,7 @@ class Router(formatOps: FormatOps) {
.exists(x => isTokenLastOrAfter(x.left, roPos))
}
} =>
val mod = getBracesToParensMod(matching(lb), Space)
val mod = getBracesToParensMod(matching(lb), Space)._1
Seq(Split(mod, 0))

// Delim
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ class TokenRanges private (val ranges: Seq[TokenRange]) extends AnyVal {
new TokenRanges(range +: ranges)
}

def filter(f: TokenRange => Boolean): TokenRanges =
new TokenRanges(ranges.filter(f))

def startOfFirstRange(): Option[Token] = ranges.lastOption.map(_.lt.left)
}

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(47236348)
override protected def totalStatesVisited: Option[Int] = Some(47238506)

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(47413622)
override protected def totalStatesVisited: Option[Int] = Some(47415656)

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(34646234)
override protected def totalStatesVisited: Option[Int] = Some(34647934)

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(43165033)
override protected def totalStatesVisited: Option[Int] = Some(43166732)

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(32264582)
override protected def totalStatesVisited: Option[Int] = Some(32265009)

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(34834391)
override protected def totalStatesVisited: Option[Int] = Some(34834721)

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(70667003)
override protected def totalStatesVisited: Option[Int] = Some(70673804)

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(74759780)
override protected def totalStatesVisited: Option[Int] = Some(74766817)

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 @@ -10645,9 +10645,8 @@ val newExprs = exprs.map { _.transform {
case a: AttributeReference => attrMap.getOrElse(a, a)
}}
>>>
val newExprs = exprs.map {
_.transform { case a: AttributeReference => attrMap.getOrElse(a, a) }
}
val newExprs = exprs
.map(_.transform { case a: AttributeReference => attrMap.getOrElse(a, a) })
<<< #4133 overflow comment with a forced break 1
maxColumn = 76
newlines.avoidForSimpleOverflow = [tooLong, punct, slc]
Expand Down
29 changes: 14 additions & 15 deletions scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat
Original file line number Diff line number Diff line change
Expand Up @@ -9950,9 +9950,8 @@ val newExprs = exprs.map { _.transform {
case a: AttributeReference => attrMap.getOrElse(a, a)
}}
>>>
val newExprs = exprs.map {
_.transform { case a: AttributeReference => attrMap.getOrElse(a, a) }
}
val newExprs = exprs
.map(_.transform { case a: AttributeReference => attrMap.getOrElse(a, a) })
<<< #4133 overflow comment with a forced break 1
maxColumn = 76
newlines.avoidForSimpleOverflow = [tooLong, punct, slc]
Expand Down Expand Up @@ -10136,18 +10135,18 @@ object a {
}
}
>>>
Idempotency violated
=> Diff (- obtained, + expected)
val metadatas = snapshotMetadatas(persistenceId, criteria)
- Future.sequence(
- metadatas.map(deleteAsync)
- )(scala.collection.immutable.IndexedSeq, streamDispatcher)
- .map(_ => ())(streamDispatcher)
+ Future.sequence(metadatas.map(deleteAsync))(
+ scala.collection.immutable.IndexedSeq,
+ streamDispatcher
+ ).map(_ => ())(streamDispatcher)
}
object a {
override def deleteAsync(
persistenceId: String,
criteria: SnapshotSelectionCriteria
): Future[Unit] = {
val metadatas = snapshotMetadatas(persistenceId, criteria)
Future.sequence {
metadatas.map(deleteAsync)
}(scala.collection.immutable.IndexedSeq, streamDispatcher)
.map(_ => ())(streamDispatcher)
}
}
<<< #4133 redundant braces with nested partial function
maxColumn = 76
rewrite.rules = [RedundantBraces]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10394,9 +10394,8 @@ val newExprs = exprs.map { _.transform {
case a: AttributeReference => attrMap.getOrElse(a, a)
}}
>>>
val newExprs = exprs.map {
_.transform { case a: AttributeReference => attrMap.getOrElse(a, a) }
}
val newExprs = exprs
.map(_.transform { case a: AttributeReference => attrMap.getOrElse(a, a) })
<<< #4133 overflow comment with a forced break 1
maxColumn = 76
newlines.avoidForSimpleOverflow = [tooLong, punct, slc]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10778,9 +10778,8 @@ val newExprs = exprs.map { _.transform {
case a: AttributeReference => attrMap.getOrElse(a, a)
}}
>>>
val newExprs = exprs.map {
_.transform { case a: AttributeReference => attrMap.getOrElse(a, a) }
}
val newExprs = exprs
.map(_.transform { case a: AttributeReference => attrMap.getOrElse(a, a) })
<<< #4133 overflow comment with a forced break 1
maxColumn = 76
newlines.avoidForSimpleOverflow = [tooLong, punct, slc]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,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, 1489102, "total explored")
assertEquals(explored, 1489127, "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 551a129

Please sign in to comment.