Skip to content

Commit 885eb5d

Browse files
kitbellewkitbellew
committed
Router: fix handling of comment-space-dot chain
1 parent 076b2cf commit 885eb5d

File tree

8 files changed

+63
-64
lines changed

8 files changed

+63
-64
lines changed

‎scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Modification.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,10 @@ case class NewlineT(
4949
override val length: Int = 0
5050
}
5151

52-
object Newline extends NewlineT
52+
object Newline extends NewlineT {
53+
def orMod(flag: Boolean, mod: => Modification): Modification =
54+
if (flag) this else mod
55+
}
5356

5457
object Newline2x extends NewlineT(isDouble = true) {
5558
def apply(isDouble: Boolean): NewlineT = if (isDouble) this else Newline

‎scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1706,7 +1706,10 @@ class Router(formatOps: FormatOps) {
17061706
findLastApplyAndNextSelect(rightOwner, enclosed)
17071707
val (prevSelect, prevApply) =
17081708
findPrevSelectAndApply(thisSelect.qual, enclosed)
1709-
val nlOnly = prevApply.exists(x => getHeadToken(x.argClause).is[T.Colon])
1709+
val afterComment = left.is[T.Comment]
1710+
val nlOnly =
1711+
style.newlines.sourceIgnored && afterComment && hasBreak() ||
1712+
prevApply.exists(x => getHeadToken(x.argClause).is[T.Colon])
17101713
val expire = getLastEnclosedToken(expireTree)
17111714
val indentLen = style.indent.main
17121715

@@ -1758,6 +1761,7 @@ class Router(formatOps: FormatOps) {
17581761
}
17591762

17601763
val ftAfterRight = tokens(ft, 2)
1764+
def modSpace = Space(afterComment)
17611765
val baseSplits = style.newlines.getSelectChains match {
17621766
case Newlines.classic =>
17631767
def getNlMod = {
@@ -1772,7 +1776,7 @@ class Router(formatOps: FormatOps) {
17721776
if (ok) Some(nd.left) else None
17731777
}
17741778
val altIndent = endSelect.map(Indent(-indentLen, _, After))
1775-
NewlineT(alt = Some(ModExt(NoSplit).withIndentOpt(altIndent)))
1779+
NewlineT(alt = Some(ModExt(modSpace).withIndentOpt(altIndent)))
17761780
}
17771781

17781782
val prevChain = inSelectChain(prevSelect, thisSelect, expireTree)
@@ -1787,7 +1791,7 @@ class Router(formatOps: FormatOps) {
17871791
val newlinePolicy = breakOnNextDot & penalizeBreaks
17881792
val ignoreNoSplit = nlOnly ||
17891793
hasBreak &&
1790-
(left.is[T.Comment] || style.optIn.breakChainOnFirstMethodDot)
1794+
(afterComment || style.optIn.breakChainOnFirstMethodDot)
17911795
val chainLengthPenalty =
17921796
if (
17931797
style.newlines.penalizeSingleSelectMultiArgList &&
@@ -1811,13 +1815,14 @@ class Router(formatOps: FormatOps) {
18111815
val nlCost = nlBaseCost + nestedPenalty + chainLengthPenalty
18121816
val nlMod = getNlMod
18131817
val legacySplit = Split(!prevChain, 1) { // must come first, for backwards compat
1814-
if (style.optIn.breaksInsideChains) NoSplit.orNL(noBreak)
1818+
if (style.optIn.breaksInsideChains) Newline
1819+
.orMod(hasBreak(), modSpace)
18151820
else nlMod
18161821
}.withPolicy(newlinePolicy).onlyFor(SplitTag.SelectChainSecondNL)
18171822
val slbSplit =
18181823
if (ignoreNoSplit) Split.ignored
18191824
else {
1820-
val noSplit = Split(NoSplit, 0)
1825+
val noSplit = Split(modSpace, 0)
18211826
if (prevChain) noSplit
18221827
else chainExpire match { // allow newlines in final {} block
18231828
case x: T.RightBrace => noSplit
@@ -1830,63 +1835,60 @@ class Router(formatOps: FormatOps) {
18301835
.withPolicy(newlinePolicy)
18311836
Seq(legacySplit, slbSplit, nlSplit)
18321837
} else {
1833-
val isComment = left.is[T.Comment]
1834-
val doBreak = nlOnly || isComment && hasBreak
1838+
val doBreak = nlOnly || afterComment && hasBreak
18351839
Seq(
18361840
Split(!prevChain, 1) {
1837-
if (style.optIn.breaksInsideChains) NoSplit.orNL(noBreak)
1838-
else if (doBreak) Newline
1839-
else getNlMod
1841+
if (style.optIn.breaksInsideChains) Newline
1842+
.orMod(hasBreak(), modSpace)
1843+
else Newline.orMod(doBreak, getNlMod)
18401844
}.withPolicy(breakOnNextDot)
18411845
.onlyFor(SplitTag.SelectChainSecondNL),
1842-
Split(if (doBreak) Newline else Space(isComment), 0),
1846+
Split(Newline.orMod(doBreak, modSpace), 0),
18431847
)
18441848
}
18451849

1846-
case _ if left.is[T.Comment] => Seq(Split(Space.orNL(noBreak), 0))
1847-
18481850
case Newlines.keep =>
18491851
if (hasBreak()) Seq(Split(Newline, 0))
18501852
else if (hasBreakAfterRightBeforeNonComment(ft))
1851-
Seq(Split(NoSplit, 0).withPolicy(
1853+
Seq(Split(modSpace, 0).withPolicy(
18521854
decideNewlinesOnlyAfterClose(Split(Newline, 0))(r),
18531855
ignore = next(ft).right.is[T.Comment],
18541856
))
18551857
else Seq(
1856-
Split(NoSplit, 0),
1858+
Split(modSpace, 0),
18571859
Split(Newline, 1).onlyFor(SplitTag.SelectChainBinPackNL),
18581860
)
18591861

18601862
case Newlines.unfold =>
18611863
val nlCost = if (nlOnly) 0 else 1
18621864
if (prevSelect.isEmpty && nextSelect.isEmpty) Seq(
1863-
Split(nlOnly, 0)(NoSplit).withSingleLine(getSlbEnd()),
1865+
Split(nlOnly, 0)(modSpace).withSingleLine(getSlbEnd()),
18641866
Split(Newline, nlCost),
18651867
)
18661868
else Seq(
1867-
Split(nlOnly, 0)(NoSplit)
1869+
Split(nlOnly, 0)(modSpace)
18681870
.withSingleLine(expire, noSyntaxNL = true),
1869-
Split(NewlineT(alt = Some(NoSplit)), nlCost)
1871+
Split(NewlineT(alt = Some(modSpace)), nlCost)
18701872
.withPolicy(forcedBreakOnNextDotPolicy),
18711873
)
18721874

18731875
case Newlines.fold =>
18741876
val nlCost = if (nlOnly) 0 else 1
18751877
def nlSplitBase(implicit fileLine: FileLine) =
1876-
Split(NewlineT(alt = Some(NoSplit)), nlCost)
1878+
Split(NewlineT(alt = Some(modSpace)), nlCost)
18771879
if (nextDotIfSig.isEmpty) {
18781880
val noSplit =
18791881
if (nlOnly) Split.ignored
18801882
else {
18811883
val end = nextSelect
18821884
.fold(expire)(x => getLastNonTrivialToken(x.qual))
18831885
val exclude = insideBracesBlock(ft, end, parensToo = true)
1884-
Split(NoSplit, 0).withSingleLine(end, exclude = exclude)
1886+
Split(modSpace, 0).withSingleLine(end, exclude = exclude)
18851887
}
18861888
Seq(noSplit, nlSplitBase)
18871889
} else {
18881890
val policy: Policy = forcedBreakOnNextDotPolicy
1889-
val noSplit = Split(nlOnly, 0)(NoSplit).withPolicy(policy)
1891+
val noSplit = Split(nlOnly, 0)(modSpace).withPolicy(policy)
18901892
Seq(noSplit, nlSplitBase.withPolicy(policy))
18911893
}
18921894
}

‎scalafmt-tests-community/scala2/src/test/scala/org/scalafmt/community/scala2/CommunityScala2Suite.scala

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ class CommunityScala2_12Suite extends CommunityScala2Suite("scala-2.12") {
1818
"classic" -> TestStats.Style(expectedStatesVisited = 3742070),
1919
"classicWithAlign" -> TestStats.Style(expectedStatesVisited = 3743075),
2020
"classicWithRewrites" -> TestStats.Style(expectedStatesVisited = 3782691),
21-
"fold" -> TestStats.Style(expectedStatesVisited = 5833337),
21+
"fold" -> TestStats.Style(expectedStatesVisited = 5833324),
2222
"keep" -> TestStats.Style(expectedStatesVisited = 3511074),
2323
"keepWithAlign" -> TestStats.Style(expectedStatesVisited = 3511201),
2424
"keepWithRewrites" -> TestStats.Style(expectedStatesVisited = 3557273),
2525
"keepWithScalaJS" -> TestStats.Style(expectedStatesVisited = 3976667),
26-
"unfold" -> TestStats.Style(expectedStatesVisited = 4192416),
26+
"unfold" -> TestStats.Style(expectedStatesVisited = 4192369),
2727
),
2828
))
2929

@@ -36,15 +36,15 @@ class CommunityScala2_13Suite extends CommunityScala2Suite("scala-2.13") {
3636
dialects.Scala213,
3737
1287,
3838
statsPerStyle = Map(
39-
"classic" -> TestStats.Style(expectedStatesVisited = 4633745),
40-
"classicWithAlign" -> TestStats.Style(expectedStatesVisited = 4636185),
41-
"classicWithRewrites" -> TestStats.Style(expectedStatesVisited = 4666467),
42-
"fold" -> TestStats.Style(expectedStatesVisited = 7586090),
39+
"classic" -> TestStats.Style(expectedStatesVisited = 4633753),
40+
"classicWithAlign" -> TestStats.Style(expectedStatesVisited = 4636193),
41+
"classicWithRewrites" -> TestStats.Style(expectedStatesVisited = 4666475),
42+
"fold" -> TestStats.Style(expectedStatesVisited = 7586002),
4343
"keep" -> TestStats.Style(expectedStatesVisited = 4329252),
4444
"keepWithAlign" -> TestStats.Style(expectedStatesVisited = 4329386),
4545
"keepWithRewrites" -> TestStats.Style(expectedStatesVisited = 4373731),
4646
"keepWithScalaJS" -> TestStats.Style(expectedStatesVisited = 4928298),
47-
"unfold" -> TestStats.Style(expectedStatesVisited = 5125436),
47+
"unfold" -> TestStats.Style(expectedStatesVisited = 5125363),
4848
),
4949
))
5050

‎scalafmt-tests-community/scala3/src/test/scala/org/scalafmt/community/scala3/CommunityScala3Suite.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class CommunityScala3_2Suite extends CommunityScala3Suite("scala-3.2") {
2323
"keepWithAlign" -> TestStats.Style(expectedStatesVisited = 3148747),
2424
"keepWithRewrites" -> TestStats.Style(expectedStatesVisited = 3079850),
2525
"keepWithScalaJS" -> TestStats.Style(expectedStatesVisited = 3631800),
26-
"unfold" -> TestStats.Style(expectedStatesVisited = 3882915),
26+
"unfold" -> TestStats.Style(expectedStatesVisited = 3882869),
2727
),
2828
))
2929

@@ -44,7 +44,7 @@ class CommunityScala3_3Suite extends CommunityScala3Suite("scala-3.3") {
4444
"keepWithAlign" -> TestStats.Style(expectedStatesVisited = 3388188),
4545
"keepWithRewrites" -> TestStats.Style(expectedStatesVisited = 3321872),
4646
"keepWithScalaJS" -> TestStats.Style(expectedStatesVisited = 3907628),
47-
"unfold" -> TestStats.Style(expectedStatesVisited = 4201569),
47+
"unfold" -> TestStats.Style(expectedStatesVisited = 4201515),
4848
),
4949
))
5050

‎scalafmt-tests-community/spark/src/test/scala/org/scalafmt/community/spark/CommunitySparkSuite.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class CommunitySpark3_4Suite extends CommunitySparkSuite("spark-3.4") {
2323
"keepWithAlign" -> TestStats.Style(expectedStatesVisited = 6827259),
2424
"keepWithRewrites" -> TestStats.Style(expectedStatesVisited = 6841859),
2525
"keepWithScalaJS" -> TestStats.Style(expectedStatesVisited = 7585082),
26-
"unfold" -> TestStats.Style(expectedStatesVisited = 8655221),
26+
"unfold" -> TestStats.Style(expectedStatesVisited = 8654777),
2727
),
2828
))
2929

@@ -44,7 +44,7 @@ class CommunitySpark3_5Suite extends CommunitySparkSuite("spark-3.5") {
4444
"keepWithAlign" -> TestStats.Style(expectedStatesVisited = 7218811),
4545
"keepWithRewrites" -> TestStats.Style(expectedStatesVisited = 7237620),
4646
"keepWithScalaJS" -> TestStats.Style(expectedStatesVisited = 8057079),
47-
"unfold" -> TestStats.Style(expectedStatesVisited = 9159697),
47+
"unfold" -> TestStats.Style(expectedStatesVisited = 9159243),
4848
),
4949
))
5050

‎scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9755,8 +9755,8 @@ maxColumn = 37
97559755
===
97569756
sel.getContext /*ScImportSelectors*/ .getContext.asInstanceOf[ScImportExpr].reference
97579757
>>>
9758-
sel
9759-
.getContext /*ScImportSelectors*/ .getContext
9758+
sel.getContext /*ScImportSelectors*/
9759+
.getContext
97609760
.asInstanceOf[ScImportExpr]
97619761
.reference
97629762
<<< #4133 select chain with no-break comment before dot, long

‎scalafmt-tests/shared/src/test/resources/newlines/source_unfold.stat

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2807,7 +2807,8 @@ class Foo {
28072807
val vv = v
28082808
.aaa //
28092809
//
2810-
.bbb.ccc()
2810+
.bbb
2811+
.ccc()
28112812
val vv = v.aaa //
28122813
val vv = v.aaa
28132814
}
@@ -2827,7 +2828,8 @@ class Foo {
28272828
.bbb //
28282829
//
28292830
.ccc //
2830-
.ddd.eee()
2831+
.ddd
2832+
.eee()
28312833
}
28322834
<<< #1334 3: continue chain indent after a comment with apply
28332835
class Foo {
@@ -2885,7 +2887,8 @@ val a: Vector[Array[Double]] =
28852887
// another comment
28862888
.map { foo =>
28872889
bar
2888-
}.tail
2890+
}
2891+
.tail
28892892
<<< #1334 6: multiple select and a match after a comment
28902893
val a: Vector[Array[Double]] = b.c
28912894
// Only handle first case, others will be fixed on the next pass.
@@ -2898,7 +2901,8 @@ val a: Vector[Array[Double]] = b.c
28982901
val a: Vector[Array[Double]] =
28992902
b.c
29002903
// Only handle first case, others will be fixed on the next pass.
2901-
.headOption.a match {
2904+
.headOption
2905+
.a match {
29022906
case None =>
29032907
case _ =>
29042908
}
@@ -6291,10 +6295,12 @@ object a {
62916295
object a {
62926296
foo
62936297
// c2
6294-
.bar(baz).foo(bar, baz)
6298+
.bar(baz)
6299+
.foo(bar, baz)
62956300
foo // c1
62966301
// c2
6297-
.bar(baz).foo(bar, baz)
6302+
.bar(baz)
6303+
.foo(bar, baz)
62986304
foo
62996305
.bar(baz) // c1
63006306
// c2
@@ -10559,30 +10565,18 @@ maxColumn = 37
1055910565
===
1056010566
sel.getContext /*ScImportSelectors*/ .getContext.asInstanceOf[ScImportExpr].reference
1056110567
>>>
10562-
BestFirstSearch:278 Failed to format
10563-
UNABLE TO FORMAT,
10564-
tok #4/14: [4] /*ScImportSelectors*/∙.: Comment(ScImportSelectors) [15..36) [ ] Dot [37..38)
10565-
policies:
10566-
(NEXTSEL1NL:[Router:1896]<48d & SLB:[Router:1868]@85!d)
10567-
splits (before policy):
10568-
c=0[0] SP:[Router:1846->Router:1846](indents=[], NoPolicy)
10569-
[SelectChainFirstNL]c=0[0] SP:[Router:1846->Router:1846](indents=[], NEXTSEL1NL:[Router:1896]<61d)
10570-
splits (after policy):
10571-
[!SelectChainFirstNL]c=0[0] SP:[Router:1846->Router:1846](indents=[], NoPolicy)
10572-
c=0[0] SP:[Router:1846->Router:1846](indents=[], NEXTSEL1NL:[Router:1896]<61d)
10568+
sel
10569+
.getContext /*ScImportSelectors*/
10570+
.getContext
10571+
.asInstanceOf[ScImportExpr]
10572+
.reference
1057310573
<<< #4133 select chain with no-break comment before dot, long
1057410574
maxColumn = 80
1057510575
===
1057610576
sel.getContext /*ScImportSelectors*/ .getContext.asInstanceOf[ScImportExpr].reference
1057710577
>>>
10578-
BestFirstSearch:278 Failed to format
10579-
UNABLE TO FORMAT,
10580-
tok #12/14: [12] .∙reference: Dot [75..76) [] Ident(reference) [76..85)
10581-
policies:
10582-
SLB:[Router:1868]@85!d
10583-
SLB:[Router:1868]@85!d
10584-
SLB:[Router:1868]@85!d
10585-
splits (before policy):
10586-
c=0[0] nS:[Router:2551->Router:2551](indents=[], NoPolicy)
10587-
splits (after policy):
10588-
c=0[0] nS:[Router:2551->Router:2551](indents=[], NoPolicy)
10578+
sel
10579+
.getContext /*ScImportSelectors*/
10580+
.getContext
10581+
.asInstanceOf[ScImportExpr]
10582+
.reference

‎scalafmt-tests/shared/src/test/scala/org/scalafmt/FormatTests.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ class FormatTests extends FunSuite with CanRunTests with FormatAssertions {
138138
val explored = Debug.explored.get()
139139
logger.debug(s"Total explored: $explored")
140140
if (!onlyUnit && !onlyManual)
141-
assertEquals(explored, 1493404, "total explored")
141+
assertEquals(explored, 1493674, "total explored")
142142
val results = debugResults.result()
143143
// TODO(olafur) don't block printing out test results.
144144
// I don't want to deal with scalaz's Tasks :'(

0 commit comments

Comments
 (0)