From aae2c3b0a63074ea2abe41170de2bd11516f83a1 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Tue, 3 Oct 2023 13:20:07 -0700 Subject: [PATCH] FormatOps: fix Defn as optional braces block --- .../org/scalafmt/internal/FormatOps.scala | 20 +++++++++---------- .../scala/org/scalafmt/util/TreeOps.scala | 11 ++++++++++ .../test/resources/scala3/OptionalBraces.stat | 14 +++++-------- .../resources/scala3/OptionalBraces_fold.stat | 14 +++++-------- .../resources/scala3/OptionalBraces_keep.stat | 8 ++------ .../scala3/OptionalBraces_unfold.stat | 8 ++------ 6 files changed, 35 insertions(+), 40 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 1efbfd471c..010b1b00e4 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 @@ -2174,7 +2174,7 @@ class FormatOps( def rightBrace = blockLast(t.body) }) case t: Term.If if !nft.right.is[T.KwThen] && { - isTreeMultiStatBlock(t.thenp) || !ifWithoutElse(t) && + !isTreeSingleExpr(t.thenp) || !ifWithoutElse(t) && (isElsePWithOptionalBraces(t) || existsBlockIfWithoutElse(t.thenp, false)) } => @@ -2187,7 +2187,7 @@ class FormatOps( Some(new OptionalBracesRegion { def owner = Some(t) def splits = - if (!isTreeMultiStatBlock(b)) None + if (isTreeSingleExpr(b)) None else Some(getSplits(ft, b, true)) def rightBrace = blockLast(b) }) @@ -2195,7 +2195,7 @@ class FormatOps( Some(new OptionalBracesRegion { def owner = Some(t) def splits = - if (!isTreeMultiStatBlock(b)) None + if (isTreeSingleExpr(b)) None else Some(getSplits(ft, b, true)) def rightBrace = blockLast(b) }) @@ -2308,7 +2308,7 @@ class FormatOps( style: ScalafmtConfig ): Option[OptionalBracesRegion] = { def trySplits(expr: Term, finallyp: Option[Term], usesOB: => Boolean) = - if (isTreeMultiStatBlock(expr)) Some(getSplits(ft, expr, true)) + if (!isTreeSingleExpr(expr)) Some(getSplits(ft, expr, true)) else if (finallyp.exists(isTreeUsingOptionalBraces) || usesOB) Some(getSplits(ft, expr, shouldBreakInOptionalBraces(nft))) else None @@ -2357,7 +2357,7 @@ class FormatOps( ft.meta.leftOwner match { case t: Term.Try => t.finallyp.map { x => - val usesOB = isTreeMultiStatBlock(x) || + val usesOB = !isTreeSingleExpr(x) || isCatchUsingOptionalBraces(t) || isTreeUsingOptionalBraces(t.expr) new OptionalBracesRegion { @@ -2369,7 +2369,7 @@ class FormatOps( } case t: Term.TryWithHandler => t.finallyp.map { x => - val usesOB = isTreeMultiStatBlock(x) || + val usesOB = !isTreeSingleExpr(x) || isTreeUsingOptionalBraces(t.expr) new OptionalBracesRegion { def owner = Some(t) @@ -2460,7 +2460,7 @@ class FormatOps( case t: Term.If => (t.elsep match { case _: Term.If => None - case x if isTreeMultiStatBlock(x) => Some(true) + case x if !isTreeSingleExpr(x) => Some(true) case b @ Term.Block(List(_: Term.If)) if (matchingOpt(nft.right) match { case Some(t) => t.end < b.pos.end @@ -2544,7 +2544,7 @@ class FormatOps( case _: T.KwThen => true case _: T.LeftBrace => false case _ => - isTreeMultiStatBlock(thenp) && (!before.right.is[T.LeftBrace] || + !isTreeSingleExpr(thenp) && (!before.right.is[T.LeftBrace] || matchingOpt(before.right).exists(rb => rb.end < thenp.pos.end)) } } @@ -2559,7 +2559,7 @@ class FormatOps( case Term.Block(List(t: Term.If)) => isThenPWithOptionalBraces(t) || !ifWithoutElse(t) && isElsePWithOptionalBraces(t) - case t => isTreeMultiStatBlock(t) + case t => !isTreeSingleExpr(t) }) } @@ -2573,7 +2573,7 @@ class FormatOps( } private def isTreeUsingOptionalBraces(tree: Tree): Boolean = - isTreeMultiStatBlock(tree) && !tokenBefore(tree).left.is[T.LeftBrace] + !isTreeSingleExpr(tree) && !tokenBefore(tree).left.is[T.LeftBrace] private def isBlockStart(tree: Term.Block, ft: FormatToken): Boolean = tokens.tokenJustBeforeOpt(tree.stats).contains(ft) diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala index 04fd901d25..5b6ce054b8 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala @@ -569,6 +569,17 @@ object TreeOps { case _ => false } + @tailrec + def isTreeSingleExpr(tree: Tree): Boolean = tree match { + case t: Term.Block => + t.stats match { + case stat :: Nil => isTreeSingleExpr(stat) + case _ => false + } + case _: Defn => false + case _ => true + } + /* An end marker is really more like a closing brace for formatting purposes * (but not when rewriting) so we should ignore it when considering whether a * block contains only a single statement. NB: in FormatWriter, when choosing diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat index 44eda2a73e..9f6a9d06b8 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat @@ -5187,27 +5187,23 @@ object a: catch case e: Throwable => >>> -test does not parse object a: - try val x = 3 / 0 - ^ + try + val x = 3 / 0 + catch case e: Throwable => <<< #3653 if-!then object a: if (true) val x = 3 / 0 >>> -test does not parse object a: if (true) - val x = 3 / 0 - ^ + val x = 3 / 0 <<< #3653 for-!do object a: for (a <- b) val x = 3 / 0 >>> -test does not parse object a: for (a <- b) - val x = 3 / 0 - ^ + val x = 3 / 0 diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat index cb3bd2c320..0280669b67 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat @@ -4954,27 +4954,23 @@ object a: catch case e: Throwable => >>> -test does not parse object a: - try val x = 3 / 0 - ^ + try + val x = 3 / 0 + catch case e: Throwable => <<< #3653 if-!then object a: if (true) val x = 3 / 0 >>> -test does not parse object a: if (true) - val x = 3 / 0 - ^ + val x = 3 / 0 <<< #3653 for-!do object a: for (a <- b) val x = 3 / 0 >>> -test does not parse object a: for (a <- b) - val x = 3 / 0 - ^ + val x = 3 / 0 diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat index dc7a4ec74d..deefb7e4be 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat @@ -5232,18 +5232,14 @@ object a: if (true) val x = 3 / 0 >>> -test does not parse object a: if (true) - val x = 3 / 0 - ^ + val x = 3 / 0 <<< #3653 for-!do object a: for (a <- b) val x = 3 / 0 >>> -test does not parse object a: for (a <- b) - val x = 3 / 0 - ^ + val x = 3 / 0 diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat index 3f2ca0c6f7..8971b9ead3 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat @@ -5363,18 +5363,14 @@ object a: if (true) val x = 3 / 0 >>> -test does not parse object a: if (true) - val x = 3 / 0 - ^ + val x = 3 / 0 <<< #3653 for-!do object a: for (a <- b) val x = 3 / 0 >>> -test does not parse object a: for (a <- b) - val x = 3 / 0 - ^ + val x = 3 / 0