From b3bd9ce1d6c249cf934a254406446e37c29e266c Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Thu, 6 Jul 2023 08:07:38 -0700 Subject: [PATCH 1/2] Add tests with opt-braces rewrite and if-cond --- .../test/resources/scala3/OptionalBraces.stat | 51 +++++++++++++++++ .../resources/scala3/OptionalBraces_fold.stat | 38 +++++++++++++ .../resources/scala3/OptionalBraces_keep.stat | 52 +++++++++++++++++ .../scala3/OptionalBraces_unfold.stat | 57 +++++++++++++++++++ 4 files changed, 198 insertions(+) diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat index 44516ecc56..cb77f191e5 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat @@ -876,6 +876,57 @@ 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 +>>> +Idempotency violated +private def mtd: Res = +- if bar && ++ 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 + } +>>> +Idempotency violated +private def mtd: Res = + if foo then fooBody +- else if bar then barBody +- else bazBody ++ 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 + } +>>> +Idempotency violated +private def mtd: Res = + if foo then fooBody +- else if bar then barBody +- else bazBody ++ else if bar then barBody else bazBody <<< rewrite with given-with rewrite.scala3.removeOptionalBraces = oldSyntaxToo === diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat index 21404d9937..49e0d9cee0 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat @@ -821,6 +821,44 @@ 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 +>>> +Idempotency violated +-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 === diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat index 33d44345e5..b8bafe41a1 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat @@ -875,6 +875,58 @@ 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 + } +>>> +Idempotency violated +private def mtd: Res = + if foo then fooBody +- else if bar then +- barBody +- else bazBody ++ 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 + } +>>> +Idempotency violated +private def mtd: Res = + if foo then fooBody +- else if bar then barBody +- else bazBody ++ else if bar then barBody else bazBody <<< rewrite with given-with rewrite.scala3.removeOptionalBraces = oldSyntaxToo === diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat index deec05e20f..dfd2289a5e 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat @@ -919,6 +919,63 @@ 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 +>>> +Idempotency violated +private def mtd: Res = +- if bar && baz then ++ if ++ bar && baz ++ then + 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 + } +>>> +Idempotency violated +private def mtd: Res = + if foo then + fooBody +- else if bar then +- barBody + else +- bazBody ++ 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 === From be106be1a4f61627555198351852afd754c45fd1 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Thu, 6 Jul 2023 18:27:41 -0700 Subject: [PATCH 2/2] FormatOps: handle if with rewritten braces around --- .../org/scalafmt/internal/FormatOps.scala | 18 ++++++++++++--- .../scala/org/scalafmt/internal/Router.scala | 9 +++++++- .../test/resources/scala3/OptionalBraces.stat | 21 +++++------------- .../resources/scala3/OptionalBraces_fold.stat | 7 +----- .../resources/scala3/OptionalBraces_keep.stat | 17 +++++--------- .../scala3/OptionalBraces_unfold.stat | 22 +++++-------------- 6 files changed, 41 insertions(+), 53 deletions(-) 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 cde3c404ff..d5f1ed6603 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 @@ -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 @@ -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) @@ -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 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 fe0fc8b839..b2f038596e 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 @@ -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 diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat index cb77f191e5..fc48c6db93 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat @@ -886,11 +886,8 @@ private def mtd: Res = } then foo >>> -Idempotency violated private def mtd: Res = -- if bar && -+ if -+ bar && + if bar && baz then foo <<< remove optional braces within else-if @@ -904,14 +901,10 @@ private def mtd: Res = else bazBody } >>> -Idempotency violated private def mtd: Res = - if foo then fooBody -- else if bar then barBody -- else bazBody -+ else -+ if bar then barBody -+ else bazBody + if foo then fooBody + else if bar then barBody + else bazBody <<< remove optional braces within else-if 2 rewrite.scala3.removeOptionalBraces = yes === @@ -921,12 +914,10 @@ private def mtd: Res = if bar then barBody else bazBody } >>> -Idempotency violated private def mtd: Res = if foo then fooBody -- else if bar then barBody -- else bazBody -+ else if bar then barBody else bazBody + else if bar then barBody + else bazBody <<< rewrite with given-with rewrite.scala3.removeOptionalBraces = oldSyntaxToo === diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat index 49e0d9cee0..7328b1b9b0 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat @@ -831,12 +831,7 @@ private def mtd: Res = } then foo >>> -Idempotency violated --private def mtd: Res = if bar && baz then foo -+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 === diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat index b8bafe41a1..8d2c6d05f2 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat @@ -902,16 +902,11 @@ private def mtd: Res = else bazBody } >>> -Idempotency violated private def mtd: Res = if foo then fooBody -- else if bar then -- barBody -- else bazBody -+ else -+ if bar then -+ barBody -+ else bazBody + else if bar then + barBody + else bazBody <<< remove optional braces within else-if 2 rewrite.scala3.removeOptionalBraces = yes === @@ -921,12 +916,10 @@ private def mtd: Res = if bar then barBody else bazBody } >>> -Idempotency violated private def mtd: Res = if foo then fooBody -- else if bar then barBody -- else bazBody -+ else if bar then barBody else bazBody + else if bar then barBody + else bazBody <<< rewrite with given-with rewrite.scala3.removeOptionalBraces = oldSyntaxToo === diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat index dfd2289a5e..9d71b216db 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat @@ -929,14 +929,9 @@ private def mtd: Res = } then foo >>> -Idempotency violated private def mtd: Res = -- if bar && baz then -+ if -+ bar && baz -+ then - baz - then foo + if bar && baz then + foo <<< remove optional braces within else-if rewrite.scala3.removeOptionalBraces = yes === @@ -948,18 +943,13 @@ private def mtd: Res = else bazBody } >>> -Idempotency violated private def mtd: Res = if foo then fooBody -- else if bar then -- barBody - else -- bazBody -+ if bar then -+ barBody -+ else -+ bazBody + else if bar then + barBody + else + bazBody <<< remove optional braces within else-if 2 rewrite.scala3.removeOptionalBraces = yes ===