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 and a dreaded "Unable to format due to bug" error.

Rather than clearing the queue, let's inject states into the priority
queue with a generation value and decrement it 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 scalameta#448. Fixes scalameta#1462. Fixes scalameta#1485. Fixes scalameta#1599. Fixes scalameta#1717.

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

@@ -139,12 +139,13 @@ class BestFirstSearch(
       depth: Int = 0,
       maxCost: Int = Integer.MAX_VALUE
   ): State = {
-    val Q = new mutable.PriorityQueue[State]()
+    val Q = new mutable.PriorityQueue[(Int, State)]()
     var result = start
-    Q += start
+    Q += 0 -> start
+
     // TODO(olafur) this while loop is waaaaaaaaaaaaay tooo big.
     while (Q.nonEmpty) {
-      val curr = Q.dequeue()
+      var (gen, curr) = Q.dequeue()
       explored += 1
       runner.event(Explored(explored, depth, Q.size))
       if (explored > runner.maxStateVisits)
@@ -171,13 +172,14 @@ class BestFirstSearch(
             dequeueOnNewStatements &&
             dequeueSpots.contains(tokenHash) &&
             (depth > 0 || !isInsideNoOptZone(splitToken)))
-            Q.clear()
+            if (depth == 0) gen += 1
+            else Q.clear()
         }
 
         val blockClose = shouldRecurseOnBlock(splitToken, stop, style)
         if (blockClose.nonEmpty)
           shortestPathMemo(curr, blockClose.get, depth + 1, maxCost)
-            .foreach(Q.enqueue(_))
+            .foreach(x => Q.enqueue(gen -> x))
         else if (escapeInPathologicalCases &&
           visits(curr.depth) > maxVisitsPerToken) {
           Q.clear()
@@ -219,17 +221,17 @@ class BestFirstSearch(
                 if (hasReachedEof(nextNextState, token)) {
                   optimalNotFound = false
 //                  logger.elem(split, splitToken, formatWriter.mkString(nextNextState.splits), tokens(nextNextState.splits.length))
-                  Q.enqueue(nextNextState)
+                  Q.enqueue(gen -> nextNextState)
                 } else if (!killOnFail &&
                   nextState.cost - curr.cost <= maxCost) {
                   // TODO(olafur) DRY. This solution can still be optimal.
 //                  logger.elem(split, splitToken)
-                  Q.enqueue(nextState)
+                  Q.enqueue(gen -> nextState)
                 } // else kill branch
               case _
                   if optimalNotFound &&
                     nextState.cost - curr.cost <= maxCost =>
-                Q.enqueue(nextState)
+                Q.enqueue(gen -> nextState)
               case _ => // Kill branch.
             }
           }

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)
+  }
+
+}