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.

Sign up for GitHub

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

Jump to bottom

FormatOps: handle if with rewritten braces around #3578

Merged
merged 2 commits into from
Jul 7, 2023
Merged

FormatOps: handle if with rewritten braces around #3578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3bd9ce
Add tests with opt-braces rewrite and if-cond
kitbellew Jul 6, 2023
be106be
FormatOps: handle if with rewritten braces around
kitbellew Jul 7, 2023
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
18 changes: 15 additions & 3 deletions scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,9 @@ class FormatOps(
}
term.elsep match {
case t: Term.If => getElseChain(t, newRes)
case b @ Term.Block(List(t: Term.If))
if !tokens.areMatching(ftElsep.left)(getLastToken(b)) =>
getElseChain(t, newRes)
case _ => newRes
}
case _ => res
Expand Down Expand Up @@ -2401,14 +2404,17 @@ class FormatOps(
): Option[OptionalBracesRegion] = {
ft.meta.leftOwner match {
case t: Term.If =>
val nr = nft.right
t.cond match {
case b: Term.Block
if !matchingOpt(nft.right).exists(_.end >= b.pos.end) =>
case b: Term.Block if (matchingOpt(nr) match {
case Some(t) => t.end < b.pos.end
case None => isMultiStatBlock(b)
}) =>
Some(new OptionalBracesRegion {
def owner = Some(t)
def splits = Some {
val dangle = style.danglingParentheses.ctrlSite
val forceNL = !nft.right.is[T.LeftParen]
val forceNL = !nr.is[T.LeftParen]
getSplits(ft, b, forceNL, dangle)
}
def rightBrace = blockLast(b)
Expand All @@ -2429,6 +2435,12 @@ class FormatOps(
(t.elsep match {
case _: Term.If => None
case x if isTreeMultiStatBlock(x) => Some(true)
case b @ Term.Block(List(_: Term.If))
if (matchingOpt(nft.right) match {
case Some(t) => t.end < b.pos.end
case None => true
}) =>
None
case _ if isThenPWithOptionalBraces(t) =>
Some(shouldBreakInOptionalBraces(nft))
case _ => None
Expand Down
9 changes: 8 additions & 1 deletion scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2173,7 +2173,14 @@ class Router(formatOps: FormatOps) {
Seq(Split(Newline, 0))
// Last else branch
case FormatToken(_: T.KwElse, _, _) if (leftOwner match {
case t: Term.If => !t.elsep.is[Term.If]
case t: Term.If =>
t.elsep match {
case _: Term.If => false
case b @ Term.Block(List(_: Term.If)) =>
matchingOpt(nextNonComment(formatToken).right)
.exists(_.end >= b.pos.end)
case _ => true
}
case x => throw new UnexpectedTree[Term.If](x)
}) =>
val body = leftOwner.asInstanceOf[Term.If].elsep
Expand Down
42 changes: 42 additions & 0 deletions scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,48 @@ object a:
stat4
end if
end cond
<<< remove optional braces within if-cond
rewrite.scala3.removeOptionalBraces = yes
===
private def mtd: Res =
if {
bar &&
baz
} then
foo
>>>
private def mtd: Res =
if bar &&
baz
then foo
<<< remove optional braces within else-if
rewrite.scala3.removeOptionalBraces = yes
===
private def mtd: Res =
if foo then fooBody
else {
if bar then
barBody
else bazBody
}
>>>
private def mtd: Res =
if foo then fooBody
else if bar then barBody
else bazBody
<<< remove optional braces within else-if 2
rewrite.scala3.removeOptionalBraces = yes
===
private def mtd: Res =
if foo then fooBody
else {
if bar then barBody else bazBody
}
>>>
private def mtd: Res =
if foo then fooBody
else if bar then barBody
else bazBody
<<< rewrite with given-with
rewrite.scala3.removeOptionalBraces = oldSyntaxToo
===
Expand Down
33 changes: 33 additions & 0 deletions scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,39 @@ object a:
stat4
end if
end cond
<<< remove optional braces within if-cond
rewrite.scala3.removeOptionalBraces = yes
===
private def mtd: Res =
if {
bar &&
baz
} then
foo
>>>
private def mtd: Res = if bar && baz then foo
<<< remove optional braces within else-if
rewrite.scala3.removeOptionalBraces = yes
===
private def mtd: Res =
if foo then fooBody
else {
if bar then
barBody
else bazBody
}
>>>
private def mtd: Res = if foo then fooBody else if bar then barBody else bazBody
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit terryfing 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, it's fold after all, and the line is exactly 80 wide :)

<<< remove optional braces within else-if 2
rewrite.scala3.removeOptionalBraces = yes
===
private def mtd: Res =
if foo then fooBody
else {
if bar then barBody else bazBody
}
>>>
private def mtd: Res = if foo then fooBody else if bar then barBody else bazBody
<<< rewrite with given-with
rewrite.scala3.removeOptionalBraces = oldSyntaxToo
===
Expand Down
45 changes: 45 additions & 0 deletions scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,51 @@ object a:
stat4
end if
end cond
<<< remove optional braces within if-cond
rewrite.scala3.removeOptionalBraces = yes
===
private def mtd: Res =
if {
bar &&
baz
} then
foo
>>>
private def mtd: Res =
if
bar &&
baz
then
foo
<<< remove optional braces within else-if
rewrite.scala3.removeOptionalBraces = yes
===
private def mtd: Res =
if foo then fooBody
else {
if bar then
barBody
else bazBody
}
>>>
private def mtd: Res =
if foo then fooBody
else if bar then
barBody
else bazBody
<<< remove optional braces within else-if 2
rewrite.scala3.removeOptionalBraces = yes
===
private def mtd: Res =
if foo then fooBody
else {
if bar then barBody else bazBody
}
>>>
private def mtd: Res =
if foo then fooBody
else if bar then barBody
else bazBody
<<< rewrite with given-with
rewrite.scala3.removeOptionalBraces = oldSyntaxToo
===
Expand Down
47 changes: 47 additions & 0 deletions scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,53 @@ object a:
stat4
end if
end cond
<<< remove optional braces within if-cond
rewrite.scala3.removeOptionalBraces = yes
===
private def mtd: Res =
if {
bar &&
baz
} then
foo
>>>
private def mtd: Res =
if bar && baz then
foo
<<< remove optional braces within else-if
rewrite.scala3.removeOptionalBraces = yes
===
private def mtd: Res =
if foo then fooBody
else {
if bar then
barBody
else bazBody
}
>>>
private def mtd: Res =
if foo then
fooBody
else if bar then
barBody
else
bazBody
<<< remove optional braces within else-if 2
rewrite.scala3.removeOptionalBraces = yes
===
private def mtd: Res =
if foo then fooBody
else {
if bar then barBody else bazBody
}
>>>
private def mtd: Res =
if foo then
fooBody
else if bar then
barBody
else
bazBody
<<< rewrite with given-with
rewrite.scala3.removeOptionalBraces = oldSyntaxToo
===
Expand Down
Loading