From c98289cc8edba1379d07065bb3a6e9207072d835 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Tue, 3 Oct 2023 09:10:26 -0700 Subject: [PATCH 1/2] Add tests with failing Defn as opt-braces block --- .../test/resources/scala3/OptionalBraces.stat | 31 ++++++++++++++++++ .../resources/scala3/OptionalBraces_fold.stat | 31 ++++++++++++++++++ .../resources/scala3/OptionalBraces_keep.stat | 32 +++++++++++++++++++ .../scala3/OptionalBraces_unfold.stat | 32 +++++++++++++++++++ 4 files changed, 126 insertions(+) diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat index 8d58c20420..44eda2a73e 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat @@ -5180,3 +5180,34 @@ object a: object a: List(1, 2, 3) map: x => x + 1 +<<< #3653 try +object a: + try + val x = 3 / 0 + catch + case e: Throwable => +>>> +test does not parse +object a: + try val x = 3 / 0 + ^ +<<< #3653 if-!then +object a: + if (true) + val x = 3 / 0 +>>> +test does not parse +object a: + if (true) + 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 + ^ diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat index 651e2d3347..cb3bd2c320 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat @@ -4947,3 +4947,34 @@ object a: object a: List(1, 2, 3) map: x => x + 1 +<<< #3653 try +object a: + try + val x = 3 / 0 + catch + case e: Throwable => +>>> +test does not parse +object a: + try val x = 3 / 0 + ^ +<<< #3653 if-!then +object a: + if (true) + val x = 3 / 0 +>>> +test does not parse +object a: + if (true) + 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 + ^ diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat index d6772e0e81..dc7a4ec74d 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat @@ -5215,3 +5215,35 @@ object a: object a: List(1, 2, 3) map: x => x + 1 +<<< #3653 try +object a: + try + val x = 3 / 0 + catch + case e: Throwable => +>>> +object a: + 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 + ^ +<<< #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 + ^ diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat index 270c1a630c..3f2ca0c6f7 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat @@ -5346,3 +5346,35 @@ object a: object a: List(1, 2, 3) map: x => x + 1 +<<< #3653 try +object a: + try + val x = 3 / 0 + catch + case e: Throwable => +>>> +object a: + 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 + ^ +<<< #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 + ^ From 416be5c61959949f620bacb5fb7cb054e0584f3c 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 2/2] 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