From 811e2132d7588350ec0734320787ff6671ef801a Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Sun, 12 May 2024 09:45:34 -0700 Subject: [PATCH 1/2] Tests enclosed expressions and dangling on and off --- .../src/test/resources/default/For.stat | 25 ++++++++++++++++++- .../trailing-commas/trailingCommasAlways.stat | 24 +++++++++++++++--- .../trailingCommasMultiple.stat | 24 +++++++++++++++--- .../excludeDanglingInDef.stat | 22 +++++++++++++++- 4 files changed, 87 insertions(+), 8 deletions(-) diff --git a/scalafmt-tests/src/test/resources/default/For.stat b/scalafmt-tests/src/test/resources/default/For.stat index e475ab25b1..da5ee3bab8 100644 --- a/scalafmt-tests/src/test/resources/default/For.stat +++ b/scalafmt-tests/src/test/resources/default/For.stat @@ -126,8 +126,31 @@ for ( SelectPath(looker.path descendant Seq("a", "b").asJava) -> None )) ) checkOne(looker, l, r) -<<< multiline for with paren, no align +<<< multiline for with paren, no align, !dangle align.preset = none +danglingParentheses.preset = false +=== +for ((l, r) ← /* c1 */ ( /* c2 */Seq( + SelectString("a/b/c") -> None, + SelectString("akka://all-systems/Nobody") -> None, + SelectPath(system / "hallo") -> None, + SelectPath(looker.path child "hallo") -> None, // test Java API + SelectPath(looker.path descendant Seq("a", "b").asJava) -> None + ) // test Java API + ) +) checkOne(looker, l, r) +>>> +for ((l, r) ← /* c1 */ ( /* c2 */ Seq( + SelectString("a/b/c") -> None, + SelectString("akka://all-systems/Nobody") -> None, + SelectPath(system / "hallo") -> None, + SelectPath(looker.path child "hallo") -> None, // test Java API + SelectPath(looker.path descendant Seq("a", "b").asJava) -> None + ) // test Java API + )) checkOne(looker, l, r) +<<< multiline for with paren, no align, dangle +align.preset = none +danglingParentheses.preset = true === for ((l, r) ← /* c1 */ ( /* c2 */Seq( SelectString("a/b/c") -> None, diff --git a/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat b/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat index 8dd9acbcc6..547e26157a 100644 --- a/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat +++ b/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat @@ -391,17 +391,35 @@ class foo( b, ) } -<<< #3663 enclosed literal +<<< #3663 enclosed literal, close dangles val x = ( "a", ) >>> val x = ("a") -<<< #3663 enclosed lambda +<<< #3663 enclosed literal, open dangles +val x = ( + "a" ) +>>> +val x = ("a") +<<< #3663 enclosed literal, both dangle +val x = ( + "a", +) +>>> +val x = ( + "a" +) +<<< #3663 enclosed lambda, close dangles val x = ( x => x + 1, ) >>> val x = (x => x + 1) -<<< #3663 enclosed lambda 1 +<<< #3663 enclosed lambda, open dangles +val x = ( + x => x + 1 ) +>>> +val x = (x => x + 1) +<<< #3663 enclosed lambda, both dangle val x = ( x => x + 1, ) diff --git a/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasMultiple.stat b/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasMultiple.stat index 4f926529fa..19b5d2cea4 100644 --- a/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasMultiple.stat +++ b/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasMultiple.stat @@ -376,17 +376,35 @@ class foo( a, b, ) } -<<< #3663 enclosed literal +<<< #3663 enclosed literal, close dangles val x = ( "a", ) >>> val x = ("a") -<<< #3663 enclosed lambda +<<< #3663 enclosed literal, open dangles +val x = ( + "a" ) +>>> +val x = ("a") +<<< #3663 enclosed literal, both dangle +val x = ( + "a", +) +>>> +val x = ( + "a" +) +<<< #3663 enclosed lambda, close dangles val x = ( x => x + 1, ) >>> val x = (x => x + 1) -<<< #3663 enclosed lambda 1 +<<< #3663 enclosed lambda, open dangles +val x = ( + x => x + 1 ) +>>> +val x = (x => x + 1) +<<< #3663 enclosed lambda, both dangle val x = ( x => x + 1, ) diff --git a/scalafmt-tests/src/test/resources/vertical-multiline/excludeDanglingInDef.stat b/scalafmt-tests/src/test/resources/vertical-multiline/excludeDanglingInDef.stat index 075c224ce5..c84ca257a7 100644 --- a/scalafmt-tests/src/test/resources/vertical-multiline/excludeDanglingInDef.stat +++ b/scalafmt-tests/src/test/resources/vertical-multiline/excludeDanglingInDef.stat @@ -98,7 +98,7 @@ def getListing(a: String, b: String, c: String, d: String)(implicit ): Future[String] = { a + b + c + d + e + g + f } -<<< with many implicits (non-vm, with exclude) +<<< with many implicits (non-vm, with exclude), with dangle in source verticalMultiline.atDefnSite = false danglingParentheses.preset = true danglingParentheses.exclude = [def] @@ -118,3 +118,23 @@ def getListing(a: String, b: String, c: String, d: String)(implicit f: String): Future[String] = { a + b + c + d + e + g + f } +<<< with many implicits (non-vm, with exclude), without dangle in source +verticalMultiline.atDefnSite = false +danglingParentheses.preset = true +danglingParentheses.exclude = [def] +=== +def getListing( + a: String, + b: String, + c: String, + d: String)(implicit e: String, g: String, + f: String): Future[String] = { + a + b + c + d + e + g + f +} +>>> +def getListing(a: String, b: String, c: String, d: String)(implicit + e: String, + g: String, + f: String): Future[String] = { + a + b + c + d + e + g + f +} From 0414b5c1639d3542999ddab2dc7914528d97bcee Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Sun, 12 May 2024 09:50:40 -0700 Subject: [PATCH 2/2] FormatOps: expand config-style source detection Here, in case dangling is off, rely only on close-delimiter breaks to detect desire for config-style formatting, scala.js-style. --- docs/configuration.md | 26 ++++++++++++------- .../org/scalafmt/internal/FormatOps.scala | 5 +++- .../scala/org/scalafmt/internal/Router.scala | 3 ++- .../src/test/resources/default/For.stat | 15 ++++++----- .../resources/newlines/source_classic.stat | 10 ++++--- .../test/resources/newlines/source_keep.stat | 12 ++++++--- .../trailing-commas/trailingCommasAlways.stat | 23 ++++++++++++---- .../trailingCommasMultiple.stat | 23 ++++++++++++---- .../excludeDanglingInDef.stat | 3 ++- 9 files changed, 82 insertions(+), 38 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index d132c2e0ec..557c1ab685 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -213,7 +213,7 @@ This preset is defined as This preset intends to approximate the [style used for `scala.js`](https://github.com/scala-js/scala-js/blob/main/CODINGSTYLE.md). -It uses modified detection of [config-style formatting](#newlines-config-style-formatting): +It uses modified detection of [config-style formatting](#newlinesconfigstylexxxsiteprefer): - [according to Sébastien Doeraene](https://github.com/scala-js/scala-js/pull/4522#issuecomment-879168123), config-style should be driven solely by presence of a dangling closing parenthesis @@ -2850,12 +2850,10 @@ other(a, b)(c, d) ## Newlines: Config-style formatting -This formatting applies to argument lists in class definitions and method calls. -It normally involves a newline after the opening parenthesis (or after the -`implicit` keyword) and a newline before the closing parenthesis. - -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). +This formatting applies to argument clauses in method calls or parameter +clauses in definitions. It outputs a newline after the opening parenthesis +(or after the `implicit` keyword) and a newline before the closing parenthesis, +with arguments or parameters listed one per line. ### `newlines.configStyleXxxSite.prefer` @@ -2866,8 +2864,16 @@ falls back to its value (which is enabled by default). If true, applies config-style formatting: -- if single-line formatting is impossible -- if `newlines.source = fold/unfold` or the source uses config-style +- `newlines.source = fold/unfold`: + if single-line formatting is impossible +- `newlines.source = classic/keep`: + if preference for config-style is detected in the source: + - there's a newline present before the closing delimiter + - if [`danglingParentheses.xxxSite = false`](#danglingparenthesescallsite) + (and [`align.openParenXxxSite = false`](#alignopenparencallsite)): + no other condition needs to be present (the [scala.js rule](#presetscalajs)) + - otherwise, there needs to be a newline after the opening delimiter, + or after the `implicit` keyword if present. Please note that other parameters might also force config-style (see below). @@ -4951,7 +4957,7 @@ and similarly has cross-parameter interactions: - for `newlines.source=classic`, behaviour depends on [config-style](#newlinesconfigstylexxxsiteprefer): - if enabled, config style is used if - - it is [detected](#newlines-config-style-formatting), or + - it is [detected](#newlinesconfigstylexxxsiteprefer), or - configured to use [scala.js style](#presetscalajs) - otherwise, uses binpacking - for other values of [`newlines.source`](#newlinessource), 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 efb6008dee..8686f32208 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 @@ -882,8 +882,11 @@ class FormatOps( couldPreserveConfigStyle(ft, breakBeforeClose) def couldPreserveConfigStyle(ft: FormatToken, breakBeforeClose: => Boolean)( - implicit style: ScalafmtConfig, + implicit + style: ScalafmtConfig, + clauseSiteFlags: ClauseSiteFlags, ): Boolean = !style.newlines.sourceIgnored && { + !clauseSiteFlags.dangleCloseDelim && !clauseSiteFlags.alignOpenDelim || ft.hasBreak || (next(ft).hasBreak || style.newlines.forceAfterImplicitParamListModifier) && opensConfigStyleImplicitParamList(ft) 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 84ca47aaae..5ef6fb1982 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 @@ -2023,6 +2023,7 @@ class Router(formatOps: FormatOps) { case FormatToken(open: T.LeftParen, right, _) => val close = matching(open) val beforeClose = tokens.justBefore(close) + implicit val clauseSiteFlags = ClauseSiteFlags.atCallSite(leftOwner) val isConfig = couldPreserveConfigStyle(ft, beforeClose.hasBreak) val enclosed = findEnclosedBetweenParens(open, close, leftOwner) @@ -2051,7 +2052,7 @@ class Router(formatOps: FormatOps) { def newlineSplit(cost: Int, forceDangle: Boolean)(implicit fileLine: FileLine, ) = { - val shouldDangle = forceDangle || style.danglingParentheses.callSite + val shouldDangle = forceDangle || clauseSiteFlags.dangleCloseDelim Split(Newline, cost) .withPolicy(decideNewlinesOnlyBeforeClose(close), !shouldDangle) .withIndent(style.indent.callSite, close, Before) diff --git a/scalafmt-tests/src/test/resources/default/For.stat b/scalafmt-tests/src/test/resources/default/For.stat index da5ee3bab8..d45909fff1 100644 --- a/scalafmt-tests/src/test/resources/default/For.stat +++ b/scalafmt-tests/src/test/resources/default/For.stat @@ -140,13 +140,14 @@ for ((l, r) ← /* c1 */ ( /* c2 */Seq( ) ) checkOne(looker, l, r) >>> -for ((l, r) ← /* c1 */ ( /* c2 */ Seq( - SelectString("a/b/c") -> None, - SelectString("akka://all-systems/Nobody") -> None, - SelectPath(system / "hallo") -> None, - SelectPath(looker.path child "hallo") -> None, // test Java API - SelectPath(looker.path descendant Seq("a", "b").asJava) -> None - ) // test Java API +for ((l, r) ← /* c1 */ ( + /* c2 */ Seq( + SelectString("a/b/c") -> None, + SelectString("akka://all-systems/Nobody") -> None, + SelectPath(system / "hallo") -> None, + SelectPath(looker.path child "hallo") -> None, // test Java API + SelectPath(looker.path descendant Seq("a", "b").asJava) -> None + ) // test Java API )) checkOne(looker, l, r) <<< multiline for with paren, no align, dangle align.preset = none diff --git a/scalafmt-tests/src/test/resources/newlines/source_classic.stat b/scalafmt-tests/src/test/resources/newlines/source_classic.stat index 75e759075b..c084c35f8b 100644 --- a/scalafmt-tests/src/test/resources/newlines/source_classic.stat +++ b/scalafmt-tests/src/test/resources/newlines/source_classic.stat @@ -2970,8 +2970,9 @@ object a { >>> object a { val foo = bar.map(x => - x.copy(baz = - Option.when(false)(x.qux))) + x.copy( + baz = Option.when(false)(x.qux) + )) } <<< literalsIncludeSimpleExpr with named parameter values, !configStyleArguments + danglingParentheses binPack.literalsIncludeSimpleExpr = true @@ -7936,8 +7937,9 @@ object Main { def foo1( x1: X, x2: X, xs: X* ): Set[Int] - def foo1(x1: X, x2: X, - xs: X*): Set[Int] + def foo1( + x1: X, x2: X, xs: X* + ): Set[Int] def foo1(x1: X, x2: X, xs: X*): Set[Int] def foo1(x1: X, x2: X, diff --git a/scalafmt-tests/src/test/resources/newlines/source_keep.stat b/scalafmt-tests/src/test/resources/newlines/source_keep.stat index 59ece1297d..6d2c697018 100644 --- a/scalafmt-tests/src/test/resources/newlines/source_keep.stat +++ b/scalafmt-tests/src/test/resources/newlines/source_keep.stat @@ -7785,8 +7785,10 @@ 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) @@ -7838,8 +7840,10 @@ object Main { x1: X, x2: X, xs: X* ): Set[Int] - def foo1(x1: X, - x2: X, xs: X*): Set[Int] + def foo1( + x1: X, + x2: X, xs: X* + ): Set[Int] def foo1( x1: X, x2: X, xs: X*): Set[Int] diff --git a/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat b/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat index 547e26157a..0f04168c60 100644 --- a/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat +++ b/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasAlways.stat @@ -195,9 +195,18 @@ class foo(a: String, } >>> import foo.{a, b} -class foo(a: String, b: String) { - def method(a: String, b: String) = ??? - method(a, b) +class foo( + a: String, + b: String, +) { + def method( + a: String, + b: String, + ) = ??? + method( + a, + b, + ) } <<< #2755 one, no comma maxColumn = 80 @@ -395,7 +404,9 @@ class foo( val x = ( "a", ) >>> -val x = ("a") +val x = ( + "a" +) <<< #3663 enclosed literal, open dangles val x = ( "a" ) @@ -413,7 +424,9 @@ val x = ( val x = ( x => x + 1, ) >>> -val x = (x => x + 1) +val x = ( + x => x + 1, +) <<< #3663 enclosed lambda, open dangles val x = ( x => x + 1 ) diff --git a/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasMultiple.stat b/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasMultiple.stat index 19b5d2cea4..138bb8e255 100644 --- a/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasMultiple.stat +++ b/scalafmt-tests/src/test/resources/trailing-commas/trailingCommasMultiple.stat @@ -195,9 +195,18 @@ class foo(a: String, } >>> import foo.{a, b} -class foo(a: String, b: String) { - def method(a: String, b: String) = ??? - method(a, b) +class foo( + a: String, + b: String, +) { + def method( + a: String, + b: String, + ) = ??? + method( + a, + b, + ) } <<< #2755 one, no comma maxColumn = 80 @@ -380,7 +389,9 @@ class foo( val x = ( "a", ) >>> -val x = ("a") +val x = ( + "a" +) <<< #3663 enclosed literal, open dangles val x = ( "a" ) @@ -398,7 +409,9 @@ val x = ( val x = ( x => x + 1, ) >>> -val x = (x => x + 1) +val x = ( + x => x + 1 +) <<< #3663 enclosed lambda, open dangles val x = ( x => x + 1 ) diff --git a/scalafmt-tests/src/test/resources/vertical-multiline/excludeDanglingInDef.stat b/scalafmt-tests/src/test/resources/vertical-multiline/excludeDanglingInDef.stat index c84ca257a7..7c68a8f68e 100644 --- a/scalafmt-tests/src/test/resources/vertical-multiline/excludeDanglingInDef.stat +++ b/scalafmt-tests/src/test/resources/vertical-multiline/excludeDanglingInDef.stat @@ -115,7 +115,8 @@ def getListing( def getListing(a: String, b: String, c: String, d: String)(implicit e: String, g: String, - f: String): Future[String] = { + f: String +): Future[String] = { a + b + c + d + e + g + f } <<< with many implicits (non-vm, with exclude), without dangle in source