BestFirstSearch: don't give up too soon

Currently, the algorithm would clear the search priority queue when the
top candidate points to a new statement. Sometimes this leads to losing
a viable path and the dreaded "Unable to format due to bug" error.

Rather than clearing the queue, let's maintain several generations of
priority queues and create a new generation on a new statement, thus
continuing to process subsequent states just like the old algorithm did
but with the previously discarded states available as backup.

Apply this modification to top-level invocations only, so that nested
calls will still terminate early; thus the backup candidates are really
processed at the very end of the existing logic.

Fixes #448. Fixes #1462. Fixes #1485. Fixes #1599. Fixes #1717.

scalafmt-core/shared/src/main/scala/org/scalafmt/internal/BestFirstSearch.scala

@@ -133,11 +133,18 @@ private class BestFirstSearch private (
       depth: Int = 0,
       maxCost: Int = Integer.MAX_VALUE
   ): State = {
-    val Q = new mutable.PriorityQueue[State]()
-    var result: State = null
+    def newGeneration = new mutable.PriorityQueue[State]()
+    var Q = newGeneration
+    var generations: List[mutable.PriorityQueue[State]] = Nil
+    def addGeneration =
+      if (Q.nonEmpty) {
+        generations = Q :: generations
+        Q = newGeneration
+      }
     Q += start
+
     // TODO(olafur) this while loop is waaaaaaaaaaaaay tooo big.
-    while (Q.nonEmpty) {
+    while (true) {
       val curr = Q.dequeue()
       explored += 1
       runner.event(Explored(explored, depth, Q.size))
@@ -152,9 +159,10 @@ private class BestFirstSearch private (
       if (null == splitToken ||
         splitToken.left.start >= stop.start &&
         splitToken.left.start < splitToken.left.end) {
-        result = curr
-        Q.clear()
-      } else if (shouldEnterState(curr)) {
+        return curr
+      }
+
+      if (shouldEnterState(curr)) {
         val style = styleMap.at(splitToken)
         if (curr.depth > deepestYet.depth) {
           deepestYet = curr
@@ -168,7 +176,8 @@ private class BestFirstSearch private (
             dequeueOnNewStatements &&
             dequeueSpots.contains(tokenHash) &&
             (depth > 0 || !isInsideNoOptZone(splitToken)))
-            Q.clear()
+            if (depth == 0) addGeneration
+            else Q.clear()
         }
 
         val blockClose = shouldRecurseOnBlock(splitToken, stop, style)
@@ -230,8 +239,18 @@ private class BestFirstSearch private (
           }
         }
       }
+
+      if (Q.isEmpty) {
+        if (generations.isEmpty)
+          return null
+
+        Q = generations.head
+        generations = generations.tail
+      }
     }
-    result
+
+    // unreachable
+    null
   }
 
   def getBestPath: SearchResult = {

scalafmt-tests/src/test/resources/default/Lambda.stat

@@ -398,7 +398,16 @@ object Foo {
   }
 }
 >>>
-Unable to format file due to bug in scalafmt
+object Foo {
+  code.collect {
+    case d: Defn.Def
+        if (d match {
+          case _ => false
+
+        }) &&
+          d =>
+  }
+}
 <<< #1485
 style = IntelliJ
 maxColumn = 120
@@ -422,4 +431,17 @@ class A {
       .f("f")
 }
 >>>
-Unable to format file due to bug in scalafmt
+class A {
+  def foo: B =
+    a.b() { c =>
+        val d = c.add(D[E](1, {
+          case conditionNameShouldLongEnough =>
+            foo1(objectNameShouldLongEnough, objectNameShouldLongEnough)
+            0
+          case conditionNameShouldLongEnough =>
+            foo1(objectNameShouldLongEnough, objectNameShouldLongEnough)
+            1
+        }))
+      }
+      .f("f")
+}

scalafmt-tests/src/test/resources/newlines/afterCurlyLambdaNever.stat

@@ -56,4 +56,18 @@ class Bar {
   }
 }
 >>>
-Unable to format file due to bug in scalafmt
+class Bar {
+  def foo(request: Request): RpcResult[Response] = {
+    repository
+      .flatMap { campaigns =>
+        request match {
+          case Nil => connection.pure(Right(toResponse(campaigns, Map.empty)))
+          case _ =>
+            getProfileEmails(request.securityContext, profileIds).to[ConnectionIO].map {
+              profileEmails => Right(toResponse(campaigns, profileEmails))
+            }
+        }
+      }
+      .transact(managerTransactor)
+  }
+}

scalafmt-tests/src/test/resources/newlines/beforeConfigSingleArgParenLambdaParamsF_danglingT.stat

@@ -714,7 +714,23 @@ object A {
   }
 }
 >>>
-Unable to format file due to bug in scalafmt
+object A {
+  def foo() = {
+    x.map(
+      _.y(
+        abcd,
+        { case ((abcdefghij, aswbasdfaw), asfda) => aswdf }, {
+          case (abcdefghij, sadfasdass) =>
+            (
+              asdfa.sadvfs(abcdefghij).get,
+              asdfasdfasfdasda.asdfas(asdfasdaas).get
+            )
+        },
+        foo
+      )
+    )
+  }
+}
 <<< #1599 2: modified, with partial func inside block
 object A {
   def foo() = {
@@ -727,7 +743,25 @@ object A {
   }
 }
 >>>
-Unable to format file due to bug in scalafmt
+object A {
+  def foo() = {
+    x.map(
+      _.y(
+        abcd,
+        { { case ((abcdefghij, aswbasdfaw), asfda) => aswdf } }, {
+          {
+            case (abcdefghij, sadfasdass) =>
+              (
+                asdfa.sadvfs(abcdefghij).get,
+                asdfasdfasfdasda.asdfas(asdfasdaas).get
+              )
+          }
+        },
+        foo
+      )
+    )
+  }
+}
 <<< #1599 3: modified, with lambda
 object A {
   def foo() = {
@@ -740,4 +774,20 @@ object A {
   }
 }
 >>>
-Unable to format file due to bug in scalafmt
+object A {
+  def foo() = {
+    x.map(
+      _.y(
+        abcd,
+        { case_abcdefghij_aswbasdfaw_asfda => aswdf }, {
+          case_abcdefghij_sadfasdass =>
+            (
+              asdfa.sadvfs(abcdefghij).get,
+              asdfasdfasfdasda.asdfas(asdfasdaas).get
+            )
+        },
+        foo
+      )
+    )
+  }
+}

scalafmt-tests/src/test/resources/vertical-multiline/VerticalMultilineDefnSite.stat

@@ -28,4 +28,14 @@ object Test {
 
 }
 >>>
-Unable to format file due to bug in scalafmt
+object Test {
+
+  def create(
+    somethingInfo: foo.T = Something("my-something", Some("test-something"), "something",
+      Some(Format("something", "something")), Some(something.Something("something",
+          "something", "something", "something")))
+  ): Unit = {
+    w(foo)
+  }
+
+}