Skip to content

Commit

Permalink
Config-style: allow optimizing defn site clause
Browse files Browse the repository at this point in the history
Initially, we would only optimize around call-site argument clauses.
Now let's add defn-site parameter clauses as well.

Also, let's rename the optimizer parameters to clarify the fact that
they don't necessarily _force_ config-style formatting.
  • Loading branch information
kitbellew committed May 6, 2024
1 parent 607b814 commit f20aef7
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 69 deletions.
24 changes: 12 additions & 12 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,9 @@ top-level.
neverBeforeJsNative = true
sometimesBeforeColonInMethodReturnType = false
}
runner.optimizer {
forceConfigStyleMinSpan = 500
forceConfigStyleMinArgCount = 5
runner.optimizer.callSite {
minSpan = 500
minCount = 5
}
```

Expand Down Expand Up @@ -2822,7 +2822,7 @@ object a {
### Forcing config style

When `newlines.configStyleXxxSite.forceIfOptimized` is enabled and
[call-site clause optimization](#route-search-optimizations-call-site-clause)
[route search optimization](#route-search-optimizations-arg-or-param-clause)
is applicable to a clause, the formatter will force config-style formatting.

By default, takes the same value as
Expand All @@ -2831,8 +2831,7 @@ By default, takes the same value as
```scala mdoc:scalafmt
newlines.configStyleCallSite.forceIfOptimized = true
newlines.configStyleDefnSite.forceIfOptimized = false
runner.optimizer.forceConfigStyleMinSpan = 5
runner.optimizer.forceConfigStyleMinArgCount = 2
runner.optimizer.callSite { minSpan = 5, minCount = 2 }
maxColumn = 60
---
object a {
Expand Down Expand Up @@ -5357,24 +5356,25 @@ and this optimization is enabled when:
runner.optimizer.dequeueOnNewStatements
```

#### Route search optimizations: call-site clause
#### Route search optimizations: arg or param clause

Similar to statements above, we might create a new priority queue when we
encounter a long-enough call-site clause.
encounter a long-enough call-site argument or defn-site parameter clause.

```scala mdoc:defaults
runner.optimizer.forceConfigStyleMinSpan
runner.optimizer.forceConfigStyleMinArgCount
runner.optimizer.callSite
runner.optimizer.defnSite
```

This optimization is enabled when all these criteria are satisfied:

- `runner.optimizer.forceConfigStyleMinSpan`:
- `xxxSite.minSpan`:
must be non-negative, and the character distance between the matching
parentheses, excluding any whitespace, must exceed this value
(prior to v3.8.1, this parameter was called `forceConfigStyleOnOffset`)
- `runner.optimizer.forceConfigStyleMinArgCount`:
- `xxxSite.minCount`:
must be positive and may not exceed the number of arguments
(prior to v3.8.2, this parameter was called `forceConfigStyleMinCount`)

> These parameters cannot be [overridden within a file](#for-code-block)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,32 @@ case class ScalafmtOptimizer(
disableOptimizationsInsideSensitiveAreas: Boolean = true,
pruneSlowStates: Boolean = true,
recurseOnBlocks: Boolean = true,
@annotation.ExtraName("forceConfigStyleOnOffset")
forceConfigStyleMinSpan: Int = 150,
forceConfigStyleMinArgCount: Int = 2,
@annotation.ExtraName("forceConfigStyleOnOffset") @annotation.DeprecatedName(
"forceConfigStyleMinSpan",
"Use `callSite.minSpan` instead",
"3.8.2",
)
private val forceConfigStyleMinSpan: Int = 150,
@annotation.DeprecatedName(
"forceConfigStyleMinArgCount",
"Use `callSite.minCount` instead",
"3.8.2",
)
private val forceConfigStyleMinArgCount: Int = 2,
private val callSite: Option[ClauseElement] = None,
private val defnSite: ClauseElement = ClauseElement.default,
) {
def getCallSite: ClauseElement = ClauseElement(
def getCallSite: ClauseElement = callSite.getOrElse(ClauseElement(
minSpan = forceConfigStyleMinSpan,
minCount = forceConfigStyleMinArgCount,
)
))
def getDefnSite: ClauseElement = defnSite

private[config] def conservative: ScalafmtOptimizer =
private[config] def conservative: ScalafmtOptimizer = {
// The tests were not written in this style
copy(forceConfigStyleMinSpan = 500, forceConfigStyleMinArgCount = 5)
val cfg = ClauseElement(minSpan = 500, minCount = 5)
copy(callSite = Some(cfg), defnSite = cfg)
}

}

Expand All @@ -89,4 +103,12 @@ object ScalafmtOptimizer {
val isEnabled: Boolean = minSpan >= 0 && minCount > 0
}

private[config] object ClauseElement {
val default = ClauseElement()
implicit val surface: generic.Surface[ClauseElement] = generic
.deriveSurface[ClauseElement]
implicit val codec: ConfCodecEx[ClauseElement] = generic
.deriveCodecEx(default).noTypos
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -1033,12 +1033,13 @@ class FormatOps(

def getForceConfigStyle: (Set[Int], Set[Int]) = {
val callSite = initStyle.runner.optimizer.getCallSite
if (callSite.isEnabled) {
val defnSite = initStyle.runner.optimizer.getDefnSite
if (callSite.isEnabled || defnSite.isEnabled) {
val clearQueues = Set.newBuilder[Int]
val forces = Set.newBuilder[Int]
def process(clause: Member.SyntaxValuesClause, ftOpen: FormatToken)(
cfg: ScalafmtOptimizer.ClauseElement,
): Unit = matchingOpt(ftOpen.left).foreach { close =>
): Unit = if (cfg.isEnabled) matchingOpt(ftOpen.left).foreach { close =>
val values = clause.values
if (
values.lengthCompare(cfg.minCount) >= 0 &&
Expand All @@ -1052,10 +1053,13 @@ class FormatOps(
case ft @ FormatToken(_: T.LeftParen, _, m) => m.leftOwner match {
case t: Term.ArgClause if !t.parent.exists(_.is[Term.ApplyInfix]) =>
process(t, ft)(callSite)
case t: Term.ParamClause => process(t, ft)(defnSite)
case _ =>
}
case ft @ FormatToken(_: T.LeftBracket, _, m) => m.leftOwner match {
case t: Type.ArgClause => process(t, ft)(callSite)
case t: Type.ParamClause => process(t, ft)(defnSite)
case t: Type.FuncParamClause => process(t, ft)(defnSite)
case _ =>
}
case _ =>
Expand Down
30 changes: 16 additions & 14 deletions scalafmt-tests/src/test/resources/newlines/source_classic.stat
Original file line number Diff line number Diff line change
Expand Up @@ -7608,7 +7608,7 @@ newlines.configStyleCallSite.prefer = true
danglingParentheses.callSite = true
binPack.unsafeCallSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.callSite { minSpan = 20, minCount = 5 }
===
object Main {
val bar1 = foo1(
Expand Down Expand Up @@ -7660,7 +7660,7 @@ newlines.configStyleDefnSite.prefer = true
danglingParentheses.defnSite = true
binPack.unsafeDefnSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.defnSite { minSpan = 20, minCount = 5 }
===
object Main {
def foo1(
Expand Down Expand Up @@ -7698,16 +7698,17 @@ object Main {
x1: X, x2: X, x3: X,
x4: X, xs: X*
): Set[Int]
def foo3(x1: X, x2: X,
x3: X, x4: X, xs: X*)
: Set[Int]
def foo3(
x1: X, x2: X, x3: X,
x4: X, xs: X*
): Set[Int]
}
<<< binPack.callSite with !configStyle, danglingParentheses
newlines.configStyleCallSite.prefer = false
danglingParentheses.callSite = true
binPack.unsafeCallSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.callSite { minSpan = 20, minCount = 5 }
===
object Main {
val bar1 = foo1(
Expand Down Expand Up @@ -7751,7 +7752,7 @@ newlines.configStyleDefnSite.prefer = false
danglingParentheses.defnSite = true
binPack.unsafeDefnSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.defnSite { minSpan = 20, minCount = 5 }
===
object Main {
def foo1(
Expand Down Expand Up @@ -7802,7 +7803,7 @@ newlines.configStyleCallSite.prefer = true
danglingParentheses.callSite = false
binPack.unsafeCallSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.callSite { minSpan = 20, minCount = 5 }
===
object Main {
val bar1 = foo1(
Expand Down Expand Up @@ -7854,7 +7855,7 @@ newlines.configStyleDefnSite.prefer = true
danglingParentheses.defnSite = false
binPack.unsafeDefnSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.defnSite { minSpan = 20, minCount = 5 }
===
object Main {
def foo1(
Expand Down Expand Up @@ -7892,16 +7893,17 @@ object Main {
x1: X, x2: X, x3: X,
x4: X, xs: X*
): Set[Int]
def foo3(x1: X, x2: X,
x3: X, x4: X, xs: X*)
: Set[Int]
def foo3(
x1: X, x2: X, x3: X,
x4: X, xs: X*
): Set[Int]
}
<<< binPack.callSite with !configStyle, !danglingParentheses
newlines.configStyleCallSite.prefer = false
danglingParentheses.callSite = false
binPack.unsafeCallSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.callSite { minSpan = 20, minCount = 5 }
===
object Main {
val bar1 = foo1(
Expand Down Expand Up @@ -7939,7 +7941,7 @@ newlines.configStyleDefnSite.prefer = false
danglingParentheses.defnSite = false
binPack.unsafeDefnSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.defnSite { minSpan = 20, minCount = 5 }
===
object Main {
def foo1(
Expand Down
30 changes: 16 additions & 14 deletions scalafmt-tests/src/test/resources/newlines/source_fold.stat
Original file line number Diff line number Diff line change
Expand Up @@ -7138,7 +7138,7 @@ newlines.configStyleCallSite.prefer = true
danglingParentheses.callSite = true
binPack.unsafeCallSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.callSite { minSpan = 20, minCount = 5 }
===
object Main {
val bar1 = foo1(
Expand Down Expand Up @@ -7187,7 +7187,7 @@ newlines.configStyleDefnSite.prefer = true
danglingParentheses.defnSite = true
binPack.unsafeDefnSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.defnSite { minSpan = 20, minCount = 5 }
===
object Main {
def foo1(
Expand Down Expand Up @@ -7238,7 +7238,7 @@ newlines.configStyleCallSite.prefer = false
danglingParentheses.callSite = true
binPack.unsafeCallSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.callSite { minSpan = 20, minCount = 5 }
===
object Main {
val bar1 = foo1(
Expand Down Expand Up @@ -7278,7 +7278,7 @@ newlines.configStyleDefnSite.prefer = false
danglingParentheses.defnSite = true
binPack.unsafeDefnSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.defnSite { minSpan = 20, minCount = 5 }
===
object Main {
def foo1(
Expand Down Expand Up @@ -7329,7 +7329,7 @@ newlines.configStyleCallSite.prefer = true
danglingParentheses.callSite = false
binPack.unsafeCallSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.callSite { minSpan = 20, minCount = 5 }
===
object Main {
val bar1 = foo1(
Expand Down Expand Up @@ -7378,7 +7378,7 @@ newlines.configStyleDefnSite.prefer = true
danglingParentheses.defnSite = false
binPack.unsafeDefnSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.defnSite { minSpan = 20, minCount = 5 }
===
object Main {
def foo1(
Expand Down Expand Up @@ -7411,19 +7411,21 @@ object Main {
xs: X*): Set[Int]
def foo1(x1: X, x2: X,
xs: X*): Set[Int]
def foo2(x1: X, x2: X,
x3: X, x4: X, xs: X*)
: Set[Int]
def foo3(x1: X, x2: X,
x3: X, x4: X, xs: X*)
: Set[Int]
def foo2(
x1: X, x2: X, x3: X,
x4: X, xs: X*
): Set[Int]
def foo3(
x1: X, x2: X, x3: X,
x4: X, xs: X*
): Set[Int]
}
<<< binPack.callSite with !configStyle, !danglingParentheses
newlines.configStyleCallSite.prefer = false
danglingParentheses.callSite = false
binPack.unsafeCallSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.callSite { minSpan = 20, minCount = 5 }
===
object Main {
val bar1 = foo1(
Expand Down Expand Up @@ -7462,7 +7464,7 @@ newlines.configStyleDefnSite.prefer = false
danglingParentheses.defnSite = false
binPack.unsafeDefnSite = always
maxColumn = 30
runner.optimizer.forceConfigStyleMinSpan = 20
runner.optimizer.defnSite { minSpan = 20, minCount = 5 }
===
object Main {
def foo1(
Expand Down
Loading

0 comments on commit f20aef7

Please sign in to comment.