Skip to content

Commit

Permalink
Router: fix scala.js style for binPack.callSite
Browse files Browse the repository at this point in the history
Previously, how we handle `binpack.callSite=true` depended only on
`optIn.configStyleArguments`, and `danglingParentheses.callSite` was
ignored completely.

Also, the declared `scala.js` style, which purported to depend on the
existence of a break before closing parenthesis, wasn't properly done.

Both opening and closing parentheses were output tucked in all cases
but one, when both of them were dangling in the input (and the choice
of whether to dangle or tuck depended solely on `configStyleArguments`).

Now let's introduce a small modification, so that we can express more
formatting alternatives, and fix `scala.js` style.

Specifically, associate the two variants above with dangling parentheses
in the disabled state.

When it's enabled, determine whether to tuck or dangle on break before
closing parenthesis. Also, apply config style when that parenthesis is
dangling and `configStyleArguments` is disabled (because in the enabled
state, it requires both parentheses to be dangling to be triggered).
  • Loading branch information
kitbellew committed Apr 14, 2024
1 parent 9778a5e commit 760de95
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 23 deletions.
24 changes: 12 additions & 12 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -4878,25 +4878,25 @@ 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`, formatting is mandated by the
[scala.js](https://github.com/scala-js/scala-js/pull/4522#issuecomment-879168123)
coding style, determined by the position of the closing parenthesis; "tucked"
parenthesis enables binpacking, while "dangling" one forces config-style
- for `newlines.source=classic`, behaviour depends on
[config-style](#optinconfigstylearguments):
- 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`
- `danglingParentheses.callSite` is ignored
- when `config-style` is enabled: open break is preserved, close break
matches open break
- otherwise: both open and close are "tucked"
- interaction with [`danglingParentheses.callSite`](#danglingparenthesescallsite)
- `newlines.source=classic`: please see above
- `newlines.source=keep`
- open break is preserved
- when both `config-style` and `danglingParentheses.callSite` are disabled,
- when both [config-style](#optinconfigstylearguments) and
[`danglingParentheses.callSite`](#danglingparenthesescallsite) are disabled,
close break is "tucked"
- otherwise, close break matches open break
- `newlines.source=fold/unfold`
- when `danglingParentheses.callSite` is enabled,
- 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2672,14 +2672,37 @@ class FormatOps(
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 &&
!literalArgList && shouldDangle
BinpackCallsiteFlags(
literalArgList = literalArgList,
dangleForTrailingCommas = dangleForTrailingCommas,
configStyle = configStyle,
shouldDangle = shouldDangle,
scalaJsStyle = scalaJsStyle,
)
}

@tailrec
final def scalaJsOptClose(
ftBeforeClose: FormatToken,
bpFlags: BinpackCallsiteFlags,
)(implicit style: ScalafmtConfig): T =
if (bpFlags.scalaJsStyle) {
val ftAfterClose = tokens.nextNonCommentAfter(ftBeforeClose)
val continue = ftAfterClose != ftBeforeClose &&
ftAfterClose.right.is[T.RightParen] && ftAfterClose.noBreak &&
isArgClauseSite(ftAfterClose.meta.rightOwner)
if (continue) {
val open = tokens.matching(ftAfterClose.right)
val styleAtOpen = styleMap.at(open)
val bpFlagsAfter =
getBinpackCallsiteFlags(tokens(open), ftAfterClose)(styleAtOpen)
scalaJsOptClose(ftAfterClose, bpFlagsAfter)(styleAtOpen)
} else ftBeforeClose.right
} else ftBeforeClose.right

}

object FormatOps {
Expand Down Expand Up @@ -2727,6 +2750,7 @@ object FormatOps {
dangleForTrailingCommas: Boolean,
configStyle: ConfigStyle,
shouldDangle: Boolean,
scalaJsStyle: Boolean,
)

}
Original file line number Diff line number Diff line change
Expand Up @@ -1182,9 +1182,10 @@ class Router(formatOps: FormatOps) {
flags.literalArgList

val rightIsComment = right.is[T.Comment]
val scalaJsStyleNL = flags.scalaJsStyle && beforeClose.hasBreak
val nlOnly = flags.dangleForTrailingCommas ||
flags.configStyle != ConfigStyle.None ||
style.newlines.keepBreak(newlines) ||
style.newlines.keepBreak(newlines) || scalaJsStyleNL ||
rightIsComment &&
(newlines != 0 || nextNonCommentSameLine(next(ft)).hasBreak)

Expand Down Expand Up @@ -1244,11 +1245,15 @@ class Router(formatOps: FormatOps) {
style.newlines.source.eq(Newlines.unfold)
) baseNoSplit.withSingleLine(close, noSyntaxNL = true)
else {
def optClose = Some(scalaJsOptClose(beforeClose, flags))
val opt =
if (oneline) nextCommaOneline.orElse(Some(close))
if (oneline) nextCommaOneline.orElse(optClose)
else if (style.newlines.source.eq(Newlines.fold)) None
else findComma(formatToken).orElse(Some(close))
else findComma(formatToken).orElse(optClose)
def unindentPolicy = unindentAtExclude(exclude, Num(-indentLen))
def scajaJsPolicy = Policy(Policy.End.On(close)) {
case d: Decision if d.formatToken.right eq close => d.noNewlines
}
val noSplitPolicy =
if (needOnelinePolicy) {
val alignPolicy =
Expand All @@ -1272,6 +1277,7 @@ class Router(formatOps: FormatOps) {
baseNoSplit.withOptimalTokenOpt(opt).withPolicy(noSplitPolicy)
.andPolicy(unindentPolicy, !isSingleArg || noSplitIndents.isEmpty)
.andPolicy(indentOncePolicy, noSplitIndents.isEmpty)
.andPolicy(scajaJsPolicy, !flags.scalaJsStyle)
}

val nlPolicy = {
Expand All @@ -1291,6 +1297,7 @@ class Router(formatOps: FormatOps) {
styleMap.forcedBinPack(leftOwner)
) bothPolicies
else configStylePolicy
else if (scalaJsStyleNL) configStylePolicy
else if (
flags.dangleForTrailingCommas ||
flags.shouldDangle &&
Expand Down
19 changes: 11 additions & 8 deletions scalafmt-tests/src/test/resources/newlines/source_classic.stat
Original file line number Diff line number Diff line change
Expand Up @@ -2801,10 +2801,7 @@ object a {
test("foo") {
a.b(c, d) shouldBe
E(Seq(F(1, "v1"), F(2, "v2")),
G(Some(Seq(h, i)),
Some(Seq(j, k)), a.b, c.d,
e.f.g, h.i.j)
).foo
G(Some(Seq(h, i)), Some(Seq(j, k)), a.b, c.d, e.f.g, h.i.j)).foo
}
}
<<< binpack call, oneline, with syntaxNL, single arg
Expand Down Expand Up @@ -7667,12 +7664,18 @@ object Main {
}
>>>
object Main {
val bar1 = foo1(
10000,
10001,
10002 + 0
)
val bar1 = foo1(10000,
10001, 10002 + 0)
val bar1 = foo1(10000,
10001, 10002 + 0)
val bar1 = foo1(10000,
10001, 10002 + 0)
val bar1 = foo1(
10000,
10001,
10002 + 0
)
val bar1 = foo1(10000,
10001, 10002 + 0)
val bar2 = foo2(0, 1, 2, 3,
Expand Down

0 comments on commit 760de95

Please sign in to comment.