Router: consistent if-else block format for fold

scalameta · Nov 22, 2024 · 3a391d9 · 3a391d9
1 parent 2f91f3e
commit 3a391d9
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 64 deletions.
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
@@ -313,25 +313,6 @@ class Router(formatOps: FormatOps) {
         val lambdaExpire =
           if (lambdaArrow eq null) null else nextNonCommentSameLine(lambdaArrow)
 
-        def getSingleLinePolicy = {
-          val needBreak = close.right match {
-            case _: T.KwElse => close.rightOwner match {
-                case p: Term.If => p.thenp eq leftOwner
-                case _ => false
-              }
-            case _: T.KwCatch => close.rightOwner match {
-                case p: Term.TryClause => p.expr eq leftOwner
-                case _ => false
-              }
-            case _: T.KwFinally => close.rightOwner match {
-                case p: Term.TryClause => (p.expr eq leftOwner) ||
-                  p.catchClause.contains(leftOwner)
-                case _ => false
-              }
-            case _ => false
-          }
-          Policy ? needBreak && decideNewlinesOnlyAfterClose(close)
-        }
         def getClassicSingleLineDecisionOpt =
           if (noBreak()) Some(true) else None
 
@@ -379,11 +360,28 @@ class Router(formatOps: FormatOps) {
         val singleLineSplitOpt = {
           if (slbParensExclude eq null) None else singleLineDecisionOpt
         }.map { sld =>
-          val sldPolicy = getSingleLinePolicy
+          val ownerIfNeedBreakAfterClose = close.right match {
+            case _: T.KwElse => close.rightOwner match {
+                case p: Term.If if p.thenp eq leftOwner => Some(p)
+                case _ => None
+              }
+            case _: T.KwCatch => close.rightOwner match {
+                case p: Term.TryClause if p.expr eq leftOwner => Some(p)
+                case _ => None
+              }
+            case _: T.KwFinally => close.rightOwner match {
+                case p: Term.TryClause
+                    if (p.expr eq leftOwner) ||
+                      p.catchClause.contains(leftOwner) => Some(p)
+                case _ => None
+              }
+            case _ => None
+          }
           val expire = leftOwner.parent match {
             case Some(p: Term.ForYield)
-                if !sld && sldPolicy.isEmpty && style.newlines.fold &&
-                  leftOwner.is[Term.EnumeratorsBlock] => getLast(p)
+                if !sld && ownerIfNeedBreakAfterClose.isEmpty &&
+                  style.newlines.fold && leftOwner.is[Term.EnumeratorsBlock] =>
+              getLast(p)
             case _ if style.newlines.isBeforeOpenParenCallSite => close
             case _ => getSlbEndOnLeft(close)
           }
@@ -402,6 +400,22 @@ class Router(formatOps: FormatOps) {
             else Policy.RelayOnSplit((s, _) => s.isNL)(slbParensPolicy)(
               Policy.onLeft(close, "BracesToParensFailed") { case _ => Nil },
             )
+          val sldPolicy = ownerIfNeedBreakAfterClose.map { p =>
+            if (style.newlines.fold) {
+              val pend = getSlbEndOnLeft(getLast(p))
+              def pendSlb(s: Split) = s
+                .withSingleLine(pend, noSyntaxNL = true, extend = true)
+              Policy.End <= close ==>
+                Policy.onRight(close, s"RB-ELSE[${pend.idx}]") {
+                  case Decision(`close`, ss) =>
+                    if (ss.exists(_.isNL)) ss
+                      .map(s => if (s.isNL) s else pendSlb(s))
+                    else ss.flatMap { s =>
+                      Seq(pendSlb(s), s.withMod(Newline).withPenalty(1))
+                    }
+                }
+            } else decideNewlinesOnlyAfterClose(close)
+          }
           Split(slbMod, 0).withSingleLine(
             expire,
             exclude = exclude,

diff --git a/scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat b/scalafmt-tests/shared/src/test/resources/newlines/source_fold.stat
@@ -16,8 +16,7 @@ println("bbb") } else c
 }
 >>>
 object a {
-  if (a) { println("bbb") }
-  else c
+  if (a) { println("bbb") } else c
 }
 <<< 1.3: block, if, non-egyptian curlies
 object a {
@@ -31,8 +30,7 @@ c
 }}
 >>>
 object a {
-  if (a) { println("bbb") }
-  else { c }
+  if (a) { println("bbb") } else { c }
 }
 <<< 1.4: block #1043
 object a {
@@ -252,12 +250,8 @@ object a {
 >>>
 object a {
   val ok1 = if (a > 10) Some(a) else None
-  val ok2 =
-    if (a > 10) { Some(a) }
-    else { None }
-  val ok3 =
-    if (aaaa > 10000) { Some(aaaa) }
-    else { None }
+  val ok2 = if (a > 10) { Some(a) } else { None }
+  val ok3 = if (aaaa > 10000) { Some(aaaa) } else { None }
 }
 <<< 2.7 #1747: one line without else
 maxColumn = 60
@@ -296,12 +290,8 @@ object a {
 >>>
 object a {
   val ok1 = if (a > 10) Some(a) else None
-  val ok2 =
-    if (a > 10) { Some(a) }
-    else { None }
-  val ok3 =
-    if (aaaa > 10000) { Some(aaaa) }
-    else { None }
+  val ok2 = if (a > 10) { Some(a) } else { None }
+  val ok3 = if (aaaa > 10000) { Some(aaaa) } else { None }
 }
 <<< 2.9 #1747: split on else
 maxColumn = 60
@@ -319,12 +309,8 @@ object a {
 >>>
 object a {
   val ok1 = if (a > 10) Some(a) else None
-  val ok2 =
-    if (a > 10) { Some(a) }
-    else { None }
-  val ok3 =
-    if (aaaa > 10000) { Some(aaaa) }
-    else { None }
+  val ok2 = if (a > 10) { Some(a) } else { None }
+  val ok3 = if (aaaa > 10000) { Some(aaaa) } else { None }
 }
 <<< 2.10 #1747: split on then without else
 maxColumn = 60
@@ -357,12 +343,8 @@ object a {
 >>>
 object a {
   val ok1 = if (a > 10) Some(a) else None
-  val ok2 =
-    if (a > 10) { Some(a) }
-    else { None }
-  val ok3 =
-    if (aaaa > 10000) { Some(aaaa) }
-    else { None }
+  val ok2 = if (a > 10) { Some(a) } else { None }
+  val ok3 = if (aaaa > 10000) { Some(aaaa) } else { None }
 }
 <<< 2.12 #1747: split on if without else
 maxColumn = 60
@@ -6008,8 +5990,7 @@ class Foo {
 >>>
 class Foo {
   val foo =
-    if (true) { "" }
-    else { "a" }
+    if (true) { "" } else { "a" }
   val foo = if (true) "" else "a"
   val foo = if (true) "" else "a"
   val foo =
@@ -6035,8 +6016,7 @@ class Foo() {
 >>>
 class Foo() {
   def ok: Boolean =
-    if (1 == 1) { true }
-    else false
+    if (1 == 1) { true } else false
 
   def notOK: Boolean =
     if (1 == 1) {
@@ -10510,8 +10490,7 @@ object a {
 }
 >>>
 object a {
-  if (s.costWithPenalty > 0) { preFork = false; false }
-  else true
+  if (s.costWithPenalty > 0) { preFork = false; false } else true
 
   if (s.costWithPenalty <= 0) true else { preFork = false; false }
 }
diff --git a/scalafmt-tests/shared/src/test/resources/scala3/OptionalBraces_fold.stat b/scalafmt-tests/shared/src/test/resources/scala3/OptionalBraces_fold.stat
@@ -3606,8 +3606,7 @@ object a {
 }
 >>>
 object a {
-  if ({ !Yield(2); false }) { !Yield(3) }
-  else { !Yield(4) }
+  if ({ !Yield(2); false }) { !Yield(3) } else { !Yield(4) }
 }
 <<< #3005 if-then
 object a {
@@ -3620,8 +3619,7 @@ object a {
 }
 >>>
 object a {
-  if ({ !Yield(2); false }) then { !Yield(3) }
-  else { !Yield(4) }
+  if ({ !Yield(2); false }) then { !Yield(3) } else { !Yield(4) }
 }
 <<< #3005 while
 object a {
@@ -4107,9 +4105,7 @@ class Foo() {
 }
 >>>
 class Foo() {
-  def ok: Boolean =
-    if (1 == 1) { true }
-    else false
+  def ok: Boolean = if (1 == 1) { true } else false
 
   def notOK: Boolean =
     if (1 == 1) {

diff --git a/scalafmt-tests/shared/src/test/scala/org/scalafmt/FormatTests.scala b/scalafmt-tests/shared/src/test/scala/org/scalafmt/FormatTests.scala
@@ -144,7 +144,7 @@ class FormatTests extends FunSuite with CanRunTests with FormatAssertions {
     val explored = Debug.explored.get()
     logger.debug(s"Total explored: $explored")
     if (!onlyUnit && !onlyManual)
-      assertEquals(explored, 1086266, "total explored")
+      assertEquals(explored, 1085954, "total explored")
     val results = debugResults.result()
     // TODO(olafur) don't block printing out test results.
     // I don't want to deal with scalaz's Tasks :'(