Router: fix handling of comment-space-dot chain

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

@@ -49,7 +49,10 @@ case class NewlineT(
   override val length: Int = 0
 }
 
-object Newline extends NewlineT
+object Newline extends NewlineT {
+  def orMod(flag: Boolean, mod: => Modification): Modification =
+    if (flag) this else mod
+}
 
 object Newline2x extends NewlineT(isDouble = true) {
   def apply(isDouble: Boolean): NewlineT = if (isDouble) this else Newline

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

@@ -1706,7 +1706,10 @@ class Router(formatOps: FormatOps) {
           findLastApplyAndNextSelect(rightOwner, enclosed)
         val (prevSelect, prevApply) =
           findPrevSelectAndApply(thisSelect.qual, enclosed)
-        val nlOnly = prevApply.exists(x => getHeadToken(x.argClause).is[T.Colon])
+        val afterComment = left.is[T.Comment]
+        val nlOnly =
+          style.newlines.sourceIgnored && afterComment && hasBreak() ||
+            prevApply.exists(x => getHeadToken(x.argClause).is[T.Colon])
         val expire = getLastEnclosedToken(expireTree)
         val indentLen = style.indent.main
 
@@ -1758,6 +1761,7 @@ class Router(formatOps: FormatOps) {
         }
 
         val ftAfterRight = tokens(ft, 2)
+        def modSpace = Space(afterComment)
         val baseSplits = style.newlines.getSelectChains match {
           case Newlines.classic =>
             def getNlMod = {
@@ -1772,7 +1776,7 @@ class Router(formatOps: FormatOps) {
                 if (ok) Some(nd.left) else None
               }
               val altIndent = endSelect.map(Indent(-indentLen, _, After))
-              NewlineT(alt = Some(ModExt(NoSplit).withIndentOpt(altIndent)))
+              NewlineT(alt = Some(ModExt(modSpace).withIndentOpt(altIndent)))
             }
 
             val prevChain = inSelectChain(prevSelect, thisSelect, expireTree)
@@ -1787,7 +1791,7 @@ class Router(formatOps: FormatOps) {
               val newlinePolicy = breakOnNextDot & penalizeBreaks
               val ignoreNoSplit = nlOnly ||
                 hasBreak &&
-                (left.is[T.Comment] || style.optIn.breakChainOnFirstMethodDot)
+                (afterComment || style.optIn.breakChainOnFirstMethodDot)
               val chainLengthPenalty =
                 if (
                   style.newlines.penalizeSingleSelectMultiArgList &&
@@ -1811,13 +1815,14 @@ class Router(formatOps: FormatOps) {
               val nlCost = nlBaseCost + nestedPenalty + chainLengthPenalty
               val nlMod = getNlMod
               val legacySplit = Split(!prevChain, 1) { // must come first, for backwards compat
-                if (style.optIn.breaksInsideChains) NoSplit.orNL(noBreak)
+                if (style.optIn.breaksInsideChains) Newline
+                  .orMod(hasBreak(), modSpace)
                 else nlMod
               }.withPolicy(newlinePolicy).onlyFor(SplitTag.SelectChainSecondNL)
               val slbSplit =
                 if (ignoreNoSplit) Split.ignored
                 else {
-                  val noSplit = Split(NoSplit, 0)
+                  val noSplit = Split(modSpace, 0)
                   if (prevChain) noSplit
                   else chainExpire match { // allow newlines in final {} block
                     case x: T.RightBrace => noSplit
@@ -1830,63 +1835,60 @@ class Router(formatOps: FormatOps) {
                 .withPolicy(newlinePolicy)
               Seq(legacySplit, slbSplit, nlSplit)
             } else {
-              val isComment = left.is[T.Comment]
-              val doBreak = nlOnly || isComment && hasBreak
+              val doBreak = nlOnly || afterComment && hasBreak
               Seq(
                 Split(!prevChain, 1) {
-                  if (style.optIn.breaksInsideChains) NoSplit.orNL(noBreak)
-                  else if (doBreak) Newline
-                  else getNlMod
+                  if (style.optIn.breaksInsideChains) Newline
+                    .orMod(hasBreak(), modSpace)
+                  else Newline.orMod(doBreak, getNlMod)
                 }.withPolicy(breakOnNextDot)
                   .onlyFor(SplitTag.SelectChainSecondNL),
-                Split(if (doBreak) Newline else Space(isComment), 0),
+                Split(Newline.orMod(doBreak, modSpace), 0),
               )
             }
 
-          case _ if left.is[T.Comment] => Seq(Split(Space.orNL(noBreak), 0))
-
           case Newlines.keep =>
             if (hasBreak()) Seq(Split(Newline, 0))
             else if (hasBreakAfterRightBeforeNonComment(ft))
-              Seq(Split(NoSplit, 0).withPolicy(
+              Seq(Split(modSpace, 0).withPolicy(
                 decideNewlinesOnlyAfterClose(Split(Newline, 0))(r),
                 ignore = next(ft).right.is[T.Comment],
               ))
             else Seq(
-              Split(NoSplit, 0),
+              Split(modSpace, 0),
               Split(Newline, 1).onlyFor(SplitTag.SelectChainBinPackNL),
             )
 
           case Newlines.unfold =>
             val nlCost = if (nlOnly) 0 else 1
             if (prevSelect.isEmpty && nextSelect.isEmpty) Seq(
-              Split(nlOnly, 0)(NoSplit).withSingleLine(getSlbEnd()),
+              Split(nlOnly, 0)(modSpace).withSingleLine(getSlbEnd()),
               Split(Newline, nlCost),
             )
             else Seq(
-              Split(nlOnly, 0)(NoSplit)
+              Split(nlOnly, 0)(modSpace)
                 .withSingleLine(expire, noSyntaxNL = true),
-              Split(NewlineT(alt = Some(NoSplit)), nlCost)
+              Split(NewlineT(alt = Some(modSpace)), nlCost)
                 .withPolicy(forcedBreakOnNextDotPolicy),
             )
 
           case Newlines.fold =>
             val nlCost = if (nlOnly) 0 else 1
             def nlSplitBase(implicit fileLine: FileLine) =
-              Split(NewlineT(alt = Some(NoSplit)), nlCost)
+              Split(NewlineT(alt = Some(modSpace)), nlCost)
             if (nextDotIfSig.isEmpty) {
               val noSplit =
                 if (nlOnly) Split.ignored
                 else {
                   val end = nextSelect
                     .fold(expire)(x => getLastNonTrivialToken(x.qual))
                   val exclude = insideBracesBlock(ft, end, parensToo = true)
-                  Split(NoSplit, 0).withSingleLine(end, exclude = exclude)
+                  Split(modSpace, 0).withSingleLine(end, exclude = exclude)
                 }
               Seq(noSplit, nlSplitBase)
             } else {
               val policy: Policy = forcedBreakOnNextDotPolicy
-              val noSplit = Split(nlOnly, 0)(NoSplit).withPolicy(policy)
+              val noSplit = Split(nlOnly, 0)(modSpace).withPolicy(policy)
               Seq(noSplit, nlSplitBase.withPolicy(policy))
             }
         }

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

@@ -9755,8 +9755,8 @@ maxColumn = 37
 ===
 sel.getContext /*ScImportSelectors*/ .getContext.asInstanceOf[ScImportExpr].reference
 >>>
-sel
-  .getContext /*ScImportSelectors*/ .getContext
+sel.getContext /*ScImportSelectors*/
+  .getContext
   .asInstanceOf[ScImportExpr]
   .reference
 <<< #4133 select chain with no-break comment before dot, long

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

@@ -2807,7 +2807,8 @@ class Foo {
   val vv = v
     .aaa //
     //
-    .bbb.ccc()
+    .bbb
+    .ccc()
   val vv = v.aaa //
   val vv = v.aaa
 }
@@ -2827,7 +2828,8 @@ class Foo {
     .bbb //
     //
     .ccc //
-    .ddd.eee()
+    .ddd
+    .eee()
 }
 <<< #1334 3: continue chain indent after a comment with apply
 class Foo {
@@ -2885,7 +2887,8 @@ val a: Vector[Array[Double]] =
     // another comment
     .map { foo =>
       bar
-    }.tail
+    }
+    .tail
 <<< #1334 6: multiple select and a match after a comment
 val a: Vector[Array[Double]] = b.c
        // Only handle first case, others will be fixed on the next pass.
@@ -2898,7 +2901,8 @@ val a: Vector[Array[Double]] = b.c
 val a: Vector[Array[Double]] =
   b.c
     // Only handle first case, others will be fixed on the next pass.
-    .headOption.a match {
+    .headOption
+    .a match {
     case None =>
     case _ =>
   }
@@ -6291,10 +6295,12 @@ object a {
 object a {
   foo
     // c2
-    .bar(baz).foo(bar, baz)
+    .bar(baz)
+    .foo(bar, baz)
   foo // c1
     // c2
-    .bar(baz).foo(bar, baz)
+    .bar(baz)
+    .foo(bar, baz)
   foo
     .bar(baz) // c1
     // c2
@@ -10559,30 +10565,18 @@ maxColumn = 37
 ===
 sel.getContext /*ScImportSelectors*/ .getContext.asInstanceOf[ScImportExpr].reference
 >>>
-BestFirstSearch:278 Failed to format
-UNABLE TO FORMAT,
-tok #4/14: [4] /*ScImportSelectors*/∙.: Comment(ScImportSelectors) [15..36) [ ] Dot [37..38)
-policies:
-  (NEXTSEL1NL:[Router:1896]<48d & SLB:[Router:1868]@85!d)
-splits (before policy):
-  c=0[0] SP:[Router:1846->Router:1846](indents=[], NoPolicy)
-  [SelectChainFirstNL]c=0[0] SP:[Router:1846->Router:1846](indents=[], NEXTSEL1NL:[Router:1896]<61d)
-splits (after policy):
-  [!SelectChainFirstNL]c=0[0] SP:[Router:1846->Router:1846](indents=[], NoPolicy)
-  c=0[0] SP:[Router:1846->Router:1846](indents=[], NEXTSEL1NL:[Router:1896]<61d)
+sel
+  .getContext /*ScImportSelectors*/
+  .getContext
+  .asInstanceOf[ScImportExpr]
+  .reference
 <<< #4133 select chain with no-break comment before dot, long
 maxColumn = 80
 ===
 sel.getContext /*ScImportSelectors*/ .getContext.asInstanceOf[ScImportExpr].reference
 >>>
-BestFirstSearch:278 Failed to format
-UNABLE TO FORMAT,
-tok #12/14: [12] .∙reference: Dot [75..76) [] Ident(reference) [76..85)
-policies:
-  SLB:[Router:1868]@85!d
-  SLB:[Router:1868]@85!d
-  SLB:[Router:1868]@85!d
-splits (before policy):
-  c=0[0] nS:[Router:2551->Router:2551](indents=[], NoPolicy)
-splits (after policy):
-  c=0[0] nS:[Router:2551->Router:2551](indents=[], NoPolicy)
+sel
+  .getContext /*ScImportSelectors*/
+  .getContext
+  .asInstanceOf[ScImportExpr]
+  .reference

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

@@ -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, 1493404, "total explored")
+      assertEquals(explored, 1493674, "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 :'(