Router: fix bp.callsite=oneline post-comma policy

First, the subsequent break we should force is before a Dot which comes from a Select, not a Comma. Second, we should cancel forcing of that break if there's another break which is output after the argument and before the dot or parenthesis.
scalameta · May 19, 2024 · e5ad60f · e5ad60f
1 parent ce4f2f9
commit e5ad60f
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 9 deletions.
diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala
@@ -1440,13 +1440,21 @@ class Router(formatOps: FormatOps) {
               case Some(FormatToken(_, t: T.Comma, _)) =>
                 splitOneArgPerLineAfterCommaOnBreak(t)
               case Some(_) if callSite =>
-                def delayBreakBefore(token: T)(implicit fileLine: FileLine) =
-                  delayedBreakPolicyFor(token)(decideNewlinesOnlyBeforeToken)
+                def delayBreakBefore(token: T): Policy = {
+                  // force break if multiline and if there's no other break
+                  val lastEnd = lastFT.left.end
+                  delayedBreakPolicy(Policy.End > lastEnd)(
+                    Policy.RelayOnSplit { case (s, nextft) =>
+                      s.isNL && nextft.left.end > lastEnd // don't need anymore
+                    }(decideNewlinesOnlyBeforeToken(token), NoPolicy),
+                  )
+                }
+
                 val endCall = leftOwner.parent.flatMap(followedBySelectOrApply)
                 endCall.fold(Policy.noPolicy) { x =>
                   val beforeNext = nextNonCommentSameLine(getLastNonTrivial(x))
                   beforeNext.right match {
-                    case c: T.Comma => delayBreakBefore(c)
+                    case c: T.Dot => delayBreakBefore(c)
                     case LeftParenOrBracket() =>
                       nextNonCommentSameLineAfter(beforeNext).right match {
                         case _: T.Comment => NoPolicy

diff --git a/scalafmt-tests/src/test/resources/newlines/source_classic.stat b/scalafmt-tests/src/test/resources/newlines/source_classic.stat
@@ -2803,9 +2803,7 @@ object a {
   test("foo") {
     a.b(c, d) shouldBe
       E(Seq(F(1, "v1"), F(2, "v2")),
-        G(Some(Seq(h, i)),
-          Some(Seq(j, k)), a.b, c.d,
-          e.f.g, h.i.j)).foo
+        G(Some(Seq(h, i)), Some(Seq(j, k)), a.b, c.d, e.f.g, h.i.j)).foo
   }
 }
 <<< binpack call, oneline, with syntaxNL, single arg

diff --git a/scalafmt-tests/src/test/resources/newlines/source_keep.stat b/scalafmt-tests/src/test/resources/newlines/source_keep.stat
@@ -2803,9 +2803,7 @@ object a {
   test("foo") {
     a.b(c, d) shouldBe
       E(Seq(F(1, "v1"), F(2, "v2")),
-        G(Some(Seq(h, i)),
-          Some(Seq(j, k)), a.b, c.d,
-          e.f.g, h.i.j)).foo
+        G(Some(Seq(h, i)), Some(Seq(j, k)), a.b, c.d, e.f.g, h.i.j)).foo
   }
 }
 <<< binpack call, oneline, with syntaxNL, single arg