From e5ad60f2ffe4f9097e21fa6709d6b88a143b73d5 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Sat, 18 May 2024 11:08:42 -0700 Subject: [PATCH] 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. --- .../main/scala/org/scalafmt/internal/Router.scala | 14 +++++++++++--- .../test/resources/newlines/source_classic.stat | 4 +--- .../src/test/resources/newlines/source_keep.stat | 4 +--- 3 files changed, 13 insertions(+), 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 index fe621a096c..7092f06720 100644 --- 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 index 6a9d3f73e4..2083dc5333 100644 --- 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 index 272f0988d4..bdf5d3027a 100644 --- 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