From 760de95fbf0be61cd6e91616e2e680efb2d5e24d Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Thu, 11 Apr 2024 21:40:10 -0700 Subject: [PATCH] Router: fix scala.js style for binPack.callSite 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). --- docs/configuration.md | 24 +++++++++---------- .../org/scalafmt/internal/FormatOps.scala | 24 +++++++++++++++++++ .../scala/org/scalafmt/internal/Router.scala | 13 +++++++--- .../resources/newlines/source_classic.stat | 19 ++++++++------- 4 files changed, 57 insertions(+), 23 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 7fa5c29917..71ad889175 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -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, 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 index cabe230ffa..2cef53572a 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala @@ -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 { @@ -2727,6 +2750,7 @@ object FormatOps { dangleForTrailingCommas: Boolean, configStyle: ConfigStyle, shouldDangle: Boolean, + scalaJsStyle: Boolean, ) } 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 index 97453e2172..7249961a51 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala @@ -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) @@ -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 = @@ -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 = { @@ -1291,6 +1297,7 @@ class Router(formatOps: FormatOps) { styleMap.forcedBinPack(leftOwner) ) bothPolicies else configStylePolicy + else if (scalaJsStyleNL) configStylePolicy else if ( flags.dangleForTrailingCommas || flags.shouldDangle && diff --git a/scalafmt-tests/src/test/resources/newlines/source_classic.stat b/scalafmt-tests/src/test/resources/newlines/source_classic.stat index 72f4bccf30..3c2097841f 100644 --- a/scalafmt-tests/src/test/resources/newlines/source_classic.stat +++ b/scalafmt-tests/src/test/resources/newlines/source_classic.stat @@ -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 @@ -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,