fix build server promise + other tests fixes

scalameta · Jul 17, 2024 · 48ccadd · 48ccadd
1 parent 00bbd5c
commit 48ccadd
Show file tree

Hide file tree

Showing 8 changed files with 34 additions and 29 deletions.
diff --git a/metals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scala
@@ -192,11 +192,11 @@ class ConnectionProvider(
     private object DisconnectProvider
         extends ConnectProvider[Boolean, Disconnect] {
       def run(shutdownBuildServer: Boolean): Future[BuildChange] = {
-        def shutdownBsp(): Future[Boolean] = {
-          bspSession match {
-            case Some(session) if session.main.isBloop =>
+        def shutdownBsp(optMainBsp: Option[String]): Future[Boolean] = {
+          optMainBsp match {
+            case Some(BloopServers.name) =>
               Future { bloopServers.shutdownServer() }
-            case Some(session) if session.main.isSbt =>
+            case Some(SbtBuildTool.name) =>
               for {
                 res <- buildToolProvider.buildTool match {
                   case Some(sbt: SbtBuildTool) =>
@@ -217,14 +217,16 @@ class ConnectionProvider(
 
         for {
           _ <- scalaCli.stop()
-          _ <- bspSession match {
-            case None => Future.successful(())
+          optMainBsp <- bspSession match {
+            case None => Future.successful(None)
             case Some(session) =>
               bspSession = None
               mainBuildTargetsData.resetConnections(List.empty)
-              session.shutdown()
+              session.shutdown().map(_ => Some(session.main.name))
           }
-          _ <- if (shutdownBuildServer) shutdownBsp() else Future.successful(())
+          _ <-
+            if (shutdownBuildServer) shutdownBsp(optMainBsp)
+            else Future.successful(())
         } yield BuildChange.None
       }
     }
@@ -405,6 +407,8 @@ class ConnectionProvider(
 
       def run(config: (BuildServerProvider, Boolean)): Future[BuildChange] = {
         val (buildTool, shutdownServer) = config
+        tables.buildTool.chooseBuildTool(buildTool.executableName)
+        maybeChooseServer(buildTool.buildServerName, alreadySelected = false)
         for {
           _ <-
             if (shutdownServer) DisconnectProvider.run(shutdownServer)
@@ -415,8 +419,10 @@ class ConnectionProvider(
               args => bspConfigGenerator.runUnconditionally(buildTool, args),
               statusBar,
             )
-          _ = handleGenerationStatus(buildTool, status)
-          status <- CreateSessionProvider.run(false)
+          shouldConnect = handleGenerationStatus(buildTool, status)
+          status <-
+            if (shouldConnect) CreateSessionProvider.run(false)
+            else Future.successful(BuildChange.Failed)
         } yield status
       }
 
@@ -500,10 +506,7 @@ class ConnectionProvider(
                     Future.successful(BuildChange.Failed)
                   }
               } yield change
-            } else {
-              Future.successful(BuildChange.None)
-            }
-
+            } else Future.successful(BuildChange.None)
           }
         } yield change
       }
@@ -566,7 +569,7 @@ class ConnectionProvider(
         maybeChooseServer(buildTool.buildServerName, isSelected(buildTool))
         connect(CreateSession())
       // Used in tests, `.bloop` folder exists but no build tool is detected
-      case _ => connect(CreateSession())
+      case _ => quickConnectToBuildServer()
     }
   }
 
@@ -583,8 +586,7 @@ class ConnectionProvider(
       userConfig().shouldAutoImportNewProject || forceImport || isSelected ||
       buildTool.isInstanceOf[ScalaCliBuildTool]
     ) {
-      maybeChooseServer(buildTool.buildServerName, isSelected)
-      connect(CreateSession())
+      connect(GenerateBspConfigAndConnect(buildTool))
     } else if (notification.isDismissed) {
       Future.successful(BuildChange.None)
     } else {
@@ -600,8 +602,7 @@ class ConnectionProvider(
             notification.dismissForever()
             Future.successful(BuildChange.None)
           } else if (item == Messages.GenerateBspAndConnect.yes) {
-            maybeChooseServer(buildTool.buildServerName, isSelected)
-            connect(CreateSession())
+            connect(GenerateBspConfigAndConnect(buildTool))
           } else {
             notification.dismiss(2, TimeUnit.MINUTES)
             Future.successful(BuildChange.None)

diff --git a/metals/src/main/scala/scala/meta/internal/metals/FallbackMetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/FallbackMetalsLspService.scala
@@ -6,6 +6,7 @@ import java.util.concurrent.atomic.AtomicReference
 
 import scala.concurrent.ExecutionContextExecutorService
 import scala.concurrent.Future
+import scala.concurrent.Promise
 
 import scala.meta.internal.builds.ShellRunner
 import scala.meta.internal.metals.Indexer.BuildTool
@@ -57,7 +58,7 @@ class FallbackMetalsLspService(
       maxScalaCliServers = 10,
     ) {
 
-  buildServerPromise.success(())
+  val buildServerPromise: Promise[Unit] = Promise.successful(())
   indexingPromise.success(())
 
   private val files: AtomicReference[Set[AbsolutePath]] = new AtomicReference(

diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala
@@ -216,7 +216,7 @@ abstract class MetalsLspService(
     () => userConfig,
   )
   var indexingPromise: Promise[Unit] = Promise[Unit]()
-  var buildServerPromise: Promise[Unit] = Promise[Unit]()
+  def buildServerPromise: Promise[Unit]
   val parseTrees = new BatchedFunction[AbsolutePath, Unit](
     paths =>
       CancelableFuture(

diff --git a/metals/src/main/scala/scala/meta/internal/metals/ProjectMetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/ProjectMetalsLspService.scala
@@ -85,6 +85,7 @@ class ProjectMetalsLspService(
   )
 
   override def indexer: Indexer = connectionProvider
+  def buildServerPromise = connectionProvider.buildServerPromise
   def connect[T](config: ConnectConfig[T]): Future[BuildChange] =
     connectionProvider.ConnectProvider.connect(config)
 

diff --git a/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala b/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala
@@ -107,7 +107,7 @@ class BazelLspSuite
       }
       _ <- server.didSave(s"BUILD")(identity)
       _ = assertNoDiff(client.workspaceMessageRequests, "")
-      _ = server.server.buildServerPromise = Promise()
+      _ = server.headServer.connectionProvider.buildServerPromise = Promise()
       _ <- server.executeCommand(ServerCommands.GenerateBspConfig)
       // We need to wait a bit just to ensure the connection is made
       _ <- server.server.buildServerPromise.future
@@ -160,7 +160,7 @@ class BazelLspSuite
       }
       _ <- server.didSave(s"BUILD")(identity)
       _ = assertNoDiff(client.workspaceMessageRequests, "")
-      _ = server.server.buildServerPromise = Promise()
+      _ = server.headServer.connectionProvider.buildServerPromise = Promise()
       _ <- server.executeCommand(ServerCommands.ImportBuild)
       // We need to wait a bit just to ensure the connection is made
       _ <- server.server.buildServerPromise.future
@@ -182,7 +182,7 @@ class BazelLspSuite
            |  Test <- NOT SUPPORTED
            |  Compile""".stripMargin
       _ = assertNoDiff(result, expectedTarget)
-      _ = server.server.buildServerPromise = Promise()
+      _ = server.headServer.connectionProvider.buildServerPromise = Promise()
       _ = client.resetWorkspace =
         new MessageActionItem(Messages.ResetWorkspace.resetWorkspace)
       _ <- server.executeCommand(ServerCommands.ResetWorkspace)

diff --git a/tests/slow/src/test/scala/tests/mill/MillServerSuite.scala b/tests/slow/src/test/scala/tests/mill/MillServerSuite.scala
@@ -109,7 +109,7 @@ class MillServerSuite
         // below it won't wait until it reconnects to Mill like we want, so we set it back
         // and then it will be completed after the BSP config generation and the server
         // connects.
-        _ = server.server.buildServerPromise = Promise()
+        _ = server.headServer.connectionProvider.buildServerPromise = Promise()
         // At this point, we want to use mill-bsp server, so create the mill-bsp.json file.
         _ <- server.executeCommand(ServerCommands.GenerateBspConfig)
         // We need to wait a bit just to ensure the connection is made
@@ -158,7 +158,7 @@ class MillServerSuite
       )
       _ = client.messageRequests.clear() // restart
       _ = assert(!millBspConfig.exists)
-      _ = server.server.buildServerPromise = Promise()
+      _ = server.headServer.connectionProvider.buildServerPromise = Promise()
       _ <- server.executeCommand(ServerCommands.GenerateBspConfig)
       _ <- server.server.buildServerPromise.future
       _ = assert(millBspConfig.exists)

diff --git a/tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala b/tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala
@@ -295,7 +295,7 @@ class SbtServerSuite
             |""".stripMargin
       )
       _ = buffer.clear()
-      _ <- server.headServer.connect(CreateSession())
+      _ <- server.headServer.connect(CreateSession(true))
     } yield {
       val logs = buffer.result()
       assert(logs.contains("sbt server started"))

diff --git a/tests/unit/src/test/scala/tests/WorkspaceFoldersSuite.scala b/tests/unit/src/test/scala/tests/WorkspaceFoldersSuite.scala
@@ -81,6 +81,7 @@ class WorkspaceFoldersSuite
       _ = assert(server.fullServer.nonScalaProjects.length == 1)
       _ = writeLayout(
         s"""|$newScalaFile
+            |//> using scala ${V.scala213}
             |package a
             |object O {
             | val i: Int = "aaa"
@@ -92,8 +93,9 @@ class WorkspaceFoldersSuite
       _ = assert(server.fullServer.folderServices.length == 2)
       _ = assertNoDiff(
         server.client.pathDiagnostics(s"notYetScalaProject$newScalaFile"),
-        s"""|notYetScalaProject$newScalaFile:3:15: error: Found:    ("aaa" : String)
-            |Required: Int
+        s"""|notYetScalaProject/a/src/main/scala/A.scala:4:15: error: type mismatch;
+            | found   : String("aaa")
+            | required: Int
             | val i: Int = "aaa"
             |              ^^^^^
             |""".stripMargin,