Router: Align binpack.{call,defn}Site behaviour

Also, modify it so that it matches configuration parameters better. In
most cases, it will either preserve formatting (which tests may not show
as they operate on non-formatted code) or can easily be configured to
match existing formatting.

This change partially restores some of formatting changes introduced in
recent commits, by making their results more consistent with new logic.

docs/configuration.md

@@ -4792,6 +4792,7 @@ object Example2 {
 
 This group of parameters controls binpacking of an argument list if _all_ arguments are
 considered to be literals.
+These parameters take precedence over [forcing of config style](#forcing-config-style).
 
 The following parameters affect this behaviour:
 
@@ -4919,65 +4920,65 @@ object A {
 }
 ```
 
-### `binPack.defnSite`
+### `binPack.xxxSite`
 
-Controls binpacking around method/type definition sites (and was called
-`unsafeDefnSite` up until v3.8.1). The following parameter values are supported
+Controls binpacking around method/type definition sites (`binPack.defnSite`) or
+method call sites (`binPack.callSite`) (both were called
+`unsafeXxxSite` up until v3.8.1). The following parameter values are supported
 since v3.0.0:
 
 - `Never` disables the functionality (also takes `false`)
 - `Always` enables the functionality (also takes `true`)
 - `Oneline` ensures multiline arguments are not binpacked
 
-When not disabled, this parameter has complex interactions with
+When not disabled, these parameters have complex interactions with
 [`newline.source`](#newlinessource),
-[config-style formatting](#newlines-config-style-formatting) and
-[`danglingParentheses.defnSite`](#danglingparenthesesdefnsite).
-
-- `newlines.source=classic/keep`
-  - open break is preserved for `keep`, matches close break for `classic`
-  - when `config-style` is enabled: close break is preserved,
-    `danglingParentheses.defnSite` is ignored
-  - when `config-style` is disabled: `danglingParentheses.defnSite`
-    dictates close break
-- `newlines.source=fold/unfold`
-  - open break matches close break for `fold`, dangles for `unfold`
-  - `danglingParentheses.defnSite` dictates close break
-  - `config-style` is ignored
-
-### `binPack.callSite`
-
-Controls binpacking around method call sites (and was called `unsafeCallSite` up
-until v3.8.1). It takes the same values as [`binPack.defnSite`](#binpackdefnsite),
-and similarly has cross-parameter interactions:
-
-- interaction with `config-style` parameters:
-  - when [config-style is forced](#forcing-config-style), it takes precedence
-    over binpacking
-  - for `newlines.source=classic`, behaviour depends on
-    [config-style](#newlinesconfigstylexxxsiteprefer):
-    - if enabled, config style is used if
-      - it is [detected](#newlinesconfigstylexxxsiteprefer), or
-      - configured to use [scala.js style](#presetscalajs)
-    - otherwise, uses binpacking
-  - for other values of [`newlines.source`](#newlinessource),
-    binpacking takes precedence
-- interaction with [`danglingParentheses.callSite`](#danglingparenthesescallsite)
-  - `newlines.source=classic`: please see above
-  - `newlines.source=keep`
-    - open break is preserved
-    - when both [config-style](#newlinesconfigstylexxxsiteprefer) and
-      [`danglingParentheses.callSite`](#danglingparenthesescallsite) are disabled,
-      close break is "tucked"
-    - otherwise, close break matches open break
-  - `newlines.source=fold/unfold`
-    - when [`danglingParentheses.callSite`](#danglingparenthesescallsite) is enabled,
-      open break matches close break, and close is always dangling for `unfold`,
-      and only when [config-style is forced](#forcing-config-style) for `fold`
-    - otherwise, open is always dangling,
-      and close is dangling only when both
-      [`newlines.configStyleXxxSite.prefer=true`](#newlinesconfigstylexxxsiteprefer)
-      and [config-style is forced](#forcing-config-style)
+[`newlines.configStyleXxxSite.prefer`](#newlinesconfigstylexxxsiteprefer)
+(aka `cfgStyle` below) and
+[`danglingParentheses.xxxSite`](#newlines-danglingparentheses) (aka `dangle`).
+Keep in mind that when [config-style is forced](#forcing-config-style),
+it takes precedence over options described below.
+
+- `newlines.source=classic`
+  - `cfgStyle=T`, `dangle=T`:
+    use cfg-style if both parens had breaks, otherwise binpack without breaks
+    - before v3.8.2, this formatting was used for `callSite` with `dangle=F` as well
+  - `cfgStyle=T`, `dangle=F`:
+    ([scala.js](#presetscalajs))
+    use cfg-style if close paren had a break; otherwise, binpack without breaks
+  - `cfgStyle=F`, `dangle=T`:
+    binpack; if both parens had breaks, keep; otherwise, use no breaks
+    - before v3.8.2, this formatting was used for `defnSite` with
+      `cfgStyle=T` and any `dangle`
+  - `cfgStyle=F`, `dangle=F`:
+    binpack without breaks
+    - before v3.8.2, this formatting was used for `callSite` with
+      `cfgStyle=F` and any `dangle`, and for `defnSite` with
+      `cfgStyle=F` and `dangle=F`
+- `newlines.source=keep`
+  - `cfgStyle=T`, `dangle=T`:
+    use cfg-style if open paren had a break; otherwise, binpack and preserve both breaks
+  - `cfgStyle=T`, `dangle=F`:
+    ([scala.js](#presetscalajs))
+    use cfg-style if close paren had a break; otherwise, binpack without breaks
+  - `cfgStyle=F`, `dangle=T`:
+    binpack; if open paren had a break, force both breaks; otherwise, preserve both
+  - `cfgStyle=F`, `dangle=F`:
+    binpack; preserve both breaks
+- `newlines.source=fold`: if single line is not possible:
+  - `cfgStyle=T`, `dangle=T`:
+    binpack with both breaks
+  - `cfgStyle=T`, `dangle=F`:
+    binpack with dangling open and tucked close
+  - `cfgStyle=F`, `dangle=T`:
+    binpack with tucked open and dangling close
+    - if [`binPack.indentCallSiteOnce`](#binpackindentcallsiteonce) is set,
+      we will not force dangling as it might lead to consecutive lines
+      with a closing parenthesis at the same indentation level
+  - `cfgStyle=F`, `dangle=F`:
+    binpack without breaks
+- `newlines.source=unfold`: if single line is not possible:
+  - open dangles, close break matches `dangle`, `cfgStyle` is ignored
 
 > Please also see [callSite indentation parameters](#indent-for-binpackcallsite).
 

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

@@ -857,19 +857,6 @@ class FormatOps(
     case _ => tokens.getLastExceptParen(function).left -> ExpiresOn.After
   }
 
-  def mustUseConfigStyle(
-      ft: FormatToken,
-      breakBeforeClose: => Boolean,
-      allowForce: Boolean = true,
-  )(implicit
-      style: ScalafmtConfig,
-      clauseSiteFlags: ClauseSiteFlags,
-  ): ConfigStyle =
-    if (allowForce && mustForceConfigStyle(ft)(clauseSiteFlags.configStyle))
-      ConfigStyle.Forced
-    else if (preserveConfigStyle(ft, breakBeforeClose)) ConfigStyle.Source
-    else ConfigStyle.None
-
   def mustForceConfigStyle(ft: FormatToken)(implicit
       cfg: Newlines.ConfigStyleElement,
   ): Boolean = cfg.getForceIfOptimized && forceConfigStyle(ft.meta.idx)
@@ -2644,31 +2631,84 @@ class FormatOps(
     beforeClose.exists(rightIsCloseDelimToAddTrailingComma(_, closeFt))
   }
 
-  def getBinpackCallsiteFlags(
+  def getBinpackCallSiteFlags(
       ftAfterOpen: FormatToken,
       ftBeforeClose: FormatToken,
   )(implicit style: ScalafmtConfig, clauseSiteFlags: ClauseSiteFlags) = {
     val literalArgList = styleMap.opensLiteralArgumentList(ftAfterOpen)
+    getBinpackSiteFlags(ftAfterOpen, ftBeforeClose, literalArgList)
+  }
+
+  def getBinpackSiteFlags(
+      ftAfterOpen: FormatToken,
+      ftBeforeClose: FormatToken,
+      literalArgList: Boolean,
+  )(implicit style: ScalafmtConfig, clauseSiteFlags: ClauseSiteFlags) = {
+    implicit val configStyle = clauseSiteFlags.configStyle
+    val configStylePrefer = configStyle.prefer
+    val shouldDangle = clauseSiteFlags.dangleCloseDelim
+    val sourceIgnored = style.newlines.sourceIgnored
+    val configStyleSource = configStylePrefer && !sourceIgnored
     val dangleForTrailingCommas = getMustDangleForTrailingCommas(ftBeforeClose)
-    val configStyle =
-      if (dangleForTrailingCommas) ConfigStyle.None
-      else
-        mustUseConfigStyle(ftAfterOpen, ftBeforeClose.hasBreak, !literalArgList)
-    val scalaJsStyle = style.newlines.source == Newlines.classic &&
-      configStyle == ConfigStyle.None && !literalArgList &&
-      !clauseSiteFlags.dangleCloseDelim && clauseSiteFlags.configStyle.prefer
-    BinpackCallsiteFlags(
+    val scalaJsStyle = configStyleSource && !shouldDangle
+    val closeBreak = dangleForTrailingCommas || ftBeforeClose.hasBreak
+
+    def noNLPolicy(): Policy = {
+      val close = ftBeforeClose.right
+      if (scalaJsStyle) Policy(Policy.End.On(close)) {
+        case d: Decision if d.formatToken.right eq close => d.noNewlines
+      }
+      else style.newlines.source match {
+        case Newlines.keep if closeBreak => decideNewlinesOnlyBeforeClose(close)
+        case Newlines.fold
+            if shouldDangle && !style.binPack.indentCallSiteOnce =>
+          decideNewlinesOnlyBeforeCloseOnBreak(close)
+        case _ => NoPolicy
+      }
+    }
+
+    def nlOpenClose(): (Boolean, NlClosedOnOpen) =
+      if (!literalArgList && mustForceConfigStyle(ftAfterOpen))
+        (true, NlClosedOnOpen.Cfg)
+      else {
+        val openBreak = ftAfterOpen.hasBreak
+        val nlOpenExcludingCfg = dangleForTrailingCommas ||
+          (style.newlines.source match {
+            case Newlines.classic => openBreak && shouldDangle && closeBreak
+            case Newlines.keep => openBreak
+            case _ => false
+          }) || tokens.isRightCommentWithBreak(ftAfterOpen)
+        val nlBothIncludingCfg = !sourceIgnored && closeBreak && {
+          scalaJsStyle || nlOpenExcludingCfg ||
+          preserveConfigStyle(ftAfterOpen, true)
+        }
+        // close on open NL; doesn't cover case with no break after open
+        val nlClose = nlBothIncludingCfg || dangleForTrailingCommas ||
+          shouldDangle || style.newlines.keepBreak(closeBreak)
+        if (!nlClose) (nlOpenExcludingCfg && !scalaJsStyle, NlClosedOnOpen.No)
+        else {
+          val cfg = !literalArgList && configStyleSource
+          val dangle = if (cfg) NlClosedOnOpen.Cfg else NlClosedOnOpen.Yes
+          (nlBothIncludingCfg || nlOpenExcludingCfg, dangle)
+        }
+      }
+
+    BinpackSiteFlags(
       literalArgList = literalArgList,
-      dangleForTrailingCommas = dangleForTrailingCommas,
-      configStyle = configStyle,
-      scalaJsStyle = scalaJsStyle,
+      nlOpenClose = nlOpenClose,
+      noNLPolicy = style.newlines.source match {
+        case Newlines.unfold => null
+        case Newlines.fold if configStylePrefer => null
+        case _ => noNLPolicy
+      },
+      scalaJsStyle = scalaJsStyle && !literalArgList,
     )
   }
 
   @tailrec
   final def scalaJsOptClose(
       ftBeforeClose: FormatToken,
-      bpFlags: BinpackCallsiteFlags,
+      bpFlags: BinpackSiteFlags,
   ): T =
     if (bpFlags.scalaJsStyle) {
       val ftAfterClose = tokens.nextNonCommentAfter(ftBeforeClose)
@@ -2680,7 +2720,7 @@ class FormatOps(
         implicit val style: ScalafmtConfig = styleMap.at(open)
         implicit val clauseSiteFlags: ClauseSiteFlags = ClauseSiteFlags
           .atCallSite(ftAfterClose.meta.rightOwner)
-        val bpFlagsAfter = getBinpackCallsiteFlags(tokens(open), ftAfterClose)
+        val bpFlagsAfter = getBinpackCallSiteFlags(tokens(open), ftAfterClose)
         scalaJsOptClose(ftAfterClose, bpFlagsAfter)
       } else ftBeforeClose.right
     } else ftBeforeClose.right
@@ -2727,10 +2767,17 @@ object FormatOps {
     )
     else Seq(Indent(Length.StateColumn, end, ExpiresOn.Before))
 
-  case class BinpackCallsiteFlags(
+  private[internal] sealed trait NlClosedOnOpen
+  private[internal] object NlClosedOnOpen {
+    case object No extends NlClosedOnOpen
+    case object Yes extends NlClosedOnOpen
+    case object Cfg extends NlClosedOnOpen
+  }
+
+  private[internal] case class BinpackSiteFlags(
       literalArgList: Boolean,
-      dangleForTrailingCommas: Boolean,
-      configStyle: ConfigStyle,
+      nlOpenClose: () => (Boolean, NlClosedOnOpen),
+      noNLPolicy: () => Policy, // nullable
       scalaJsStyle: Boolean,
   )