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

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,
   )