Skip to content

Commit

Permalink
BinPack: drop unsafe prefix in call/defn sites
Browse files Browse the repository at this point in the history
  • Loading branch information
kitbellew committed Apr 4, 2024
1 parent df5531d commit c37f981
Show file tree
Hide file tree
Showing 9 changed files with 168 additions and 164 deletions.
23 changes: 12 additions & 11 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ foo:
.qux
```

### Indent for `binPack.unsafeCallSite`
### Indent for `binPack.callSite`

Normally, even when binpacking, there's a new level of indentation added for
each opening parenthesis starting a nested argument clause (regardless whether
Expand All @@ -697,7 +697,7 @@ binPack.indentCallSiteOnce
With the parameter enabled:

```scala mdoc:scalafmt
binPack.unsafeCallSite = true
binPack.callSite = true
binPack.indentCallSiteOnce = true
indent.callSite = 2
maxColumn = 20
Expand All @@ -708,7 +708,7 @@ foo(bar1(baz1(qux1, qux2), baz2), bar2(baz3, baz4))
With the parameter disabled:

```scala mdoc:scalafmt
binPack.unsafeCallSite = true
binPack.callSite = true
binPack.indentCallSiteOnce = false
indent.callSite = 2
maxColumn = 20
Expand All @@ -729,7 +729,7 @@ binPack.indentCallSiteSingleArg
With the parameter enabled:

```scala mdoc:scalafmt
binPack.unsafeCallSite = true
binPack.callSite = true
binPack.indentCallSiteSingleArg = true
indent.callSite = 2
maxColumn = 20
Expand All @@ -751,7 +751,7 @@ foo(bar((_, _) =>
With the parameter disabled:

```scala mdoc:scalafmt
binPack.unsafeCallSite = true
binPack.callSite = true
binPack.indentCallSiteSingleArg = false
indent.callSite = 2
maxColumn = 20
Expand Down Expand Up @@ -1522,7 +1522,7 @@ This approach attempts to preserve line breaks in the input whenever possible.
> (or prohibit) breaks in specific places (and that includes their default
> values).
>
> For instance, default for [`binPack.unsafeCallSite`](#binpackunsafexxxsite)
> For instance, default for [`binPack.callSite`](#binpackxxxsite)
> will not allow multiple arguments per line in a multiline expression.
#### `newlines.source=fold,unfold`
Expand Down Expand Up @@ -4333,7 +4333,7 @@ fileOverride {
}
"glob:**/src/test/scala/**.scala" {
maxColumn = 120
binPack.unsafeCallSite = true
binPack.callSite = true
}
}
```
Expand Down Expand Up @@ -4841,10 +4841,11 @@ object A {
}
```

### `binPack.unsafeXxxSite`
### `binPack.xxxSite`

Controls binpacking around `defn` or `call` sites. The following parameter
values are supported since v3.0.0:
values are supported since v3.0.0 (this parameter was called `unsafeXxxSite`
up until v3.8.1):

- `Never` disables the functionality (also takes `false`)
- `Always` enables the functionality (also takes `true`)
Expand All @@ -4854,14 +4855,14 @@ values are supported since v3.0.0:
> if [`optIn.configStyleArguments`](#optinconfigstylearguments) is set since binpacking
> and listing each argument/parameter on a separate line are at odds.
> Please also see [callSite indentation parameters](#indent-for-binpackunsafecallsite).
> Please also see [callSite indentation parameters](#indent-for-binpackcallsite).
### `binPack.bracketXxxSite`

> Since v3.0.4.
If set explicitly, will be used for type arguments or parameters,
instead of the respective [`binPack.unsafeXxxSite`](#binpackunsafexxxsite).
instead of the respective [`binPack.xxxSite`](#binpackxxxsite).

### binpacking of `importSelectors`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import scala.meta.tokens.Token

import metaconfig._

/** @param unsafeCallSite
/** @param callSite
* If true, will fit as many arguments on each line, only breaking at commas.
* If false, a function call's arguments will either be all on the same line
* or will have one line each.
* @param unsafeDefnSite
* Same as [[unsafeCallSite]], except for definition site.
* @param defnSite
* Same as [[callSite]], except for definition site.
* @param literalArgumentLists
* If true, automatically sets the style to bin-pack for argument lists that
* only consist of literals.
Expand All @@ -26,10 +26,12 @@ import metaconfig._
* If "Never", each parent constructor gets its own line.
*/
case class BinPack(
unsafeCallSite: BinPack.Unsafe = BinPack.Unsafe.Never,
unsafeDefnSite: BinPack.Unsafe = BinPack.Unsafe.Never,
private val bracketCallSite: Option[BinPack.Unsafe] = None,
private val bracketDefnSite: Option[BinPack.Unsafe] = None,
@annotation.ExtraName("unsafeCallSite")
callSite: BinPack.Site = BinPack.Site.Never,
@annotation.ExtraName("unsafeDefnSite")
defnSite: BinPack.Site = BinPack.Site.Never,
private val bracketCallSite: Option[BinPack.Site] = None,
private val bracketDefnSite: Option[BinPack.Site] = None,
indentCallSiteOnce: Boolean = false,
indentCallSiteSingleArg: Boolean = true,
parentConstructors: BinPack.ParentCtors = BinPack.ParentCtors.source,
Expand All @@ -49,16 +51,16 @@ case class BinPack(
parentConstructors.eq(BinPack.ParentCtors.source)

@inline
def callSite(open: Token): BinPack.Unsafe =
callSite(open.is[Token.LeftBracket])
def callSite(isBracket: Boolean): BinPack.Unsafe =
(if (isBracket) bracketCallSite else None).getOrElse(unsafeCallSite)
def callSiteFor(open: Token): BinPack.Site =
callSiteFor(open.is[Token.LeftBracket])
def callSiteFor(isBracket: Boolean): BinPack.Site =
(if (isBracket) bracketCallSite else None).getOrElse(callSite)

@inline
def defnSite(open: Token): BinPack.Unsafe =
defnSite(open.is[Token.LeftBracket])
def defnSite(isBracket: Boolean): BinPack.Unsafe =
(if (isBracket) bracketDefnSite else None).getOrElse(unsafeDefnSite)
def defnSiteFor(open: Token): BinPack.Site =
defnSiteFor(open.is[Token.LeftBracket])
def defnSiteFor(isBracket: Boolean): BinPack.Site =
(if (isBracket) bracketDefnSite else None).getOrElse(defnSite)

}

Expand All @@ -68,8 +70,8 @@ object BinPack {
implicit lazy val encoder: ConfEncoder[BinPack] = generic.deriveEncoder

val enabled = BinPack(
unsafeDefnSite = Unsafe.Always,
unsafeCallSite = Unsafe.Always,
defnSite = Site.Always,
callSite = Site.Always,
parentConstructors = ParentCtors.Always,
)

Expand Down Expand Up @@ -103,16 +105,14 @@ object BinPack {

}

sealed abstract class Unsafe {
final def isNever: Boolean = this eq Unsafe.Never
}
object Unsafe {
case object Never extends Unsafe
case object Always extends Unsafe
case object Oneline extends Unsafe
sealed abstract class Site
object Site {
case object Never extends Site
case object Always extends Site
case object Oneline extends Site

implicit val oneOfReader: ConfCodecEx[Unsafe] = ReaderUtil
.oneOfCustom[Unsafe](Never, Always, Oneline) {
implicit val oneOfReader: ConfCodecEx[Site] = ReaderUtil
.oneOfCustom[Site](Never, Always, Oneline) {
case Conf.Bool(true) => Configured.ok(Always)
case Conf.Bool(false) => Configured.ok(Never)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,8 @@ object ScalafmtConfig {
*/
val scalaJs: ScalafmtConfig = default.copy(
binPack = BinPack(
unsafeDefnSite = BinPack.Unsafe.Always,
unsafeCallSite = BinPack.Unsafe.Always,
defnSite = BinPack.Site.Always,
callSite = BinPack.Site.Always,
parentConstructors = BinPack.ParentCtors.Always,
),
danglingParentheses = DanglingParentheses(false, false),
Expand Down Expand Up @@ -403,8 +403,10 @@ object ScalafmtConfig {
}
if (!dialect.allowSignificantIndentation) addIf(newlines.beforeOpenParenCallSite.nonEmpty, errDialect)
addIfDirect( // can't use addIf on multiline conditions
!(binPack.unsafeCallSite.isNever && binPack.unsafeDefnSite.isNever) && { newlines.implicitParamListModifierForce.nonEmpty || newlines.implicitParamListModifierPrefer.nonEmpty },
"binPack.unsafeXXXSite && newlines.implicitParamListModifierXXX (not implemented)",
!(binPack.callSite == BinPack.Site.Never && binPack.defnSite == BinPack.Site.Never) && {
newlines.implicitParamListModifierForce.nonEmpty || newlines.implicitParamListModifierPrefer.nonEmpty
},
"binPack.xxxSite && newlines.implicitParamListModifierXXX (not implemented)",
)
checkPositive(indent.main, indent.callSite, indent.defnSite, indent.commaSiteRelativeToExtends)
checkNonNeg(indent.caseSite, indent.extendSite, indent.withSiteRelativeToExtends)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -839,8 +839,9 @@ class Router(formatOps: FormatOps) {
Seq(Split(NoSplit.orNL(noNL), 0))

case tok @ FormatToken(open @ LeftParenOrBracket(), right, _) if {
if (isArgClauseSite(leftOwner)) style.binPack.callSite(open).isNever
else style.binPack.defnSite(open).isNever &&
if (isArgClauseSite(leftOwner)) style.binPack.callSiteFor(open) ==
BinPack.Site.Never
else style.binPack.defnSiteFor(open) == BinPack.Site.Never &&
isParamClauseSite(leftOwner)
} =>
val close = matching(open)
Expand Down Expand Up @@ -1095,7 +1096,7 @@ class Router(formatOps: FormatOps) {
splitsNoNL ++ splitsNL ++ splitsForAssign.getOrElse(Seq.empty)

case ft @ FormatToken(open @ LeftParenOrBracket(), right, _)
if !style.binPack.defnSite(open).isNever &&
if style.binPack.defnSiteFor(open) != BinPack.Site.Never &&
isParamClauseSite(leftOwner) =>
val close = matching(open)
def slbPolicy = SingleLineBlock(close, okSLC = true, noSyntaxNL = true)
Expand All @@ -1118,8 +1119,8 @@ class Router(formatOps: FormatOps) {

val argsHeadOpt = argumentStarts.get(hash(right))
val isSingleArg = isSeqSingle(getArgs(leftOwner))
val oneline = style.binPack.defnSite(isBracket) ==
BinPack.Unsafe.Oneline
val oneline = style.binPack.defnSiteFor(isBracket) eq
BinPack.Site.Oneline
val nlOnelinePolicy = argsHeadOpt.flatMap { x =>
if (isSingleArg || !oneline) None
else findFirstOnRight[T.Comma](tokens.getLast(x), close)
Expand All @@ -1136,8 +1137,8 @@ class Router(formatOps: FormatOps) {
val penalizeOpens = bracketPenalty.fold(Policy.noPolicy) { p =>
Policy.before(close) {
case Decision(ftd @ FormatToken(o: T.LeftBracket, _, m), s)
if isParamClauseSite(m.leftOwner) &&
!styleMap.at(o).binPack.defnSite(o).isNever =>
if isParamClauseSite(m.leftOwner) && styleMap.at(o)
.binPack.defnSiteFor(o) != BinPack.Site.Never =>
if (tokens.isRightCommentThenBreak(ftd)) s
else s.map(x => if (x.isNL) x.withPenalty(p) else x)
}
Expand Down Expand Up @@ -1171,7 +1172,7 @@ class Router(formatOps: FormatOps) {
}

case ft @ FormatToken(open @ LeftParenOrBracket(), right, _)
if !style.binPack.callSite(open).isNever &&
if style.binPack.callSiteFor(open) != BinPack.Site.Never &&
isArgClauseSite(leftOwner) =>
val close = matching(open)
val beforeClose = tokens.justBefore(close)
Expand Down Expand Up @@ -1201,7 +1202,7 @@ class Router(formatOps: FormatOps) {

def findComma(ft: FormatToken) = findFirstOnRight[T.Comma](ft, close)

val oneline = style.binPack.callSite(open) == BinPack.Unsafe.Oneline
val oneline = style.binPack.callSiteFor(open) == BinPack.Site.Oneline
val nextCommaOneline =
if (!oneline || isSingleArg) None
else firstArg.map(tokens.getLast).flatMap(findComma)
Expand Down Expand Up @@ -1365,7 +1366,7 @@ class Router(formatOps: FormatOps) {
if !style.poorMansTrailingCommasInConfigStyle &&
isArgClauseSite(leftOwner) =>
val close = matching(open)
val binPackIsEnabled = !style.binPack.unsafeCallSite.isNever
val binPackIsEnabled = style.binPack.callSite != BinPack.Site.Never
val useSpace = !style.newlines.keepBreak(newlines)
val singleSplit =
if (!binPackIsEnabled) Split(Space.orNL(useSpace), 0)
Expand Down Expand Up @@ -1438,15 +1439,15 @@ class Router(formatOps: FormatOps) {
case FormatToken(_: T.Comma, right, _) if leftOwner.isNot[Template] =>
val splitsOpt = argumentStarts.get(hash(right)).flatMap { nextArg =>
val callSite = isArgClauseSite(leftOwner)
val binPackOpt =
if (callSite) Some(style.binPack.unsafeCallSite)
else if (isParamClauseSite(leftOwner))
Some(style.binPack.unsafeDefnSite)
else None
binPackOpt.filter(!_.isNever).map { binPack =>
val binPack =
if (callSite) style.binPack.callSite
else if (isParamClauseSite(leftOwner)) style.binPack.defnSite
else BinPack.Site.Never
if (binPack eq BinPack.Site.Never) None
else Some {
val lastFT = tokens.getLast(nextArg)
val loEnd = leftOwner.tokens.last.end
val oneline = binPack == BinPack.Unsafe.Oneline
val oneline = binPack == BinPack.Site.Oneline
val nextCommaOrParen = findFirst(lastFT, loEnd) {
case FormatToken(_, _: T.Comma, _) => true
case FormatToken(_, RightParenOrBracket(), _) => true
Expand Down Expand Up @@ -2249,7 +2250,7 @@ class Router(formatOps: FormatOps) {
}

case FormatToken(soft.ImplicitOrUsing(), _, ImplicitUsingOnLeft(params))
if style.binPack.unsafeDefnSite.isNever &&
if style.binPack.defnSite == BinPack.Site.Never &&
!style.verticalMultiline.atDefnSite =>
val spaceSplit = Split(Space, 0)
.notIf(style.newlines.forceAfterImplicitParamListModifier).withPolicy(
Expand Down Expand Up @@ -2520,9 +2521,9 @@ class Router(formatOps: FormatOps) {
}

val penalty = ft.meta.leftOwner match {
case _: Term.Assign if !style.binPack.unsafeCallSite.isNever =>
case _: Term.Assign if style.binPack.callSite != BinPack.Site.Never =>
Constants.BinPackAssignmentPenalty
case _: Term.Param if !style.binPack.unsafeDefnSite.isNever =>
case _: Term.Param if style.binPack.defnSite != BinPack.Site.Never =>
Constants.BinPackAssignmentPenalty
case _ => 0
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class StyleMap(tokens: FormatTokens, val init: ScalafmtConfig) {
val styleBuilder = Array.newBuilder[ScalafmtConfig]
startBuilder += 0
styleBuilder += init
val disableBinPack = mutable.Map.empty[Token, BinPack.Unsafe]
val disableBinPack = mutable.Map.empty[Token, BinPack.Site]
def warn(err: String)(implicit fileLine: FileLine): Unit = logger.elem(err)
tokens.arr.foreach { tok =>
def changeStyle(style: ScalafmtConfig): Option[ScalafmtConfig] = {
Expand Down Expand Up @@ -58,10 +58,10 @@ class StyleMap(tokens: FormatTokens, val init: ScalafmtConfig) {
if curr.binPack.literalArgumentLists &&
opensLiteralArgumentList(tok)(curr) =>
forcedBinPack += tok.meta.leftOwner
changeStyle(setBinPack(curr, callSite = BinPack.Unsafe.Always))
changeStyle(setBinPack(curr, callSite = BinPack.Site.Always))
.foreach { x =>
val unsafe = x.binPack.unsafeCallSite
tokens.matchingOpt(open).foreach(disableBinPack.update(_, unsafe))
tokens.matchingOpt(open)
.foreach(disableBinPack.update(_, x.binPack.callSite))
}
case close @ RightParen() => disableBinPack.remove(close).foreach { x =>
changeStyle(setBinPack(curr, callSite = x))
Expand Down Expand Up @@ -150,8 +150,8 @@ class StyleMap(tokens: FormatTokens, val init: ScalafmtConfig) {

object StyleMap {

def setBinPack(curr: ScalafmtConfig, callSite: BinPack.Unsafe): ScalafmtConfig =
if (curr.binPack.unsafeCallSite == callSite) curr
else curr.copy(binPack = curr.binPack.copy(unsafeCallSite = callSite))
def setBinPack(curr: ScalafmtConfig, callSite: BinPack.Site): ScalafmtConfig =
if (curr.binPack.callSite == callSite) curr
else curr.copy(binPack = curr.binPack.copy(callSite = callSite))

}
Loading

0 comments on commit c37f981

Please sign in to comment.