Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FormatOps: expand config-style source detection #3981

Merged
merged 2 commits into from
May 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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`

Expand All @@ -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).

Expand Down Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
26 changes: 25 additions & 1 deletion scalafmt-tests/src/test/resources/default/For.stat
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,32 @@ 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,
Expand Down
10 changes: 6 additions & 4 deletions scalafmt-tests/src/test/resources/newlines/source_classic.stat
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 8 additions & 4 deletions scalafmt-tests/src/test/resources/newlines/source_keep.stat
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -391,17 +400,39 @@ class foo(
b,
)
}
<<< #3663 enclosed literal
<<< #3663 enclosed literal, close dangles
val x = ( "a",
)
>>>
val x = (
"a"
)
<<< #3663 enclosed literal, open dangles
val x = (
"a" )
>>>
val x = ("a")
<<< #3663 enclosed lambda
<<< #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, open dangles
val x = (
x => x + 1 )
>>>
val x = (x => x + 1)
<<< #3663 enclosed lambda 1
<<< #3663 enclosed lambda, both dangle
val x = (
x => x + 1,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -376,17 +385,39 @@ class foo(
a, b,
)
}
<<< #3663 enclosed literal
<<< #3663 enclosed literal, close dangles
val x = ( "a",
)
>>>
val x = (
"a"
)
<<< #3663 enclosed literal, open dangles
val x = (
"a" )
>>>
val x = ("a")
<<< #3663 enclosed lambda
<<< #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, open dangles
val x = (
x => x + 1 )
>>>
val x = (x => x + 1)
<<< #3663 enclosed lambda 1
<<< #3663 enclosed lambda, both dangle
val x = (
x => x + 1,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -112,6 +112,27 @@ def getListing(
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
}
<<< 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,
Expand Down
Loading