Router: fix handling of binpack=oneline

We can't force a newline before closing parenthesis because the policies
for intervening splits might actually disable that, and for scala.js it
would also require forcing a newline after the opening parenthesis.

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

@@ -1186,8 +1186,6 @@ class Router(formatOps: FormatOps) {
         val noNextCommaOneline = oneline && nextCommaOneline.isEmpty
         val noNextCommaOnelineCurried = noNextCommaOneline &&
           leftOwner.parent.exists(followedBySelectOrApply(_).isDefined)
-        val needOnelinePolicy = nextCommaOneline.isDefined ||
-          noNextCommaOnelineCurried
 
         val indentLen = style.indent.callSite
         val indent = Indent(Num(indentLen), close, Before)
@@ -1232,17 +1230,15 @@ class Router(formatOps: FormatOps) {
             def optClose = scalaJsOptClose(beforeClose, flags)
             val opt = if (oneline) nextCommaOneline else findComma(ft)
 
-            val noSplitPolicy =
-              if (needOnelinePolicy) {
-                val alignPolicy =
-                  if (noSplitIndents.exists(_.hasStateColumn))
-                    nextCommaOnelinePolicy.map(_ & penalizeNewlinesPolicy)
-                  else None
-                alignPolicy.getOrElse {
+            val noSplitPolicy = nextCommaOnelinePolicy
+              .fold(penalizeNewlinesPolicy) { p =>
+                if (noSplitIndents.exists(_.hasStateColumn)) p &
+                  penalizeNewlinesPolicy
+                else {
                   val slbEnd = nextCommaOneline.getOrElse(close)
                   SingleLineBlock(slbEnd, noSyntaxNL = true)
                 }
-              } else penalizeNewlinesPolicy
+              }
             val indentPolicy =
               if (noSplitIndents.nonEmpty) {
                 def unindentPolicy =
@@ -1267,11 +1263,12 @@ class Router(formatOps: FormatOps) {
         val nlPolicy = {
           def newlineBeforeClose(implicit fileLine: FileLine) =
             decideNewlinesOnlyBeforeClose(close)
-          nlCloseOnOpen match {
-            case NlClosedOnOpen.No =>
-              if (needOnelinePolicy) nextCommaOnelinePolicy
-                .getOrElse(decideNewlinesOnlyBeforeCloseOnBreak(close))
-              else NoPolicy
+          val nlClosedOnOpenEffective =
+            if (!noNextCommaOnelineCurried) nlCloseOnOpen
+            else if (clauseSiteFlags.configStyle.prefer) NlClosedOnOpen.Cfg
+            else NlClosedOnOpen.Yes
+          nlClosedOnOpenEffective match {
+            case NlClosedOnOpen.No => nextCommaOnelinePolicy.getOrElse(NoPolicy)
             case NlClosedOnOpen.Cfg if !styleMap.forcedBinPack(leftOwner) =>
               splitOneArgOneLine(close, leftOwner) | newlineBeforeClose
             case _ => newlineBeforeClose & nextCommaOnelinePolicy
@@ -1434,13 +1431,23 @@ class Router(formatOps: FormatOps) {
             }
             val nlPolicy = (if (oneline) nextCommaOrParen else None) match {
               case Some(FormatToken(_, t: T.Comma, _)) =>
-                if (callSite) splitOneArgPerLineAfterCommaOnBreak(t)
-                else delayedBreakPolicyFor(t)(decideNewlinesOnlyAfterClose)
-              case Some(FormatToken(_, t, _))
-                  if !callSite ||
-                    leftOwner.parent
-                      .exists(followedBySelectOrApply(_).isDefined) =>
-                decideNewlinesOnlyBeforeCloseOnBreak(t)
+                splitOneArgPerLineAfterCommaOnBreak(t)
+              case Some(_) if callSite =>
+                def delayBreakBefore(token: T)(implicit fileLine: FileLine) =
+                  delayedBreakPolicyFor(token)(decideNewlinesOnlyBeforeToken)
+                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 LeftParenOrBracket() =>
+                      nextNonCommentSameLineAfter(beforeNext).right match {
+                        case _: T.Comment => NoPolicy
+                        case c => delayBreakBefore(c)
+                      }
+                    case _ => NoPolicy
+                  }
+                }
               case _ => NoPolicy
             }
             val indentOncePolicy =
@@ -1452,9 +1459,8 @@ class Router(formatOps: FormatOps) {
                     s.map(x => if (x.isNL) x else x.switch(trigger, true))
                 }
               } else NoPolicy
-            val noSpace = style.newlines.keepBreak(newlines)
             Seq(
-              Split(noSpace, 0)(Space)
+              Split(style.newlines.keepBreak(newlines), 0)(Space)
                 .withSingleLine(endOfSingleLineBlock(optFT), noSyntaxNL = true),
               Split(Newline, 1).withPolicy(nlPolicy & indentOncePolicy),
             )

scalafmt-tests/src/test/resources/newlines/source_classic.stat

@@ -2805,8 +2805,7 @@ object a {
       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
+          e.f.g, h.i.j)).foo
   }
 }
 <<< binpack call, oneline, with syntaxNL, single arg

scalafmt-tests/src/test/resources/newlines/source_keep.stat

@@ -2805,8 +2805,7 @@ object a {
       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
+          e.f.g, h.i.j)).foo
   }
 }
 <<< binpack call, oneline, with syntaxNL, single arg

scalafmt-tests/src/test/resources/scalajs/Apply.stat

@@ -359,7 +359,9 @@ object a {
   foo match {
     case Apply(appMeth,
             Apply(wrapRefArrayMeth,
-                StripCast(arg @ ArrayValue(elemtpt, elems)) :: Nil) :: classTagEvidence :: Nil)
+                StripCast(
+                    arg @ ArrayValue(elemtpt, elems)
+                ) :: Nil) :: classTagEvidence :: Nil)
         if WrapArray.isClassTagBasedWrapArrayMethod(wrapRefArrayMeth.symbol) &&
           appMeth.symbol == ArrayModule_genericApply =>
       bar
@@ -403,7 +405,9 @@ object a {
 object a {
   Apply(appMeth,
       Apply(wrapRefArrayMeth,
-          StripCast(ArrayValue(elemtpt, elems)) :: Nil) :: classTagEvidence :: Nil)
+          StripCast(
+              ArrayValue(elemtpt, elems)
+          ) :: Nil) :: classTagEvidence :: Nil)
 }
 <<< binpack=always, with infix
 maxColumn = 80
@@ -1002,14 +1006,13 @@ object a {
            })(OptimizerHints.empty, Unversioned)
 }
 >>>
-BestFirstSearch:290 Failed to format
-UNABLE TO FORMAT,
-tok=}∙): RightBrace [180..181) [] RightParen [181..182)
-toks.length=48
-deepestYet.length=38
-policies=List(NB:[Router:1460]@182d, NB:[Router:1460]@182d, [FormatOps:2660]@182d, NB:[Router:232]@219d)
-nextSplits=List(NoSplit:[Router:2360](cost=0, indents=[], NoPolicy))
-splitsAfterPolicy=Decision(}∙): RightBrace [180..181) [] RightParen [181..182),List())
+object a {
+  js.MethodDef(flags, methodIdent, originalName, jsParams, resultType,
+      Some {
+        genApplyMethod(genLoadModule(moduleClass), m, jsParams.map(_.ref))
+      })(
+      OptimizerHints.empty, Unversioned)
+}
 <<< nested with oneline, keep
 newlines.source = keep
 binPack.preset = oneline
@@ -1022,11 +1025,10 @@ object a {
            })(OptimizerHints.empty, Unversioned)
 }
 >>>
-BestFirstSearch:290 Failed to format
-UNABLE TO FORMAT,
-tok=}∙): RightBrace [180..181) [] RightParen [181..182)
-toks.length=48
-deepestYet.length=38
-policies=List(NB:[Router:1460]@182d, NB:[Router:1460]@182d, [FormatOps:2660]@182d, NB:[Router:232]@219d)
-nextSplits=List(NoSplit:[Router:2360](cost=0, indents=[], NoPolicy))
-splitsAfterPolicy=Decision(}∙): RightBrace [180..181) [] RightParen [181..182),List())
+object a {
+  js.MethodDef(flags, methodIdent, originalName, jsParams, resultType,
+      Some {
+        genApplyMethod(genLoadModule(moduleClass), m, jsParams.map(_.ref))
+      })(
+      OptimizerHints.empty, Unversioned)
+}