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.

docs/configuration.md

@@ -4753,6 +4753,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:
 
@@ -4880,66 +4881,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: used if [detected](#newlines-config-style-formatting), otherwise binpacked
-    - if disabled with both [`danglingParentheses.callSite`](#danglingparenthesescallsite)
-      enabled and closing parenthesis following a break: forces config-style, as described in
-      [scala.js](https://github.com/scala-js/scala-js/pull/4522#issuecomment-879168123)
-    - 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](https://github.com/scala-js/scala-js/pull/4522#issuecomment-879168123))
+    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](https://github.com/scala-js/scala-js/pull/4522#issuecomment-879168123))
+    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

@@ -861,13 +861,9 @@ class FormatOps(
       ft: FormatToken,
       breakBeforeClose: => Boolean,
       allowForce: Boolean = true,
-  )(implicit
-      style: ScalafmtConfig,
-      cfg: Newlines.ConfigStyleElement,
-  ): ConfigStyle =
-    if (allowForce && mustForceConfigStyle(ft)) ConfigStyle.Forced
-    else if (preserveConfigStyle(ft, breakBeforeClose)) ConfigStyle.Source
-    else ConfigStyle.None
+  )(implicit style: ScalafmtConfig, cfg: Newlines.ConfigStyleElement): Boolean =
+    allowForce && mustForceConfigStyle(ft) ||
+      preserveConfigStyle(ft, breakBeforeClose)
 
   def mustForceConfigStyle(ft: FormatToken)(implicit
       cfg: Newlines.ConfigStyleElement,
@@ -2644,30 +2640,98 @@ class FormatOps(
       ftBeforeClose: FormatToken,
   )(implicit style: ScalafmtConfig) = {
     val literalArgList = styleMap.opensLiteralArgumentList(ftAfterOpen)
-    val dangleForTrailingCommas = getMustDangleForTrailingCommas(ftBeforeClose)
     implicit val configStyleFlags = style.configStyleCallSite
-    val configStyle =
-      if (dangleForTrailingCommas) ConfigStyle.None
-      else
-        mustUseConfigStyle(ftAfterOpen, ftBeforeClose.hasBreak, !literalArgList)
     val shouldDangle = style.danglingParentheses
       .atCallSite(ftAfterOpen.meta.leftOwner)
-    val scalaJsStyle = style.newlines.source == Newlines.classic &&
-      configStyle == ConfigStyle.None && !configStyleFlags.prefer &&
-      !literalArgList && shouldDangle
-    BinpackCallsiteFlags(
+    getBinpackSiteFlags(ftAfterOpen, ftBeforeClose, literalArgList, shouldDangle)
+  }
+
+  @inline
+  def getBinpackDefnsiteFlags(
+      ftAfterOpen: FormatToken,
+      ftBeforeClose: FormatToken,
+  )(implicit style: ScalafmtConfig): BinpackSiteFlags = {
+    implicit val configStyleFlags = style.configStyleDefnSite
+    getBinpackSiteFlags(
+      ftAfterOpen,
+      ftBeforeClose,
+      literalArgList = false,
+      shouldDangle = style.danglingParentheses
+        .atDefnSite(ftAfterOpen.meta.leftOwner),
+    )
+  }
+
+  def getBinpackSiteFlags(
+      ftAfterOpen: FormatToken,
+      ftBeforeClose: FormatToken,
+      literalArgList: Boolean,
+      shouldDangle: Boolean,
+  )(implicit
+      style: ScalafmtConfig,
+      configStyleFlags: Newlines.ConfigStyleElement,
+  ): BinpackSiteFlags = {
+    val configStylePrefer = configStyleFlags.prefer
+    val sourceIgnored = style.newlines.sourceIgnored
+    val configStyleSource = configStylePrefer && !sourceIgnored
+    val scalaJsStyle = configStyleSource && !shouldDangle
+    val closeBreak = ftBeforeClose.hasBreak
+
+    def noNLPolicy(): Policy = {
+      val close = ftBeforeClose.right
+      if (scalaJsStyle) Policy(Policy.End.On(ftBeforeClose.right)) {
+        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 && !configStylePrefer &&
+              !style.binPack.indentCallSiteOnce =>
+          decideNewlinesOnlyBeforeCloseOnBreak(close)
+        case _ => NoPolicy
+      }
+    }
+
+    def nlOpenClose(): (Boolean, NlClosedOnOpen) =
+      if (!literalArgList && mustForceConfigStyle(ftAfterOpen))
+        (true, NlClosedOnOpen.Cfg)
+      else {
+        val openBreak = ftAfterOpen.hasBreak
+        val dangleForTrailingCommas =
+          getMustDangleForTrailingCommas(ftBeforeClose)
+        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,
+      nlOpenClose = nlOpenClose,
+      noNLPolicy = noNLPolicy,
       shouldDangle = shouldDangle,
-      scalaJsStyle = scalaJsStyle,
+      scalaJsStyle = scalaJsStyle && !literalArgList,
     )
   }
 
   @tailrec
   final def scalaJsOptClose(
       ftBeforeClose: FormatToken,
-      bpFlags: BinpackCallsiteFlags,
+      bpFlags: BinpackSiteFlags,
   )(implicit style: ScalafmtConfig): T =
     if (bpFlags.scalaJsStyle) {
       val ftAfterClose = tokens.nextNonCommentAfter(ftBeforeClose)
@@ -2725,10 +2789,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,
       shouldDangle: Boolean,
       scalaJsStyle: Boolean,
   )