From 3d89198c58b39517fb8630175adc25aaca2ed5df Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Mon, 5 Jun 2023 11:24:04 +0100 Subject: [PATCH] improvement: on new build tool added suggest switching --- .../internal/builds/BuildToolSelector.scala | 19 +++++++++ .../meta/internal/builds/BuildTools.scala | 19 +++++++-- .../internal/builds/ScalaCliBuildTool.scala | 39 +++++++++---------- .../scala/meta/internal/metals/Messages.scala | 19 +++++++++ .../internal/metals/MetalsLspService.scala | 30 +++++++++++++- .../meta/internal/metals/doctor/Doctor.scala | 4 +- .../internal/metals/scalacli/ScalaCli.scala | 6 +-- .../scala/tests/scalacli/ScalaCliSuite.scala | 36 +++++++++++++++++ .../src/main/scala/tests/TestingClient.scala | 12 ++++++ 9 files changed, 155 insertions(+), 29 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/builds/BuildToolSelector.scala b/metals/src/main/scala/scala/meta/internal/builds/BuildToolSelector.scala index bc2cb791897..e30da44a4de 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BuildToolSelector.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BuildToolSelector.scala @@ -3,6 +3,7 @@ package scala.meta.internal.builds import scala.concurrent.ExecutionContext import scala.concurrent.Future +import scala.meta.internal.metals.Messages import scala.meta.internal.metals.Messages.ChooseBuildTool import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.Tables @@ -49,4 +50,22 @@ final class BuildToolSelector( foundBuildTool } } + + def onNewBuildToolAdded( + newBuildTool: BuildTool, + currentBuildTool: BuildTool, + ): Future[Boolean] = { + languageClient + .showMessageRequest( + Messages.NewBuildToolDetected.params(newBuildTool, currentBuildTool) + ) + .asScala + .map { + case Messages.NewBuildToolDetected.switch => + tables.buildServers.reset() + tables.buildTool.chooseBuildTool(newBuildTool.executableName) + true + case _ => 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 index 9f12adb5c9c..60c74966c7a 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala @@ -2,6 +2,7 @@ package scala.meta.internal.builds import java.nio.file.Files import java.util.Properties +import java.util.concurrent.atomic.AtomicReference import scala.meta.internal.io.PathIO import scala.meta.internal.metals.BloopServers @@ -27,6 +28,7 @@ final class BuildTools( userConfig: () => UserConfiguration, explicitChoiceMade: () => Boolean, ) { + private val lastDetectedBuildTools = new AtomicReference(Set.empty[String]) // NOTE: We do a couple extra check here before we say a workspace with a // `.bsp` is auto-connectable, and we ensure that a user has explicity chosen // to use another build server besides Bloop or it's a BSP server for a build @@ -64,7 +66,7 @@ final class BuildTools( } def isMill: Boolean = workspace.resolve("build.sc").isFile def isScalaCli: Boolean = - ScalaCliBuildTool.pathToScalaCliBsp(workspace).isFile + ScalaCliBuildTool.pathsToScalaCliBsp(workspace).exists(_.isFile) def isGradle: Boolean = { val defaultGradlePaths = List( "settings.gradle", @@ -84,6 +86,7 @@ final class BuildTools( GradleBuildTool(userConfig), MavenBuildTool(userConfig), MillBuildTool(userConfig), + ScalaCliBuildTool(workspace), ) } @@ -122,8 +125,7 @@ final class BuildTools( } def isBuildRelated( - workspace: AbsolutePath, - path: AbsolutePath, + path: AbsolutePath ): Option[String] = { if (isSbt && SbtBuildTool.isSbtRelatedPath(workspace, path)) Some(SbtBuildTool.name) @@ -135,6 +137,17 @@ final class BuildTools( Some(MillBuildTool.name) else None } + + def initialize(): Set[String] = { + lastDetectedBuildTools.getAndSet( + loadSupported().map(_.executableName).toSet + ) + } + + def newBuildTool(buildTool: String): Boolean = { + val before = lastDetectedBuildTools.getAndUpdate(_ + buildTool) + !before.contains(buildTool) + } } object BuildTools { diff --git a/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala index c38590d78b6..263c6e72216 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala @@ -3,14 +3,12 @@ package scala.meta.internal.builds import java.security.MessageDigest import scala.concurrent.Future -import scala.util.Try import scala.meta.internal.metals.BuildInfo import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.metals.scalacli.ScalaCli import scala.meta.io.AbsolutePath -import com.google.gson.JsonParser - class ScalaCliBuildTool(val workspaceVersion: Option[String]) extends BuildTool { @@ -23,7 +21,7 @@ class ScalaCliBuildTool(val workspaceVersion: Option[String]) override def digest(workspace: AbsolutePath): Option[String] = ScalaCliDigest.current(workspace) - override def minimumVersion: String = "0.1.4" + override def minimumVersion: String = ScalaCli.minVersion override def version: String = workspaceVersion.getOrElse(recommendedVersion) @@ -37,20 +35,20 @@ class ScalaCliBuildTool(val workspaceVersion: Option[String]) object ScalaCliBuildTool { def name = "scala-cli" - - def pathToScalaCliBsp(root: AbsolutePath): AbsolutePath = - root.resolve(".bsp").resolve("scala-cli.json") + def pathsToScalaCliBsp(root: AbsolutePath): List[AbsolutePath] = List( + root.resolve(".bsp").resolve("scala-cli.json"), + root.resolve(".bsp").resolve("scala.json"), + ) def apply(root: AbsolutePath): ScalaCliBuildTool = { - val workspaceFolder = - Try( - JsonParser - .parseString(pathToScalaCliBsp(root).readText) - .getAsJsonObject() - .get("version") - .getAsString() - ).toOption - new ScalaCliBuildTool(workspaceFolder) + val workspaceFolderVersions = + for { + path <- pathsToScalaCliBsp(root) + text <- path.readTextOpt + json = ujson.read(text) + version <- json("version").strOpt + } yield version + new ScalaCliBuildTool(workspaceFolderVersions.headOption) } } @@ -59,9 +57,10 @@ object ScalaCliDigest extends Digestable { workspace: AbsolutePath, digest: MessageDigest, ): Boolean = { - Digest.digestFileBytes( - ScalaCliBuildTool.pathToScalaCliBsp(workspace), - digest, - ) + ScalaCliBuildTool + .pathsToScalaCliBsp(workspace) + .foldRight(true)((path, acc) => + acc && Digest.digestFileBytes(path, digest) + ) } } diff --git a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala index 554a59e36cb..23b82ef2fa2 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala @@ -203,6 +203,25 @@ object Messages { } } + object NewBuildToolDetected { + val switch = new MessageActionItem("yes") + val dontSwitch = new MessageActionItem("no") + def params( + newBuildTool: BuildTool, + currentBuildTool: BuildTool, + ): ShowMessageRequestParams = { + val params = new ShowMessageRequestParams() + params.setMessage( + s"""|Would you like to switch from the current build tool (${currentBuildTool.executableName}) to newly detected ${newBuildTool.executableName}? + |This action will also reset build server choice. + |""".stripMargin + ) + params.setType(MessageType.Info) + params.setActions(List(switch, dontSwitch).asJava) + params + } + } + def partialNavigation(icons: Icons) = new MetalsStatusParams( s"${icons.info} Partial navigation", diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala index 088c89ff50e..1905ff76a94 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -884,6 +884,7 @@ class MetalsLspService( Future .sequence( List[Future[Unit]]( + Future(buildTools.initialize()), quickConnectToBuildServer().ignoreValue, slowConnectToBuildServer(forceImport = false).ignoreValue, Future(workspaceSymbols.indexClasspath()), @@ -1253,6 +1254,7 @@ class MetalsLspService( Future(indexer.reindexWorkspaceSources(paths)), compilations.compileFiles(paths), onBuildChanged(paths).ignoreValue, + Future.sequence(paths.map(onBuildToolAdded)), ) ++ paths.map(f => Future(interactiveSemanticdbs.textDocument(f))) ) .ignoreValue @@ -2220,7 +2222,7 @@ class MetalsLspService( private def onBuildChangedUnbatched( paths: Seq[AbsolutePath] ): Future[BuildChange] = { - val changedBuilds = paths.flatMap(buildTools.isBuildRelated(folder, _)) + val changedBuilds = paths.flatMap(buildTools.isBuildRelated) val buildChange = for { chosenBuildTool <- tables.buildTool.selectedBuildTool() if (changedBuilds.contains(chosenBuildTool)) @@ -2228,6 +2230,32 @@ class MetalsLspService( buildChange.getOrElse(Future.successful(BuildChange.None)) } + private def onBuildToolAdded( + path: AbsolutePath + ): Future[BuildChange] = { + val supportedBuildTools = buildTools.loadSupported() + val maybeBuildChange = for { + currentBuildToolName <- tables.buildTool.selectedBuildTool() + currentBuildTool <- supportedBuildTools.find( + _.executableName == currentBuildToolName + ) + addedBuildName <- buildTools.isBuildRelated(path) + if (buildTools.newBuildTool(addedBuildName)) + if (addedBuildName != currentBuildToolName) + newBuildTool <- supportedBuildTools.find( + _.executableName == addedBuildName + ) + } yield { + buildToolSelector + .onNewBuildToolAdded(newBuildTool, currentBuildTool) + .flatMap { switch => + if (switch) slowConnectToBuildServer(forceImport = false) + else Future.successful(BuildChange.None) + } + } + maybeBuildChange.getOrElse(Future.successful(BuildChange.None)) + } + /** * Returns the the definition location or reference locations of a symbol at a * given text document position. If the symbol represents the definition diff --git a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala index 4fba1f49ead..ecb6ce5e595 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala @@ -117,9 +117,9 @@ final class Doctor( buildTargets.allBuildTargetIds private def selectedBuildToolMessage(): Option[(String, Boolean)] = { - val explicitChoice = buildTools.loadSupported().length > 1 + val isExplicitChoice = buildTools.loadSupported().length > 1 tables.buildTool.selectedBuildTool().map { value => - (s"Build definition is coming from ${value}.", explicitChoice) + (s"Build definition is coming from ${value}.", isExplicitChoice) } } diff --git a/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala b/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala index a8137074743..6f1b4c69a3b 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala @@ -217,19 +217,18 @@ class ScalaCli( version.exists(ver => minVersion0.compareTo(ver) <= 0) } - val minVersion = "0.1.3" val cliCommand = userConfig().scalaCliLauncher .filter(_.trim.nonEmpty) .map(Seq(_)) .orElse { findInPath("scala-cli") .orElse(findInPath("scala")) - .filter(requireMinVersion(_, minVersion)) + .filter(requireMinVersion(_, ScalaCli.minVersion)) .map(p => Seq(p.toString)) } .getOrElse { scribe.warn( - s"scala-cli >= $minVersion not found in PATH, fetching and starting a JVM-based Scala CLI" + s"scala-cli >= ${ScalaCli.minVersion} not found in PATH, fetching and starting a JVM-based Scala CLI" ) val cp = ScalaCli.scalaCliClassPath() Seq( @@ -298,6 +297,7 @@ class ScalaCli( } object ScalaCli { + val minVersion = "0.1.3" private def socketConn( command: Seq[String], diff --git a/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala b/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala index 5020b25ef7f..1645a719741 100644 --- a/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala +++ b/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala @@ -2,6 +2,7 @@ package tests.scalacli import scala.concurrent.Future +import scala.meta.internal.metals.Messages import scala.meta.internal.metals.ServerCommands import scala.meta.internal.metals.{BuildInfo => V} @@ -225,4 +226,39 @@ class ScalaCliSuite extends BaseScalaCliSuite(V.scala3) { } yield () } + test("add-sbt") { + cleanWorkspace() + for { + _ <- scalaCliInitialize(useBsp = true)( + s"""/src/Main.scala + |object Main: + | def foo = 3 + |""".stripMargin + ) + _ = assert( + server.server.tables.buildTool.selectedBuildTool().contains("scala-cli") + ) + _ = assert( + server.server.tables.buildServers.selectedServer().contains("scala-cli") + ) + _ = FileLayout.fromString( + s"""|/build.sbt + |ThisBuild / scalaVersion := "3.3.0" + |ThisBuild / version := "0.1.0-SNAPSHOT" + | + |lazy val root = (project in file(".")) + |""".stripMargin, + workspace, + ) + _ = client.switchBuildTool = Messages.NewBuildToolDetected.switch + _ = client.importBuild = Messages.ImportBuild.yes + _ <- server.didSave("build.sbt")(identity) + _ = assert( + server.server.tables.buildTool.selectedBuildTool().contains("sbt") + ) + _ = assert(server.server.tables.buildServers.selectedServer().isEmpty) + _ = assert(server.server.bspSession.exists(_.main.isBloop)) + } yield () + } + } diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index fb501cb0368..26ed29bc886 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -63,6 +63,7 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) // Customization of the window/showMessageRequest response var importBuildChanges: MessageActionItem = ImportBuildChanges.notNow var importBuild: MessageActionItem = ImportBuild.notNow + var switchBuildTool: MessageActionItem = NewBuildToolDetected.dontSwitch var restartBloop: MessageActionItem = BloopVersionChange.notNow var getDoctorInformation: MessageActionItem = CheckDoctor.moreInformation var selectBspServer: Seq[MessageActionItem] => MessageActionItem = { _ => @@ -305,6 +306,15 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) params == targetParams } + def isNewBuildToolDetectedMessage(): Boolean = { + val buildTools = BuildTools.default().allAvailable + buildTools.exists(newBuildTool => + buildTools.exists(oldBuildTool => + NewBuildToolDetected.params(newBuildTool, oldBuildTool) == params + ) + ) + } + CompletableFuture.completedFuture { messageRequests.addLast(params.getMessage) showMessageRequestHandler(params).getOrElse { @@ -324,6 +334,8 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) createScalaFmtConf } else if (params.getMessage() == MainClass.message) { chooseMainClass(params.getActions.asScala.toSeq) + } else if (isNewBuildToolDetectedMessage()) { + switchBuildTool } else { throw new IllegalArgumentException(params.toString) }