improvement: Simplify slowConnectToBuildServer

scalameta · Mar 8, 2024 · 79d276c · 79d276c
1 parent 4bb870a
commit 79d276c
Show file tree

Hide file tree

Showing 8 changed files with 43 additions and 67 deletions.
diff --git a/metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala
@@ -22,8 +22,6 @@ trait BuildTool {
 
   def projectRoot: AbsolutePath
 
-  def isAutoConnectable: Boolean = false
-
   val forcesBuildServer = false
 
   val isBloopInstallProvider = false

diff --git a/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala b/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala
@@ -242,7 +242,7 @@ final class BuildTools(
       Some(BazelBuildTool.name)
     else if (isInBsp(path)) {
       val name = path.filename.stripSuffix(".json")
-      if (knownBsps(name)) None
+      if (knownBsps(name) && !ScalaCli.names(name)) None
       else Some(name)
     } else None
   }

diff --git a/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala
@@ -62,8 +62,6 @@ class ScalaCliBuildTool(
   override val forcesBuildServer = true
 
   override def possibleBuildServerNames = ScalaCli.names.toList
-
-  override def isAutoConnectable: Boolean = true
 }
 
 object ScalaCliBuildTool {

diff --git a/metals/src/main/scala/scala/meta/internal/builds/WorkspaceLoadedStatus.scala b/metals/src/main/scala/scala/meta/internal/builds/WorkspaceLoadedStatus.scala
@@ -12,7 +12,8 @@ sealed abstract class WorkspaceLoadedStatus extends Product with Serializable {
       case Duplicate(status) => status.toString
       case _ => this.toString
     }
-  def isInstalled: Boolean = this == Installed
+  def isInstalled: Boolean =
+    this == Installed || this == Duplicate(Status.Installed)
   def isFailed: Boolean = this.isInstanceOf[Failed]
   def toChecksumStatus: Option[Status] =
     Option(this).collect {

diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala
@@ -185,7 +185,8 @@ final class Compilations(
   ): Future[Option[b.BuildTargetIdentifier]] = {
     val isCompilable =
       (path.isScalaOrJava || path.isSbt) &&
-        !path.isDependencySource(workspace())
+        !path.isDependencySource(workspace()) &&
+        !path.isInTmpDirectory(workspace())
 
     if (isCompilable) {
       val targetOpt = buildTargets.inverseSourcesBsp(path)

diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala
@@ -381,9 +381,8 @@ object MetalsEnrichments
     }
 
     def isDependencySource(workspace: AbsolutePath): Boolean =
-      isLocalFileSystem(workspace) &&
-        (isInReadonlyDirectory(workspace) || isInTmpDirectory(workspace)) ||
-        isJarFileSystem
+      (isLocalFileSystem(workspace) &&
+        isInReadonlyDirectory(workspace)) || isJarFileSystem
 
     def isWorkspaceSource(workspace: AbsolutePath): Boolean =
       isLocalFileSystem(workspace) &&

diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala
@@ -905,19 +905,10 @@ class MetalsLspService(
   def fullConnect(): Future[Unit] = {
     buildTools.initialize()
     for {
-      found <- supportedBuildTool()
-      chosenBuildServer = found match {
-        case Some(BuildTool.Found(buildTool, _))
-            if buildTool.isAutoConnectable =>
-          tables.buildServers.chooseServer(buildTool.buildServerName)
-          Some(buildTool.buildServerName)
-        case _ => tables.buildServers.selectedServer()
-      }
-      _ <- slowConnectToBuildServer(
-        forceImport = false,
-        found,
-        chosenBuildServer,
-      ),
+      _ <-
+        if (buildTools.isAutoConnectable(optProjectRoot()))
+          autoConnectToBuildServer()
+        else slowConnectToBuildServer(forceImport = false)
     } yield buildServerPromise.trySuccess(())
   }
 
@@ -2056,73 +2047,58 @@ class MetalsLspService(
 
   def slowConnectToBuildServer(
       forceImport: Boolean
-  ): Future[BuildChange] = for {
-    buildTool <- supportedBuildTool()
-    chosenBuildServer = tables.buildServers.selectedServer()
-    buildChange <- slowConnectToBuildServer(
-      forceImport,
-      buildTool,
-      chosenBuildServer,
-    )
-  } yield {
-    buildServerPromise.trySuccess(())
-    buildChange
-  }
-
-  def slowConnectToBuildServer(
-      forceImport: Boolean,
-      buildTool: Option[BuildTool.Found],
-      chosenBuildServer: Option[String],
   ): Future[BuildChange] = {
-    def isBloopOrEmpty = chosenBuildServer.exists(
-      _ == BloopServers.name
-    ) || chosenBuildServer.isEmpty
-
-    def useBuildToolBsp(buildTool: BuildServerProvider) =
+    val chosenBuildServer = tables.buildServers.selectedServer()
+    def useBuildToolBsp(buildTool: BuildTool) =
       buildTool match {
         case _: BloopInstallProvider => userConfig.defaultBspToBuildTool
-        case _ => true
+        case _: BuildServerProvider => true
+        case _ => false
       }
-    buildTool match {
-      case Some(BuildTool.Found(buildTool: BuildServerProvider, _))
-          if chosenBuildServer.isEmpty && useBuildToolBsp(buildTool) =>
-        slowConnectToBuildToolBsp(buildTool, forceImport)
+
+    def isSelected(buildTool: BuildTool) =
+      buildTool match {
+        case _: BuildServerProvider =>
+          chosenBuildServer.contains(buildTool.buildServerName)
+        case _ => false
+      }
+
+    supportedBuildTool().flatMap {
       case Some(BuildTool.Found(buildTool: BloopInstallProvider, digest))
-          if isBloopOrEmpty =>
+          if chosenBuildServer.contains(BloopServers.name) ||
+            chosenBuildServer.isEmpty && !useBuildToolBsp(buildTool) =>
         slowConnectToBloopServer(forceImport, buildTool, digest)
-      case Some(BuildTool.Found(buildTool, _))
-          if !chosenBuildServer.contains(buildTool.buildServerName) &&
-            (buildTool.isAutoConnectable || forceImport) =>
-        tables.buildServers.chooseServer(buildTool.buildServerName)
-        quickConnectToBuildServer()
-      case Some(BuildTool.Found(buildTool: BuildServerProvider, _))
-          if chosenBuildServer.contains(buildTool.buildServerName) && !buildTool
-            .isBspGenerated(folder) =>
-        generateBspAndConnect(buildTool)
-      case Some(found) =>
+      case Some(found)
+          if isSelected(found.buildTool) &&
+            found.buildTool.isBspGenerated(folder) =>
         indexer.reloadWorkspaceAndIndex(
           forceImport,
           found.buildTool,
           found.digest,
           importBuild,
           quickConnectToBuildServer,
         )
-      case None =>
-        quickConnectToBuildServer()
+      case Some(BuildTool.Found(buildTool: BuildServerProvider, _)) =>
+        slowConnectToBuildToolBsp(buildTool, forceImport, isSelected(buildTool))
+      // Used in tests, `.bloop` folder exists but no build tool is detected
+      case _ => quickConnectToBuildServer()
     }
   }
 
   private def slowConnectToBuildToolBsp(
       buildTool: BuildServerProvider,
       forceImport: Boolean,
+      isSelected: Boolean,
   ) = {
     val notification = tables.dismissedNotifications.ImportChanges
     if (buildTool.isBspGenerated(folder)) {
-      tables.buildServers.chooseServer(buildTool.buildServerName)
+      maybeChooseServer(buildTool.buildServerName, isSelected)
       quickConnectToBuildServer()
     } else if (
-      userConfig.shouldAutoImportNewProject || buildTool.isAutoConnectable || forceImport
+      userConfig.shouldAutoImportNewProject || forceImport || isSelected ||
+      buildTool.isInstanceOf[ScalaCliBuildTool]
     ) {
+      maybeChooseServer(buildTool.buildServerName, isSelected)
       generateBspAndConnect(buildTool)
     } else if (notification.isDismissed) {
       Future.successful(BuildChange.None)
@@ -2139,6 +2115,7 @@ class MetalsLspService(
             notification.dismissForever()
             Future.successful(BuildChange.None)
           } else if (item == Messages.GenerateBspAndConnect.yes) {
+            maybeChooseServer(buildTool.buildServerName, isSelected)
             generateBspAndConnect(buildTool)
           } else {
             notification.dismiss(2, TimeUnit.MINUTES)
@@ -2148,10 +2125,13 @@ class MetalsLspService(
     }
   }
 
+  private def maybeChooseServer(name: String, alreadySelected: Boolean) =
+    if (alreadySelected) Future.successful(())
+    else tables.buildServers.chooseServer(name)
+
   private def generateBspAndConnect(
       buildTool: BuildServerProvider
   ): Future[BuildChange] = {
-    tables.buildServers.chooseServer(buildTool.buildServerName)
     buildTool
       .generateBspConfig(
         folder,

diff --git a/tests/unit/src/test/scala/tests/BillLspSuite.scala b/tests/unit/src/test/scala/tests/BillLspSuite.scala
@@ -202,7 +202,6 @@ class BillLspSuite extends BaseLspSuite("bill") {
       _ = assertNoDiff(
         client.workspaceMessageRequests,
         List(
-          Messages.ChooseBuildTool.message,
           Messages.BspSwitch.message,
           Messages.CheckDoctor.allProjectsMisconfigured,
         ).mkString("\n"),