Skip to content

Commit

Permalink
Router: zero cost for dot NL in braces-to-parens
Browse files Browse the repository at this point in the history
Instead of penalizing the split, set its cost to zero.
Also, do the same even for parens, to avoid non-idempotence.
  • Loading branch information
kitbellew committed Nov 9, 2024
1 parent eea37fb commit 04be0dd
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1961,22 +1961,20 @@ class Router(formatOps: FormatOps) {
.fold(expire)(x => getLastNonTrivialToken(x.qual))
val exclude = insideBracesBlock(ft, end, parensToo = true)
.excludeCloseDelim
val bracesToParensOwner =
if (!ftAfterRight.right.is[T.LeftBrace]) None
else (ftAfterRight.rightOwner match {
case t: Member.ArgClause if t.values.lengthCompare(1) == 0 => Some(t)
case t: Term.Block => t.parent.filter(_.is[Member.ArgClause])
case _ => None
}).filter { _ =>
val bracesToParens = ftAfterRight.right.is[T.OpenDelim] &&
(ftAfterRight.rightOwner match {
case t: Member.ArgClause => t.values.lengthCompare(1) == 0
case t: Term.Block => t.parent.is[Member.ArgClause]
case _ => false
}) && {
implicit val ft: FormatToken = next(ftAfterRight)
val rb = matching(ftAfterRight.right)
getBracesToParensMod(rb, Space, isWithinBraces = true)._1 ne
Space
}
val nlPenalty = bracesToParensOwner.fold(0)(nestedApplies(_) + 1)
val noSplit = Split(modSpace, 0)
.withSingleLine(end, exclude = exclude)
Seq(noSplit, nlSplitBase(1 + nlPenalty))
Seq(noSplit, nlSplitBase(if (bracesToParens) 0 else 1))
}
else {
val policy: Policy = forcedBreakOnNextDotPolicy
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
47 changes: 20 additions & 27 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 @@ -10186,9 +10185,8 @@ object a {
}
>>>
object a {
val defaultMethodNames = defaultGetterNames.map {
_.replace { case DefaultGetterName(methName, _) => methName }
}
val defaultMethodNames = defaultGetterNames
.map(_.replace { case DefaultGetterName(methName, _) => methName })
}
<<< #4133 braces to parens with two curried params
maxColumn = 74
Expand Down Expand Up @@ -10227,12 +10225,12 @@ val offsets = Utils.tryWithResource {
buffer.asLongBuffer
}
>>>
val offsets = Utils.tryWithResource {
new DataInputStream(Files.newInputStream(indexFile.toPath))
} { dis =>
dis.readFully(buffer.array)
buffer.asLongBuffer
}
val offsets = Utils
.tryWithResource(new DataInputStream(Files.newInputStream(indexFile.toPath))) {
dis =>
dis.readFully(buffer.array)
buffer.asLongBuffer
}
<<< #4133 braces to parens with two curried params, second func and comment
maxColumn = 76
newlines.avoidForSimpleOverflow = all
Expand Down Expand Up @@ -10304,9 +10302,9 @@ repeatTakeMapAndFold = fuse(
>>>
repeatTakeMapAndFold = fuse(
Source.repeat(new MutableElement(0)).take(ElementCount).map(addFunc)
.map(addFunc).fold(new MutableElement(0)) { (acc, x) =>
acc.value += x.value; acc
}.toMat(testSink)(Keep.right)
.map(addFunc)
.fold(new MutableElement(0)) { (acc, x) => acc.value += x.value; acc }
.toMat(testSink)(Keep.right)
)
<<< #4133 parens to braces, with function as argument
maxColumn = 72
Expand Down Expand Up @@ -10374,9 +10372,8 @@ val properties = propertyArgs.map {
}
}
>>>
val properties = propertyArgs.map {
_.drop(2).span(_ != '=') match { case (key, v) => key -> v.tail }
}
val properties = propertyArgs
.map(_.drop(2).span(_ != '=') match { case (key, v) => key -> v.tail })
<<< #4133 braces-to-parens with select, apply of nested for-yield, with overflow
maxColumn = 74
newlines.avoidForSimpleOverflow = all
Expand All @@ -10388,9 +10385,9 @@ runtime.unsafe.run {
} yield ()
}.getOrThrowFiberFailure()
>>>
runtime.unsafe.run {
for { _ <- ZIO.foreachDiscard(1.to(n))(_ => ZIO.yieldNow) } yield ()
}.getOrThrowFiberFailure()
runtime.unsafe
.run(for { _ <- ZIO.foreachDiscard(1.to(n))(_ => ZIO.yieldNow) } yield ())
.getOrThrowFiberFailure()
<<< #4133 first call in removed braces
maxColumn = 78
newlines.avoidForSimpleOverflow = all
Expand All @@ -10415,9 +10412,5 @@ def commaSeparated[T](part: () => T): List[T] =
commaSeparatedRest(part(), part)
}
>>>
Idempotency violated
=> Diff (- obtained, + expected)
-def commaSeparated[T](part: () => T): List[T] = in.currentRegion
- .withCommasExpected(commaSeparatedRest(part(), part))
+def commaSeparated[T](part: () => T): List[T] =
+ in.currentRegion.withCommasExpected(commaSeparatedRest(part(), part))
def commaSeparated[T](part: () => T): List[T] = in.currentRegion
.withCommasExpected(commaSeparatedRest(part(), part))
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 @@ -1959,9 +1959,9 @@ object a {
numBreaks: Int,
nest: Int,
): Option[NumBlanks] = topStatBlankLinesSorted.iterator
.takeWhile(_.minBreaks <= numBreaks).find { x =>
x.checkNest(nest) && x.pattern.forall(_.matcher(prefix).find())
}.flatMap(_.blanks)
.takeWhile(_.minBreaks <= numBreaks)
.find(x => x.checkNest(nest) && x.pattern.forall(_.matcher(prefix).find()))
.flatMap(_.blanks)
}
<<< fold ctrl body with redundant braces, !trailing commas
maxColumn = 80
Expand All @@ -1987,9 +1987,9 @@ object a {
numBreaks: Int,
nest: Int
): Option[NumBlanks] = topStatBlankLinesSorted.iterator
.takeWhile(_.minBreaks <= numBreaks).find { x =>
x.checkNest(nest) && x.pattern.forall(_.matcher(prefix).find())
}.flatMap(_.blanks)
.takeWhile(_.minBreaks <= numBreaks)
.find(x => x.checkNest(nest) && x.pattern.forall(_.matcher(prefix).find()))
.flatMap(_.blanks)
}
<<< #4108 try with redundant parens in block, scala212
runner.dialect = scala212
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, 1116220, "total explored")
assertEquals(explored, 1114174, "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 04be0dd

Please sign in to comment.