bugfix: allow for mixed multiple custom and known build servers

scalameta · Apr 4, 2024 · 3d3bc12 · 3d3bc12
1 parent 0abd3b9
commit 3d3bc12
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 27 deletions.
diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspServers.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspServers.scala
@@ -10,6 +10,7 @@ import scala.concurrent.Promise
 import scala.util.Properties
 import scala.util.Try
 
+import scala.meta.internal.bsp.BspServers.readInBspConfig
 import scala.meta.internal.io.FileIO
 import scala.meta.internal.metals.BuildServerConnection
 import scala.meta.internal.metals.Cancelable
@@ -153,25 +154,8 @@ final class BspServers(
    *  entries. Notes that this will not return Bloop even though it
    *  may be a server in the current workspace
    */
-  def findAvailableServers(): List[BspConnectionDetails] = {
-    val jsonFiles = findJsonFiles()
-    val gson = new Gson()
-    for {
-      candidate <- jsonFiles
-      text = FileIO.slurp(candidate, charset)
-      details <- Try(gson.fromJson(text, classOf[BspConnectionDetails])).fold(
-        e => {
-          scribe.error(s"parse error: $candidate", e)
-          List()
-        },
-        details => {
-          List(details)
-        },
-      )
-    } yield {
-      details
-    }
-  }
+  def findAvailableServers(): List[BspConnectionDetails] =
+    findJsonFiles().flatMap(readInBspConfig(_, charset))
 
   private def findJsonFiles(): List[AbsolutePath] = {
     val buf = List.newBuilder[AbsolutePath]
@@ -206,4 +190,22 @@ object BspServers {
       .map(path => Try(AbsolutePath(path)).toOption)
       .flatten
   }
+
+  def readInBspConfig(
+      path: AbsolutePath,
+      charset: Charset,
+  ): Option[BspConnectionDetails] = {
+    val text = FileIO.slurp(path, charset)
+    val gson = new Gson()
+    Try(gson.fromJson(text, classOf[BspConnectionDetails])).fold(
+      e => {
+        scribe.error(s"parse error: $path", e)
+        None
+      },
+      details => {
+        Some(details)
+      },
+    )
+  }
+
 }
diff --git a/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala b/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala
@@ -22,4 +22,6 @@ case class BspOnly(
     else None
   }
   override val forcesBuildServer = true
+
+  override def isBspGenerated(workspace: AbsolutePath): Boolean = true
 }
diff --git a/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala b/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala
@@ -1,9 +1,12 @@
 package scala.meta.internal.builds
 
+import java.nio.charset.Charset
+import java.nio.charset.StandardCharsets
 import java.nio.file.Files
 import java.util.Properties
 import java.util.concurrent.atomic.AtomicReference
 
+import scala.meta.internal.bsp.BspServers
 import scala.meta.internal.bsp.ScalaCliBspScope
 import scala.meta.internal.io.PathIO
 import scala.meta.internal.metals.BloopServers
@@ -32,6 +35,7 @@ final class BuildTools(
     bspGlobalDirectories: List[AbsolutePath],
     userConfig: () => UserConfiguration,
     explicitChoiceMade: () => Boolean,
+    charset: Charset,
 ) {
   private val lastDetectedBuildTools = new AtomicReference(Set.empty[String])
   // NOTE: We do a couple extra check here before we say a workspace with a
@@ -146,16 +150,19 @@ final class BuildTools(
       if (bspFolder.exists && bspFolder.isDirectory)
       buildTool <- bspFolder.toFile
         .listFiles()
-        .collect {
-          case file
-              if file.isFile() && file.getName().endsWith(".json") &&
-                !knownBsps(file.getName().stripSuffix(".json")) =>
-            BspOnly(
-              file.getName().stripSuffix(".json"),
+        .flatMap(file =>
+          if (file.isFile() && file.getName().endsWith(".json")) {
+            val absolutePath = AbsolutePath(file.toPath())
+            for {
+              config <- BspServers.readInBspConfig(absolutePath, charset)
+              if !knownBsps(config.getName())
+            } yield BspOnly(
+              config.getName(),
               root,
-              AbsolutePath(file.toPath()),
+              absolutePath,
             )
-        }
+          } else None
+        )
         .toList
     } yield buildTool
   }
@@ -276,5 +283,6 @@ object BuildTools {
       Nil,
       () => UserConfiguration(),
       explicitChoiceMade = () => false,
+      charset = StandardCharsets.UTF_8,
     )
 }
diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala
@@ -199,6 +199,7 @@ class MetalsLspService(
     bspGlobalDirectories,
     () => userConfig,
     () => tables.buildServers.selectedServer().nonEmpty,
+    charset,
   )
 
   def javaHome = userConfig.javaHome
@@ -2147,6 +2148,11 @@ class MetalsLspService(
         )
       case Some(BuildTool.Found(buildTool: BuildServerProvider, _)) =>
         slowConnectToBuildToolBsp(buildTool, forceImport, isSelected(buildTool))
+      // Used when there are multiple `.bsp/<name>.json` configs and a known build tool (e.g. sbt)
+      case Some(BuildTool.Found(buildTool, _))
+          if buildTool.isBspGenerated(folder) =>
+        maybeChooseServer(buildTool.buildServerName, isSelected(buildTool))
+        quickConnectToBuildServer()
       // Used in tests, `.bloop` folder exists but no build tool is detected
       case _ => quickConnectToBuildServer()
     }

diff --git a/tests/slow/src/test/scala/tests/MultipleBuildFilesLspSuite.scala b/tests/slow/src/test/scala/tests/MultipleBuildFilesLspSuite.scala
@@ -86,4 +86,26 @@ class MultipleBuildFilesLspSuite
     } yield ()
   }
 
+  test("custom-bsp-2") {
+    cleanWorkspace()
+    client.chooseBuildTool = actions =>
+      actions
+        .find(_.getTitle == "Custom")
+        .getOrElse(throw new Exception("no Custom as build tool"))
+    for {
+      _ <- initialize(
+        s"""|/.bsp/custom.json
+            |${ScalaCli.scalaCliBspJsonContent(bspName = "Custom")}
+            |/.bsp/other-custom.json
+            |${ScalaCli.scalaCliBspJsonContent(bspName = "Other custom")}
+            |/build.sbt
+            |scalaVersion := "${V.scala213}"
+            |""".stripMargin
+      )
+      _ <- server.server.indexingPromise.future
+      _ = assert(server.server.bspSession.nonEmpty)
+      _ = assert(server.server.bspSession.get.main.name == "Custom")
+    } yield ()
+  }
+
 }