FormatOps: fix Defn as optional braces block

scalameta · Oct 4, 2023 · aae2c3b · aae2c3b
1 parent c9097e2
commit aae2c3b
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 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
@@ -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,15 +2187,15 @@ 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)
             })
           case t @ Term.While(_, b) if !nft.right.is[T.KwDo] =>
             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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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