Config-style: separate control for call/defn sites

Currently, the `optIn.configStyleArguments` parameter applies both to call-site as well as defn-site clauses. Instead, let's move it to `newlines.configStyle.xxxSite` and separate the two.
scalameta · May 6, 2024 · ae5da57 · ae5da57
1 parent c32070f
commit ae5da57
Show file tree

Hide file tree

Showing 10 changed files with 132 additions and 96 deletions.
diff --git a/docs/configuration.md b/docs/configuration.md
@@ -2306,14 +2306,14 @@ newlines.implicitParamListModifierForce = [before,after]
 def format(code: String, age: Int)(implicit ev: Parser, c: Context): String
 ```
 
-#### Implicit with `optIn.configStyleArguments`
+#### Implicit with `newlines.configStyleDefnSite.prefer`
 
 While config-style normally requires a newline after the opening parenthesis,
 postponing that break until after the `implicit` keyword is allowed if other
 parameters require keeping this keyword attached to the opening brace.
 
 Therefore, any of the parameters described in this section will take precedence
-even when `optIn.configStyleArguments = true` is used.
+even when `newlines.configStyleDefnSite.prefer = true` is used.
 
 ### `newlines.afterInfix`
 
@@ -2787,10 +2787,12 @@ It normally involves a newline after the opening parenthesis (or after the
 As part of the formatting output, arguments are output one per line (but this is
 not used in determining whether the source uses config-style formatting).
 
-While this parameter is not technically under the `newlines` section, it
-logically belongs there.
+### `newlines.configStyleXxxSite.prefer`
 
-### `optIn.configStyleArguments`
+`CallSite` applies to argument clauses method calls, while `DefnSite` to
+parameter clauses in method or class definitions.
+In v3.8.2 replaced a single parameter `optIn.configStyleArguments` and
+falls back to its value (which is enabled by default).
 
 If true, applies config-style formatting:
 
@@ -2799,12 +2801,8 @@ If true, applies config-style formatting:
 
 Please note that other parameters might also force config-style (see below).
 
-```scala mdoc:defaults
-optIn.configStyleArguments
-```
-
 ```scala mdoc:scalafmt
-optIn.configStyleArguments = true
+newlines.configStyleDefnSite.prefer = true
 maxColumn=45
 ---
 object a {
@@ -4870,7 +4868,7 @@ and similarly has cross-parameter interactions:
   - when [config-style is forced](#forcing-config-style), it takes precedence
     over binpacking
   - for `newlines.source=classic`, behaviour depends on
-    [config-style](#optinconfigstylearguments):
+    [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
@@ -4882,7 +4880,7 @@ and similarly has cross-parameter interactions:
   - `newlines.source=classic`: please see above
   - `newlines.source=keep`
     - open break is preserved
-    - when both [config-style](#optinconfigstylearguments) and
+    - when both [config-style](#newlinesconfigstylexxxsiteprefer) and
       [`danglingParentheses.callSite`](#danglingparenthesescallsite) are disabled,
       close break is "tucked"
     - otherwise, close break matches open break
@@ -4892,7 +4890,7 @@ and similarly has cross-parameter interactions:
       and only when [config-style is forced](#forcing-config-style) for `fold`
     - otherwise, open is always dangling,
       and close is dangling only when both
-      [`optIn.configStyleArguments=true`](#optinconfigstylearguments)
+      [`newlines.configStyleXxxSite.prefer=true`](#newlinesconfigstylexxxsiteprefer)
       and [config-style is forced](#forcing-config-style)
 
 > Please also see [callSite indentation parameters](#indent-for-binpackcallsite).

diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/config/Newlines.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/config/Newlines.scala
@@ -217,6 +217,8 @@ case class Newlines(
     inInterpolation: InInterpolation = InInterpolation.allow,
     ignoreInSyntax: Boolean = true,
     avoidAfterYield: Boolean = true,
+    configStyleCallSite: Option[ConfigStyleElement] = None,
+    configStyleDefnSite: Option[ConfigStyleElement] = None,
 ) {
   if (
     implicitParamListModifierForce.nonEmpty &&
@@ -539,4 +541,17 @@ object Newlines {
     }
   }
 
+  /** Clauses where there is a newline after opening `(`` and newline before
+    * closing `)`. If true, preserves the newlines and keeps one line per
+    * argument.
+    */
+  case class ConfigStyleElement(prefer: Boolean = true)
+  private[config] object ConfigStyleElement {
+    private val default = ConfigStyleElement()
+    implicit val surface: Surface[ConfigStyleElement] = generic
+      .deriveSurface[ConfigStyleElement]
+    implicit val codec: ConfCodecEx[ConfigStyleElement] = generic
+      .deriveCodecEx(default).noTypos
+  }
+
 }
diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/config/OptIn.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/config/OptIn.scala
@@ -100,7 +100,12 @@ import metaconfig.generic.Surface
   *   }}}
   */
 case class OptIn(
-    configStyleArguments: Boolean = true,
+    @annotation.DeprecatedName(
+      "configStyleArguments",
+      "Use `newlines.configStyleCallSite` instead",
+      "3.8.2",
+    )
+    private[config] val configStyleArguments: Boolean = true,
     breaksInsideChains: Boolean = false,
     breakChainOnFirstMethodDot: Boolean = true,
     encloseClassicChains: Boolean = false,

diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/config/ScalafmtConfig.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/config/ScalafmtConfig.scala
@@ -277,6 +277,16 @@ case class ScalafmtConfig(
   def getFewerBraces(): Indents.FewerBraces =
     if (indent.getSignificant < 2) Indents.FewerBraces.never
     else indent.fewerBraces
+
+  private def getOptInConfigStyle: Newlines.ConfigStyleElement = Newlines
+    .ConfigStyleElement(prefer = optIn.configStyleArguments)
+
+  lazy val configStyleCallSite: Newlines.ConfigStyleElement = newlines
+    .configStyleCallSite.getOrElse(getOptInConfigStyle)
+
+  lazy val configStyleDefnSite: Newlines.ConfigStyleElement = newlines
+    .configStyleDefnSite.getOrElse(getOptInConfigStyle)
+
 }
 
 object ScalafmtConfig {
@@ -363,12 +373,12 @@ object ScalafmtConfig {
       implicit val errors = new mutable.ArrayBuffer[String]
       if (newlines.sourceIgnored) {
         newlines.beforeOpenParenCallSite.fold(addIfDirect(
-          optIn.configStyleArguments && align.openParenCallSite,
-          "optIn.configStyleArguments && align.openParenCallSite && !newlines.beforeOpenParenCallSite",
+          configStyleCallSite.prefer && align.openParenCallSite,
+          "newlines.configStyleCallSite && align.openParenCallSite && !newlines.beforeOpenParenCallSite",
         ))(x => addIfDirect(x.src eq Newlines.keep, "newlines.beforeOpenParenCallSite.src = keep"))
         newlines.beforeOpenParenDefnSite.fold(addIfDirect(
-          optIn.configStyleArguments && align.openParenDefnSite,
-          "optIn.configStyleArguments && align.openParenDefnSite && !newlines.beforeOpenParenDefnSite",
+          configStyleDefnSite.prefer && align.openParenDefnSite,
+          "newlines.configStyleDefnSite && align.openParenDefnSite && !newlines.beforeOpenParenDefnSite",
         ))(x => addIfDirect(x.src eq Newlines.keep, "newlines.beforeOpenParenDefnSite.src = keep"))
         def mustIgnoreSourceSplit(what: sourcecode.Text[Option[Newlines.IgnoreSourceSplit]]) = what.value
           .foreach(x => addIfDirect(!x.ignoreSourceSplit, s"${what.source}=$x"))

diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala
@@ -861,19 +861,23 @@ class FormatOps(
       ft: FormatToken,
       beforeCloseFt: => FormatToken,
       allowForce: Boolean = true,
-  )(implicit style: ScalafmtConfig): ConfigStyle =
-    if (style.optIn.configStyleArguments)
-      couldUseConfigStyle(ft, beforeCloseFt, allowForce)
+  )(implicit
+      style: ScalafmtConfig,
+      cfg: Newlines.ConfigStyleElement,
+  ): ConfigStyle =
+    if (allowForce && mustForceConfigStyle(ft)) ConfigStyle.Forced
+    else if (preserveConfigStyle(ft, beforeCloseFt)) ConfigStyle.Source
     else ConfigStyle.None
 
-  def couldUseConfigStyle(
+  def mustForceConfigStyle(ft: FormatToken)(implicit
+      cfg: Newlines.ConfigStyleElement,
+  ): Boolean = cfg.prefer && forceConfigStyle(ft.meta.idx)
+
+  def preserveConfigStyle(
       ft: FormatToken,
       beforeCloseFt: => FormatToken,
-      allowForce: Boolean = true,
-  )(implicit style: ScalafmtConfig): ConfigStyle =
-    if (allowForce && forceConfigStyle(ft.meta.idx)) ConfigStyle.Forced
-    else if (couldPreserveConfigStyle(ft, beforeCloseFt)) ConfigStyle.Source
-    else ConfigStyle.None
+  )(implicit style: ScalafmtConfig, cfg: Newlines.ConfigStyleElement): Boolean =
+    cfg.prefer && couldPreserveConfigStyle(ft, beforeCloseFt)
 
   def couldPreserveConfigStyle(ft: FormatToken, beforeCloseFt: => FormatToken)(
       implicit style: ScalafmtConfig,
@@ -1192,7 +1196,7 @@ class FormatOps(
         else Nil
       val noSlb = implicitNL || aboveArityThreshold ||
         ft.hasBreak &&
-        !style.newlines.sourceIgnored && style.optIn.configStyleArguments ||
+        !style.newlines.sourceIgnored && style.configStyleDefnSite.prefer ||
         implicitParams.nonEmpty &&
         style.newlines.forceAfterImplicitParamListModifier
       val nlNoAlt = implicitNL ||
@@ -2637,13 +2641,14 @@ class FormatOps(
   )(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, !literalArgList)
     val shouldDangle = style.danglingParentheses
       .tupleOrCallSite(isTuple(ftAfterOpen.meta.leftOwner))
     val scalaJsStyle = style.newlines.source == Newlines.classic &&
-      configStyle == ConfigStyle.None && !style.optIn.configStyleArguments &&
+      configStyle == ConfigStyle.None && !configStyleFlags.prefer &&
       !literalArgList && shouldDangle
     BinpackCallsiteFlags(
       literalArgList = literalArgList,

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
@@ -865,9 +865,11 @@ class Router(formatOps: FormatOps) {
         val bracketCoef = if (isBracket) Constants.BracketPenalty else 1
 
         val rightIsComment = right.is[T.Comment]
+        implicit val configStyleFlags =
+          if (defnSite) style.configStyleDefnSite else style.configStyleCallSite
         val configStyle = mustUseConfigStyle(formatToken, beforeClose)
         val onlyConfigStyle = ConfigStyle.None != configStyle
-        val configStyleFlag = style.optIn.configStyleArguments
+        val configStyleFlag = configStyleFlags.prefer
 
         val sourceIgnored = style.newlines.sourceIgnored
         val (onlyArgument, isSingleEnclosedArgument) =
@@ -1105,6 +1107,7 @@ class Router(formatOps: FormatOps) {
           val penalizeBrackets = bracketPenalty
             .map(p => PenalizeAllNewlines(close, p + 3))
           val beforeClose = tokens.justBefore(close)
+          implicit val configStyleFlags = style.configStyleDefnSite
           val onlyConfigStyle = getMustDangleForTrailingCommas(beforeClose) ||
             ConfigStyle.None != mustUseConfigStyle(formatToken, beforeClose)
 
@@ -1120,7 +1123,7 @@ class Router(formatOps: FormatOps) {
 
           val mustDangle = onlyConfigStyle ||
             style.danglingParentheses.defnSite &&
-            (style.newlines.sourceIgnored || !style.optIn.configStyleArguments)
+            (style.newlines.sourceIgnored || !configStyleFlags.prefer)
           val slbOnly = mustDangle || style.newlines.source.eq(Newlines.unfold)
           def noSplitPolicy: Policy =
             if (slbOnly) slbPolicy
@@ -1301,7 +1304,7 @@ class Router(formatOps: FormatOps) {
           else if (
             flags.dangleForTrailingCommas ||
             flags.shouldDangle &&
-            (style.newlines.sourceIgnored || !style.optIn.configStyleArguments)
+            (style.newlines.sourceIgnored || !style.configStyleCallSite.prefer)
           ) bothPolicies
           else binPackOnelinePolicyOpt
             .getOrElse(decideNewlinesOnlyBeforeCloseOnBreak(close))

diff --git a/scalafmt-tests/src/test/resources/newlines/source_classic.stat b/scalafmt-tests/src/test/resources/newlines/source_classic.stat
@@ -7603,8 +7603,8 @@ object a {
       " try {x.add(y, null);} catch(e) {if (typeof(e) == 'object' && typeof(e.number) == 'number' && (e.number & 0xFFFF) == 5){ x.add(y,x.options.length); } } "
   }
 }
-<<< binPack.callSite with configStyleArguments, danglingParentheses
-optIn.configStyleArguments = true
+<<< binPack.callSite with configStyle, danglingParentheses
+newlines.configStyleCallSite.prefer = true
 danglingParentheses.callSite = true
 binPack.unsafeCallSite = always
 maxColumn = 30
@@ -7655,8 +7655,8 @@ object Main {
     10 + 0
   )
 }
-<<< binPack.defnSite with configStyleArguments, danglingParentheses
-optIn.configStyleArguments = true
+<<< binPack.defnSite with configStyle, danglingParentheses
+newlines.configStyleDefnSite.prefer = true
 danglingParentheses.defnSite = true
 binPack.unsafeDefnSite = always
 maxColumn = 30
@@ -7702,8 +7702,8 @@ object Main {
       x3: X, x4: X, xs: X*)
       : Set[Int]
 }
-<<< binPack.callSite with !configStyleArguments, danglingParentheses
-optIn.configStyleArguments = false
+<<< binPack.callSite with !configStyle, danglingParentheses
+newlines.configStyleCallSite.prefer = false
 danglingParentheses.callSite = true
 binPack.unsafeCallSite = always
 maxColumn = 30
@@ -7746,8 +7746,8 @@ object Main {
   val bar2 = foo2(0, 1, 2, 3,
     4, 5, 6, 7, 8, 9, 10 + 0)
 }
-<<< binPack.defnSite with !configStyleArguments, danglingParentheses
-optIn.configStyleArguments = false
+<<< binPack.defnSite with !configStyle, danglingParentheses
+newlines.configStyleDefnSite.prefer = false
 danglingParentheses.defnSite = true
 binPack.unsafeDefnSite = always
 maxColumn = 30
@@ -7797,8 +7797,8 @@ object Main {
       x4: X, xs: X*
   ): Set[Int]
 }
-<<< binPack.callSite with configStyleArguments, !danglingParentheses
-optIn.configStyleArguments = true
+<<< binPack.callSite with configStyle, !danglingParentheses
+newlines.configStyleCallSite.prefer = true
 danglingParentheses.callSite = false
 binPack.unsafeCallSite = always
 maxColumn = 30
@@ -7849,8 +7849,8 @@ object Main {
     10 + 0
   )
 }
-<<< binPack.defnSite with configStyleArguments, !danglingParentheses
-optIn.configStyleArguments = true
+<<< binPack.defnSite with configStyle, !danglingParentheses
+newlines.configStyleDefnSite.prefer = true
 danglingParentheses.defnSite = false
 binPack.unsafeDefnSite = always
 maxColumn = 30
@@ -7896,8 +7896,8 @@ object Main {
       x3: X, x4: X, xs: X*)
       : Set[Int]
 }
-<<< binPack.callSite with !configStyleArguments, !danglingParentheses
-optIn.configStyleArguments = false
+<<< binPack.callSite with !configStyle, !danglingParentheses
+newlines.configStyleCallSite.prefer = false
 danglingParentheses.callSite = false
 binPack.unsafeCallSite = always
 maxColumn = 30
@@ -7934,8 +7934,8 @@ object Main {
   val bar2 = foo2(0, 1, 2, 3,
     4, 5, 6, 7, 8, 9, 10 + 0)
 }
-<<< binPack.defnSite with !configStyleArguments, !danglingParentheses
-optIn.configStyleArguments = false
+<<< binPack.defnSite with !configStyle, !danglingParentheses
+newlines.configStyleDefnSite.prefer = false
 danglingParentheses.defnSite = false
 binPack.unsafeDefnSite = always
 maxColumn = 30