From 34a05ba9e99aef663a1bc453df8274a2d6a910fe Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Mon, 30 Oct 2023 19:27:40 +0100 Subject: [PATCH 1/5] improvement: add custom bsp as possible build tool --- .../meta/internal/bsp/BspConnector.scala | 23 +- .../meta/internal/builds/BloopInstall.scala | 4 +- .../builds/BloopInstallProvider.scala | 2 +- .../scala/meta/internal/builds/BspOnly.scala | 14 ++ .../meta/internal/builds/BuildTool.scala | 37 ++- .../meta/internal/builds/BuildTools.scala | 23 ++ .../internal/builds/GradleBuildTool.scala | 3 +- .../meta/internal/builds/MavenBuildTool.scala | 3 +- .../meta/internal/builds/MillBuildTool.scala | 3 +- .../meta/internal/builds/SbtBuildTool.scala | 3 +- .../internal/builds/ScalaCliBuildTool.scala | 22 +- .../builds/VersionRecommendation.scala | 7 + .../scala/meta/internal/metals/Messages.scala | 9 +- .../internal/metals/MetalsLspService.scala | 230 ++++++++++-------- .../internal/metals/PopupChoiceReset.scala | 1 - .../internal/metals/scalacli/ScalaCli.scala | 3 +- .../tests/MultipleBuildFilesLspSuite.scala | 21 ++ .../src/main/scala/tests/TestingClient.scala | 14 +- .../src/test/scala/tests/BillLspSuite.scala | 15 +- 19 files changed, 265 insertions(+), 172 deletions(-) create mode 100644 metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala create mode 100644 metals/src/main/scala/scala/meta/internal/builds/VersionRecommendation.scala diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala index 163119842e2..439aa19c85a 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala @@ -6,7 +6,9 @@ import scala.concurrent.ExecutionContext import scala.concurrent.Future import scala.meta.internal.bsp.BspConfigGenerationStatus._ +import scala.meta.internal.builds.BloopInstallProvider import scala.meta.internal.builds.BuildServerProvider +import scala.meta.internal.builds.BuildTool import scala.meta.internal.builds.BuildTools import scala.meta.internal.builds.SbtBuildTool import scala.meta.internal.builds.ShellRunner @@ -43,13 +45,9 @@ class BspConnector( * Resolves the current build servers that either have a bsp entry or if the * workspace can support Bloop, it will also resolve Bloop. */ - def resolve(): BspResolvedResult = { + def resolve(buildTool: Option[BuildTool]): BspResolvedResult = { resolveExplicit().getOrElse { - if ( - buildTools - .loadSupported() - .exists(_.isBloopDefaultBsp) || buildTools.isBloop - ) + if (buildTool.isInstanceOf[BloopInstallProvider] || buildTools.isBloop) ResolvedBloop else bspServers.resolve() } @@ -74,11 +72,12 @@ class BspConnector( * of the bsp entry has already happened at this point. */ def connect( - projectRoot: AbsolutePath, + buildTool: Option[BuildTool], workspace: AbsolutePath, userConfiguration: UserConfiguration, shellRunner: ShellRunner, )(implicit ec: ExecutionContext): Future[Option[BspSession]] = { + val projectRoot = buildTool.map(_.projectRoot).getOrElse(workspace) def connect( projectRoot: AbsolutePath, bspTraceRoot: AbsolutePath, @@ -86,7 +85,7 @@ class BspConnector( ): Future[Option[BuildServerConnection]] = { def bspStatusOpt = Option.when(addLivenessMonitor)(bspStatus) scribe.info("Attempting to connect to the build server...") - resolve() match { + resolve(buildTool) match { case ResolvedNone => scribe.info("No build server found") Future.successful(None) @@ -180,7 +179,9 @@ class BspConnector( possibleBuildServerConn match { case None => Future.successful(None) case Some(buildServerConn) - if buildServerConn.isBloop && buildTools.isSbt => + if buildServerConn.isBloop && buildTool.exists( + _.isInstanceOf[SbtBuildTool] + ) => // NOTE: (ckipp01) we special case this here since sbt bsp server // doesn't yet support metabuilds. So in the future when that // changes, re-work this and move the creation of this out above @@ -277,7 +278,9 @@ class BspConnector( BspConnectionDetails, ]] = { if ( - bloopPresent || buildTools.loadSupported().exists(_.isBloopDefaultBsp) + bloopPresent || buildTools + .loadSupported() + .exists(_.isInstanceOf[BloopInstallProvider]) ) new BspConnectionDetails( BloopServers.name, diff --git a/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala b/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala index d6593e8f1f8..a24bcb62466 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala @@ -37,7 +37,7 @@ final class BloopInstall( override def toString: String = s"BloopInstall($workspace)" def runUnconditionally( - buildTool: BuildTool, + buildTool: BloopInstallProvider, isImportInProcess: AtomicBoolean, ): Future[WorkspaceLoadedStatus] = { if (isImportInProcess.compareAndSet(false, true)) { @@ -121,7 +121,7 @@ final class BloopInstall( // notifications. This method is synchronized to prevent asking the user // twice whether to import the build. def runIfApproved( - buildTool: BuildTool, + buildTool: BloopInstallProvider, digest: String, isImportInProcess: AtomicBoolean, ): Future[WorkspaceLoadedStatus] = diff --git a/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala b/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala index ca18924a2a4..ccc4c5b12aa 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala @@ -6,7 +6,7 @@ import scala.meta.io.AbsolutePath /** * Helper trait for build tools that have a Bloop plugin */ -trait BloopInstallProvider { this: BuildTool => +trait BloopInstallProvider extends BuildTool { /** * Method used to generate the necesary .bloop files for the diff --git a/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala b/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala new file mode 100644 index 00000000000..c175d8e23c7 --- /dev/null +++ b/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala @@ -0,0 +1,14 @@ +package scala.meta.internal.builds + +import scala.meta.io.AbsolutePath + +/** + * Build tool for custom bsp detected in `.bsp/.json` + */ +case class BspOnly( + override val executableName: String, + override val projectRoot: AbsolutePath, +) extends BuildTool { + override def digest(workspace: AbsolutePath): Option[String] = Some("OK") + override val forcesBuildServer = true +} diff --git a/metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala index 5724e745a19..c6670036ee7 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala @@ -4,32 +4,12 @@ import java.nio.file.Files import java.nio.file.Path import java.nio.file.StandardCopyOption -import scala.concurrent.Future - import scala.meta.io.AbsolutePath trait BuildTool { - /** - * Export the build to Bloop - * - * This operation should be roughly equivalent to running `sbt bloopInstall` - * and should work for both updating an existing Bloop build or creating a new - * Bloop build. - */ - def bloopInstall( - workspace: AbsolutePath, - systemProcess: List[String] => Future[WorkspaceLoadedStatus], - ): Future[WorkspaceLoadedStatus] - def digest(workspace: AbsolutePath): Option[String] - def version: String - - def minimumVersion: String - - def recommendedVersion: String - protected lazy val tempDir: Path = { val dir = Files.createTempDirectory("metals") dir.toFile.deleteOnExit() @@ -40,15 +20,14 @@ trait BuildTool { def executableName: String - def isBloopDefaultBsp = true - def projectRoot: AbsolutePath + val forcesBuildServer = false + } object BuildTool { - case class Found(buildTool: BuildTool, digest: String) def copyFromResource( tempDir: Path, filePath: String, @@ -62,4 +41,16 @@ object BuildTool { outFile } + trait Verified + case class IncompatibleVersion(buildTool: VersionRecommendation) + extends Verified { + def message: String = s"Unsupported $buildTool version ${buildTool.version}" + } + case class NoChecksum(buildTool: BuildTool, root: AbsolutePath) + extends Verified { + def message: String = + s"Could not calculate checksum for ${buildTool.executableName} in $root" + } + case class Found(buildTool: BuildTool, digest: String) extends Verified + } 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 b9c44746772..13b891d309a 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala @@ -9,6 +9,7 @@ import scala.meta.internal.io.PathIO import scala.meta.internal.metals.BloopServers import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.UserConfiguration +import scala.meta.internal.metals.scalacli.ScalaCli import scala.meta.io.AbsolutePath import ujson.ParsingFailedException @@ -121,6 +122,26 @@ final class BuildTools( ) def isBazel: Boolean = bazelProject.isDefined + private def customBsps: List[BspOnly] = { + val bspFolder = workspace.resolve(".bsp") + val root = customProjectRoot.getOrElse(workspace) + if (bspFolder.exists && bspFolder.isDirectory) { + bspFolder.toFile + .listFiles() + .collect { + case file + if file.isFile() && file.getName().endsWith(".json") && !knowBsps( + file.getName().stripSuffix(".json") + ) => + BspOnly(file.getName().stripSuffix(".json"), root) + } + .toList + } else Nil + } + + private def knowBsps = + Set(SbtBuildTool.name, MillBuildTool.name) ++ ScalaCli.names + private def customProjectRoot = userConfig().customProjectRoot .map(relativePath => workspace.resolve(relativePath.trim())) @@ -187,6 +208,7 @@ final class BuildTools( mavenProject.foreach(buf += MavenBuildTool(userConfig, _)) millProject.foreach(buf += MillBuildTool(userConfig, _)) scalaCliProject.foreach(buf += ScalaCliBuildTool(workspace, _, userConfig)) + buf.addAll(customBsps) buf.result() } @@ -221,6 +243,7 @@ final class BuildTools( val before = lastDetectedBuildTools.getAndUpdate(_ + buildTool) !before.contains(buildTool) } + } object BuildTools { diff --git a/metals/src/main/scala/scala/meta/internal/builds/GradleBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/GradleBuildTool.scala index 3b7f38deefb..077817c9260 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/GradleBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/GradleBuildTool.scala @@ -20,7 +20,8 @@ case class GradleBuildTool( userConfig: () => UserConfiguration, projectRoot: AbsolutePath, ) extends BuildTool - with BloopInstallProvider { + with BloopInstallProvider + with VersionRecommendation { private val initScriptName = "init-script.gradle" private val gradleBloopVersion = BuildInfo.gradleBloopVersion diff --git a/metals/src/main/scala/scala/meta/internal/builds/MavenBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/MavenBuildTool.scala index a2ab5d48ffe..ba655d9f601 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/MavenBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/MavenBuildTool.scala @@ -10,7 +10,8 @@ case class MavenBuildTool( userConfig: () => UserConfiguration, projectRoot: AbsolutePath, ) extends BuildTool - with BloopInstallProvider { + with BloopInstallProvider + with VersionRecommendation { private lazy val embeddedMavenLauncher: AbsolutePath = { val out = BuildTool.copyFromResource(tempDir, "maven-wrapper.jar") diff --git a/metals/src/main/scala/scala/meta/internal/builds/MillBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/MillBuildTool.scala index 0510de98a62..48c373e036d 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/MillBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/MillBuildTool.scala @@ -14,7 +14,8 @@ case class MillBuildTool( projectRoot: AbsolutePath, ) extends BuildTool with BloopInstallProvider - with BuildServerProvider { + with BuildServerProvider + with VersionRecommendation { private def getMillVersion(workspace: AbsolutePath): String = { import scala.meta.internal.jdk.CollectionConverters._ diff --git a/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala index 14690d8a747..2850245e564 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala @@ -26,7 +26,8 @@ case class SbtBuildTool( userConfig: () => UserConfiguration, ) extends BuildTool with BloopInstallProvider - with BuildServerProvider { + with BuildServerProvider + with VersionRecommendation { import SbtBuildTool._ 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 091f0b85e18..5fbd6013164 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala @@ -17,7 +17,8 @@ class ScalaCliBuildTool( val projectRoot: AbsolutePath, userConfig: () => UserConfiguration, ) extends BuildTool - with BuildServerProvider { + with BuildServerProvider + with VersionRecommendation { lazy val runScalaCliCommand: Option[Seq[String]] = ScalaCli.localScalaCli(userConfig()) @@ -47,12 +48,6 @@ class ScalaCliBuildTool( ) ) - override def bloopInstall( - workspace: AbsolutePath, - systemProcess: List[String] => Future[WorkspaceLoadedStatus], - ): Future[WorkspaceLoadedStatus] = - Future.successful(WorkspaceLoadedStatus.Dismissed) - override def digest(workspace: AbsolutePath): Option[String] = ScalaCliDigest.current(workspace) @@ -64,16 +59,19 @@ class ScalaCliBuildTool( override def executableName: String = ScalaCliBuildTool.name - override def isBloopDefaultBsp = false + override val forcesBuildServer = true + + def isBspGenerated(workspace: AbsolutePath): Boolean = + ScalaCliBuildTool.pathsToScalaCliBsp(workspace).exists(_.isFile) } object ScalaCliBuildTool { def name = "scala-cli" - def pathsToScalaCliBsp(root: AbsolutePath): List[AbsolutePath] = List( - root.resolve(".bsp").resolve("scala-cli.json"), - root.resolve(".bsp").resolve("scala.json"), - ) + def pathsToScalaCliBsp(root: AbsolutePath): List[AbsolutePath] = + ScalaCli.names.toList.map(name => + root.resolve(".bsp").resolve(s"$name.json") + ) def apply( workspace: AbsolutePath, diff --git a/metals/src/main/scala/scala/meta/internal/builds/VersionRecommendation.scala b/metals/src/main/scala/scala/meta/internal/builds/VersionRecommendation.scala new file mode 100644 index 00000000000..5d70db053b2 --- /dev/null +++ b/metals/src/main/scala/scala/meta/internal/builds/VersionRecommendation.scala @@ -0,0 +1,7 @@ +package scala.meta.internal.builds + +trait VersionRecommendation { self: BuildTool => + def minimumVersion: String + def recommendedVersion: String + def version: String +} 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 317091cb03c..cbd8048f6a7 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala @@ -5,6 +5,7 @@ import java.nio.file.Path import scala.collection.mutable import scala.meta.internal.builds.BuildTool +import scala.meta.internal.builds.VersionRecommendation import scala.meta.internal.jdk.CollectionConverters._ import scala.meta.internal.metals.BloopJsonUpdateCause.BloopJsonUpdateCause import scala.meta.internal.metals.clients.language.MetalsInputBoxParams @@ -193,13 +194,13 @@ object Messages { } object ChooseBuildTool { + def message = + "Multiple build definitions found. Which would you like to use?" def params(builtTools: List[BuildTool]): ShowMessageRequestParams = { val messageActionItems = builtTools.map(bt => new MessageActionItem(bt.executableName)) val params = new ShowMessageRequestParams() - params.setMessage( - "Multiple build definitions found. Which would you like to use?" - ) + params.setMessage(message) params.setType(MessageType.Info) params.setActions(messageActionItems.asJava) params @@ -288,7 +289,7 @@ object Messages { def learnMoreUrl: String = Urls.docs("import-build") - def params(tool: BuildTool): ShowMessageRequestParams = { + def params(tool: VersionRecommendation): ShowMessageRequestParams = { def toFixMessage = s"To fix this problem, upgrade to $tool ${tool.recommendedVersion} " 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 44aa20d0c85..d52e544d83c 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -29,14 +29,17 @@ import scala.meta.internal.bsp.BuildChange import scala.meta.internal.bsp.ConnectionBspStatus import scala.meta.internal.bsp.ScalaCliBspScope import scala.meta.internal.builds.BloopInstall +import scala.meta.internal.builds.BloopInstallProvider import scala.meta.internal.builds.BspErrorHandler import scala.meta.internal.builds.BuildServerProvider import scala.meta.internal.builds.BuildTool import scala.meta.internal.builds.BuildToolSelector import scala.meta.internal.builds.BuildTools +import scala.meta.internal.builds.Digest import scala.meta.internal.builds.SbtBuildTool import scala.meta.internal.builds.ScalaCliBuildTool import scala.meta.internal.builds.ShellRunner +import scala.meta.internal.builds.VersionRecommendation import scala.meta.internal.builds.WorkspaceReload import scala.meta.internal.decorations.PublishDecorationsParams import scala.meta.internal.decorations.SyntheticHoverProvider @@ -756,7 +759,7 @@ class MetalsLspService( diagnostics, languageClient, () => bspSession, - () => bspConnector.resolve(), + () => bspConnector.resolve(buildTool), tables, clientConfig, mtagsResolver, @@ -771,7 +774,7 @@ class MetalsLspService( () => tables.buildTool.selectedBuildTool(), buildTargets, () => bspSession, - () => bspConnector.resolve(), + () => bspConnector.resolve(buildTool), buildTools, ) @@ -909,6 +912,29 @@ class MetalsLspService( val isInitialized = new AtomicBoolean(false) + def fullConnect(): Future[Unit] = + for { + found <- supportedBuildTool() + chosenBuildServer = found match { + case Some(BuildTool.Found(buildServer, _)) + if buildServer.forcesBuildServer => + tables.buildServers.chooseServer(buildServer.executableName) + Some(buildServer.executableName) + case _ => tables.buildServers.selectedServer() + } + _ <- Future + .sequence( + List( + quickConnectToBuildServer(), + slowConnectToBuildServer( + forceImport = false, + found, + chosenBuildServer, + ), + ) + ) + } yield () + def initialized(): Future[Unit] = { loadFingerPrints() registerNiceToHaveFilePatterns() @@ -921,8 +947,7 @@ class MetalsLspService( .sequence( List[Future[Unit]]( Future(buildTools.initialize()), - quickConnectToBuildServer().ignoreValue, - slowConnectToBuildServer(forceImport = false).ignoreValue, + fullConnect().ignoreValue, Future(workspaceSymbols.indexClasspath()), Future(formattingProvider.load()), ) @@ -969,7 +994,7 @@ class MetalsLspService( if (userConfig.customProjectRoot != old.customProjectRoot) { tables.buildTool.reset() tables.buildServers.reset() - slowConnectToBuildServer(false).ignoreValue + fullConnect() } else Future.successful(()) val resetDecorations = @@ -1482,11 +1507,10 @@ class MetalsLspService( val path = params.getTextDocument.getUri.toAbsolutePath if (path.isJava) javaFormattingProvider.format(params) - else - for { - projectRoot <- calculateOptProjectRoot().map(_.getOrElse(folder)) - res <- formattingProvider.format(path, projectRoot, token) - } yield res + else { + val projectRoot = optProjectRoot.getOrElse(folder) + formattingProvider.format(path, projectRoot, token) + } } override def onTypeFormatting( @@ -1744,8 +1768,7 @@ class MetalsLspService( } yield bloopServers.shutdownServer() case Some(session) if session.main.isSbt => for { - currentBuildTool <- buildTool() - res <- currentBuildTool match { + res <- buildTool match { case Some(sbt: SbtBuildTool) => for { _ <- disconnectOldBuildServer() @@ -2022,46 +2045,34 @@ class MetalsLspService( }) } - private def buildTool(): Future[Option[BuildTool]] = { - buildTools.loadSupported match { - case Nil => Future(None) - case buildTools => - for { - buildTool <- buildToolSelector.checkForChosenBuildTool( - buildTools - ) - } yield buildTool.filter(isCompatibleVersion) - } - } - - private def isCompatibleVersion(buildTool: BuildTool) = - SemVer.isCompatibleVersion( - buildTool.minimumVersion, - buildTool.version, - ) - - def supportedBuildTool(): Future[Option[BuildTool.Found]] = { - def isCompatibleVersion(buildTool: BuildTool) = { - val isCompatibleVersion = this.isCompatibleVersion(buildTool) - if (isCompatibleVersion) { + private def buildTool: Option[BuildTool] = + for { + name <- tables.buildTool.selectedBuildTool() + buildTool <- buildTools.loadSupported.find(_.executableName == name) + found <- isCompatibleVersion(buildTool) match { + case BuildTool.Found(bt, _) => Some(bt) + case _ => None + } + } yield found + + def isCompatibleVersion(buildTool: BuildTool): BuildTool.Verified = { + buildTool match { + case buildTool: VersionRecommendation + if !SemVer.isCompatibleVersion( + buildTool.minimumVersion, + buildTool.version, + ) => + BuildTool.IncompatibleVersion(buildTool) + case _ => buildTool.digest(folder) match { case Some(digest) => - Some(BuildTool.Found(buildTool, digest)) - case None => - scribe.warn( - s"Could not calculate checksum for ${buildTool.executableName} in $folder" - ) - None + BuildTool.Found(buildTool, digest) + case None => BuildTool.NoChecksum(buildTool, folder) } - } else { - scribe.warn(s"Unsupported $buildTool version ${buildTool.version}") - languageClient.showMessage( - Messages.IncompatibleBuildToolVersion.params(buildTool) - ) - None - } } + } + def supportedBuildTool(): Future[Option[BuildTool.Found]] = { buildTools.loadSupported match { case Nil => { if (!buildTools.isAutoConnectable()) { @@ -2077,49 +2088,76 @@ class MetalsLspService( buildTools ) } yield { - buildTool.flatMap(isCompatibleVersion) + buildTool.flatMap { bt => + isCompatibleVersion(bt) match { + case found: BuildTool.Found => Some(found) + case warn @ BuildTool.IncompatibleVersion(buildTool) => + scribe.warn(warn.message) + languageClient.showMessage( + Messages.IncompatibleBuildToolVersion.params(buildTool) + ) + None + case warn: BuildTool.NoChecksum => + scribe.warn(warn.message) + None + } + } } } } def slowConnectToBuildServer( forceImport: Boolean - ): Future[BuildChange] = - for { - possibleBuildTool <- supportedBuildTool() - chosenBuildServer = tables.buildServers.selectedServer() - isBloopOrEmpty = chosenBuildServer.isEmpty || chosenBuildServer.exists( - _ == BloopServers.name - ) - buildChange <- possibleBuildTool match { - case Some(BuildTool.Found(buildTool: ScalaCliBuildTool, _)) - if chosenBuildServer.isEmpty => - tables.buildServers.chooseServer(ScalaCliBuildTool.name) - val scalaCliBspConfigExists = - ScalaCliBuildTool.pathsToScalaCliBsp(folder).exists(_.isFile) - if (scalaCliBspConfigExists) Future.successful(BuildChange.None) - else - buildTool - .generateBspConfig( - folder, - args => bspConfigGenerator.runUnconditionally(buildTool, args), - statusBar, - ) - .flatMap(_ => quickConnectToBuildServer()) - case Some(found) if isBloopOrEmpty => - slowConnectToBloopServer(forceImport, found.buildTool, found.digest) - case Some(found) => - indexer.reloadWorkspaceAndIndex( - forceImport, - found.buildTool, - found.digest, - importBuild, - ) - case None => - Future.successful(BuildChange.None) + ): Future[BuildChange] = for { + buildTool <- supportedBuildTool() + chosenBuildServer = tables.buildServers.selectedServer() + buildChange <- slowConnectToBuildServer( + forceImport, + buildTool, + chosenBuildServer, + ) + } yield buildChange - } - } yield buildChange + def slowConnectToBuildServer( + forceImport: Boolean, + buildTool: Option[BuildTool.Found], + chosenBuildServer: Option[String], + ): Future[BuildChange] = { + val isBloopOrEmpty = chosenBuildServer.isEmpty || chosenBuildServer.exists( + _ == BloopServers.name + ) + + buildTool match { + case Some(BuildTool.Found(buildTool: BloopInstallProvider, digest)) + if isBloopOrEmpty => + slowConnectToBloopServer(forceImport, buildTool, digest) + case Some(BuildTool.Found(buildTool: ScalaCliBuildTool, _)) + if !buildTool.isBspGenerated(folder) => + tables.buildServers.chooseServer(buildTool.executableName) + buildTool + .generateBspConfig( + folder, + args => bspConfigGenerator.runUnconditionally(buildTool, args), + statusBar, + ) + .flatMap(_ => quickConnectToBuildServer()) + case Some(BuildTool.Found(buildTool, _)) + if !chosenBuildServer.exists( + _ == buildTool.executableName + ) && buildTool.forcesBuildServer => + tables.buildServers.chooseServer(buildTool.executableName) + quickConnectToBuildServer() + case Some(found) => + indexer.reloadWorkspaceAndIndex( + forceImport, + found.buildTool, + found.digest, + importBuild, + ) + case None => + Future.successful(BuildChange.None) + } + } /** * If there is no auto-connectable build server and no supported build tool is found @@ -2136,7 +2174,7 @@ class MetalsLspService( private def slowConnectToBloopServer( forceImport: Boolean, - buildTool: BuildTool, + buildTool: BloopInstallProvider, checksum: String, ): Future[BuildChange] = for { @@ -2149,9 +2187,8 @@ class MetalsLspService( if (result.isInstalled) quickConnectToBuildServer() else if (result.isFailed) { for { - maybeProjectRoot <- calculateOptProjectRoot() change <- - if (buildTools.isAutoConnectable(maybeProjectRoot)) { + if (buildTools.isAutoConnectable(optProjectRoot)) { // TODO(olafur) try to connect but gracefully error languageClient.showMessage( Messages.ImportProjectPartiallyFailed @@ -2172,16 +2209,13 @@ class MetalsLspService( } } yield change - def calculateOptProjectRoot(): Future[Option[AbsolutePath]] = - for { - possibleBuildTool <- buildTool() - } yield possibleBuildTool.map(_.projectRoot).orElse(buildTools.bloopProject) + def optProjectRoot(): Option[AbsolutePath] = + buildTool.map(_.projectRoot).orElse(buildTools.bloopProject) def quickConnectToBuildServer(): Future[BuildChange] = for { - optRoot <- calculateOptProjectRoot() change <- - if (!buildTools.isAutoConnectable(optRoot)) { + if (!buildTools.isAutoConnectable(optProjectRoot)) { scribe.warn("Build server is not auto-connectable.") Future.successful(BuildChange.None) } else { @@ -2259,10 +2293,9 @@ class MetalsLspService( (for { _ <- disconnectOldBuildServer() - maybeProjectRoot <- calculateOptProjectRoot() maybeSession <- timerProvider.timed("Connected to build server", true) { bspConnector.connect( - maybeProjectRoot.getOrElse(folder), + buildTool, folder, userConfig, shellRunner, @@ -2347,10 +2380,16 @@ class MetalsLspService( s"Connected to Build server: ${session.main.name} v${session.version}" ) cancelables.add(session) + buildTool.foreach( + workspaceReload.persistChecksumStatus(Digest.Status.Started, _) + ) bspSession = Some(session) for { _ <- importBuild(session) _ <- indexer.profiledIndexWorkspace(runDoctorCheck) + _ = buildTool.foreach( + workspaceReload.persistChecksumStatus(Digest.Status.Installed, _) + ) _ = if (session.main.isBloop) checkRunningBloopVersion(session.version) } yield { BuildChange.Reconnected @@ -2767,9 +2806,8 @@ class MetalsLspService( def resetWorkspace(): Future[Unit] = for { - maybeProjectRoot <- calculateOptProjectRoot() _ <- disconnectOldBuildServer() - _ = maybeProjectRoot match { + _ = optProjectRoot match { case Some(path) if buildTools.isBloop(path) => bloopServers.shutdownServer() clearBloopDir(path) diff --git a/metals/src/main/scala/scala/meta/internal/metals/PopupChoiceReset.scala b/metals/src/main/scala/scala/meta/internal/metals/PopupChoiceReset.scala index dc8b63b7398..a4544eb2302 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/PopupChoiceReset.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/PopupChoiceReset.scala @@ -28,7 +28,6 @@ class PopupChoiceReset( val result = if (value == BuildTool) { scribe.info("Resetting build tool selection.") tables.buildTool.reset() - tables.buildServers.reset() slowConnect().ignoreValue } else if (value == BuildImport) { tables.dismissedNotifications.ImportChanges.reset() 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 2c2bcf854d4..592ffe4396f 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 @@ -425,6 +425,7 @@ object ScalaCli { def scalaCliBspJsonContent( args: List[String] = Nil, projectRoot: String = ".", + bspName: String = "scala-cli", ): String = { val argv = List( ScalaCli.javaCommand, @@ -435,7 +436,7 @@ object ScalaCli { projectRoot, ) ++ args val bsjJson = ujson.Obj( - "name" -> "scala-cli", + "name" -> bspName, "argv" -> argv, "version" -> BuildInfo.scalaCliVersion, "bspVersion" -> scalaCliBspVersion, diff --git a/tests/slow/src/test/scala/tests/MultipleBuildFilesLspSuite.scala b/tests/slow/src/test/scala/tests/MultipleBuildFilesLspSuite.scala index 3dfa69f7d8d..a24339b97c7 100644 --- a/tests/slow/src/test/scala/tests/MultipleBuildFilesLspSuite.scala +++ b/tests/slow/src/test/scala/tests/MultipleBuildFilesLspSuite.scala @@ -3,6 +3,7 @@ package tests import scala.meta.internal.builds.MillBuildTool import scala.meta.internal.builds.SbtBuildTool import scala.meta.internal.metals.Messages.ChooseBuildTool +import scala.meta.internal.metals.scalacli.ScalaCli import scala.meta.internal.metals.{BuildInfo => V} import scala.meta.io.AbsolutePath @@ -65,4 +66,24 @@ class MultipleBuildFilesLspSuite } } + test("custom-bsp") { + 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")} + |/build.sbt + |scalaVersion := "${V.scala213}" + |""".stripMargin + ) + _ <- server.server.indexingPromise.future + _ = assert(server.server.bspSession.nonEmpty) + _ = assert(server.server.bspSession.get.main.name == "custom") + } yield () + } + } diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index a8a2d8ebf77..9792c7821cf 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -14,6 +14,7 @@ import scala.concurrent.Promise import scala.meta.inputs.Input import scala.meta.internal.bsp.ConnectionBspStatus import scala.meta.internal.builds.BuildTool +import scala.meta.internal.builds.BspErrorHandler import scala.meta.internal.builds.BuildTools import scala.meta.internal.decorations.PublishDecorationsParams import scala.meta.internal.metals.Buffers @@ -306,17 +307,6 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) // NOTE: (ckipp01) Just for easiness of testing, we are going to just look // for sbt and mill builds together, which are most common. The logic however // is identical for all build tools. - def isSameMessageFromList( - createParams: List[BuildTool] => ShowMessageRequestParams - ): Boolean = { - val buildTools = BuildTools - .default() - .allAvailable - .filter(bt => bt.executableName == "sbt" || bt.executableName == "mill") - - val targetParams = createParams(buildTools) - params == targetParams - } def isNewBuildToolDetectedMessage(): Boolean = { val buildTools = BuildTools.default().allAvailable @@ -347,7 +337,7 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) getDoctorInformation } else if (BspSwitch.isSelectBspServer(params)) { selectBspServer(params.getActions.asScala.toSeq) - } else if (isSameMessageFromList(ChooseBuildTool.params)) { + } else if (params.getMessage == ChooseBuildTool.message) { chooseBuildTool(params.getActions.asScala.toSeq) } else if (MissingScalafmtConf.isCreateScalafmtConf(params)) { createScalaFmtConf diff --git a/tests/unit/src/test/scala/tests/BillLspSuite.scala b/tests/unit/src/test/scala/tests/BillLspSuite.scala index 73d6611f099..35c26fe2b87 100644 --- a/tests/unit/src/test/scala/tests/BillLspSuite.scala +++ b/tests/unit/src/test/scala/tests/BillLspSuite.scala @@ -187,7 +187,9 @@ class BillLspSuite extends BaseLspSuite("bill") { testRoundtripCompilation() } - def testSelectServerDialogue(): Future[Unit] = { + def testSelectServerDialogue( + additionalMessages: List[String] = Nil + ): Future[Unit] = { // when asked, choose the Bob build tool client.selectBspServer = { actions => actions.find(_.getTitle == "Bob").get @@ -201,10 +203,11 @@ class BillLspSuite extends BaseLspSuite("bill") { ) _ = assertNoDiff( client.workspaceMessageRequests, - List( - Messages.BspSwitch.message, - Messages.CheckDoctor.allProjectsMisconfigured, - ).mkString("\n"), + (additionalMessages ++ + List( + Messages.BspSwitch.message, + Messages.CheckDoctor.allProjectsMisconfigured, + )).mkString("\n"), ) } yield () } @@ -213,7 +216,7 @@ class BillLspSuite extends BaseLspSuite("bill") { cleanWorkspace() Bill.installWorkspace(workspace.toNIO, "Bill") Bill.installWorkspace(workspace.toNIO, "Bob") - testSelectServerDialogue() + testSelectServerDialogue(List(Messages.ChooseBuildTool.message)) } test("mix") { From ca720024809f7bc58c27795ba3907dd608f2acc5 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Mon, 20 Nov 2023 16:13:06 +0100 Subject: [PATCH 2/5] reconnect on `.bsp` change --- .../meta/internal/bsp/BspConnector.scala | 12 +++--- .../builds/BloopInstallProvider.scala | 4 +- .../scala/meta/internal/builds/BspOnly.scala | 12 +++++- .../meta/internal/builds/BuildTool.scala | 2 + .../meta/internal/builds/BuildTools.scala | 12 ++++-- .../scala/meta/internal/builds/Digest.scala | 17 ++++++++ .../scala/meta/internal/metals/Configs.scala | 1 + .../scala/meta/internal/metals/Indexer.scala | 39 ++++++++++++------- .../internal/metals/MetalsLspService.scala | 1 + 9 files changed, 73 insertions(+), 27 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala index 439aa19c85a..b0b29ee14ef 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala @@ -6,7 +6,6 @@ import scala.concurrent.ExecutionContext import scala.concurrent.Future import scala.meta.internal.bsp.BspConfigGenerationStatus._ -import scala.meta.internal.builds.BloopInstallProvider import scala.meta.internal.builds.BuildServerProvider import scala.meta.internal.builds.BuildTool import scala.meta.internal.builds.BuildTools @@ -47,7 +46,7 @@ class BspConnector( */ def resolve(buildTool: Option[BuildTool]): BspResolvedResult = { resolveExplicit().getOrElse { - if (buildTool.isInstanceOf[BloopInstallProvider] || buildTools.isBloop) + if (buildTool.exists(_.isBloopInstallProvider) || buildTools.isBloop) ResolvedBloop else bspServers.resolve() } @@ -179,9 +178,10 @@ class BspConnector( possibleBuildServerConn match { case None => Future.successful(None) case Some(buildServerConn) - if buildServerConn.isBloop && buildTool.exists( - _.isInstanceOf[SbtBuildTool] - ) => + if buildServerConn.isBloop && buildTool.exists { + case _: SbtBuildTool => true + case _ => false + } => // NOTE: (ckipp01) we special case this here since sbt bsp server // doesn't yet support metabuilds. So in the future when that // changes, re-work this and move the creation of this out above @@ -280,7 +280,7 @@ class BspConnector( if ( bloopPresent || buildTools .loadSupported() - .exists(_.isInstanceOf[BloopInstallProvider]) + .exists(_.isBloopInstallProvider) ) new BspConnectionDetails( BloopServers.name, diff --git a/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala b/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala index ccc4c5b12aa..c18e118046c 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala @@ -9,7 +9,7 @@ import scala.meta.io.AbsolutePath trait BloopInstallProvider extends BuildTool { /** - * Method used to generate the necesary .bloop files for the + * Method used to generate the necessary .bloop files for the * build tool. */ def bloopInstall( @@ -22,4 +22,6 @@ trait BloopInstallProvider extends BuildTool { * Args necessary for build tool to generate the .bloop files. */ def bloopInstallArgs(workspace: AbsolutePath): List[String] + + override val isBloopInstallProvider = true } diff --git a/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala b/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala index c175d8e23c7..fe6ef611789 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala @@ -1,5 +1,9 @@ package scala.meta.internal.builds +import java.security.MessageDigest + +import scala.meta.internal.builds.Digest +import scala.meta.internal.mtags.MD5 import scala.meta.io.AbsolutePath /** @@ -9,6 +13,12 @@ case class BspOnly( override val executableName: String, override val projectRoot: AbsolutePath, ) extends BuildTool { - override def digest(workspace: AbsolutePath): Option[String] = Some("OK") + override def digest(workspace: AbsolutePath): Option[String] = { + val digest = MessageDigest.getInstance("MD5") + val isSuccess = + Digest.digestJson(workspace.resolve(s"$executableName.json"), digest) + if (isSuccess) Some(MD5.bytesToHex(digest.digest())) + else None + } override val forcesBuildServer = true } diff --git a/metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala index c6670036ee7..9dff270b338 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala @@ -24,6 +24,8 @@ trait BuildTool { val forcesBuildServer = false + val isBloopInstallProvider = false + } object BuildTool { 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 13b891d309a..9d7c72a7cb0 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala @@ -130,16 +130,15 @@ final class BuildTools( .listFiles() .collect { case file - if file.isFile() && file.getName().endsWith(".json") && !knowBsps( - file.getName().stripSuffix(".json") - ) => + if file.isFile() && file.getName().endsWith(".json") && + !knownBsps(file.getName().stripSuffix(".json")) => BspOnly(file.getName().stripSuffix(".json"), root) } .toList } else Nil } - private def knowBsps = + private def knownBsps = Set(SbtBuildTool.name, MillBuildTool.name) ++ ScalaCli.names private def customProjectRoot = @@ -230,6 +229,11 @@ final class BuildTools( Some(MavenBuildTool.name) else if (isMill && MillBuildTool.isMillRelatedPath(path)) Some(MillBuildTool.name) + else if ( + path.isFile && path.filename.endsWith(".json") && + path.parent.filename == ".bsp" + ) + Some(path.filename.stripSuffix(".json")) else None } diff --git a/metals/src/main/scala/scala/meta/internal/builds/Digest.scala b/metals/src/main/scala/scala/meta/internal/builds/Digest.scala index 719e47dc3d5..bb2fc11a5b7 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/Digest.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/Digest.scala @@ -4,6 +4,8 @@ import java.nio.charset.StandardCharsets import java.nio.file.Files import java.security.MessageDigest +import scala.util.Success +import scala.util.Try import scala.util.control.NonFatal import scala.xml.Comment import scala.xml.Node @@ -15,6 +17,8 @@ import scala.meta.internal.mtags.MD5 import scala.meta.internal.parsing.Trees import scala.meta.io.AbsolutePath +import ujson.BytesRenderer + case class Digest( md5: String, status: Status, @@ -174,6 +178,19 @@ object Digest { false } } + + def digestJson( + file: AbsolutePath, + digest: MessageDigest, + ): Boolean = { + Try(ujson.read(file.readText)) match { + case Success(json) => + val bytes = json.transform(BytesRenderer()).toByteArray() + digest.update(bytes) + true + case _ => false + } + } } trait Digestable { diff --git a/metals/src/main/scala/scala/meta/internal/metals/Configs.scala b/metals/src/main/scala/scala/meta/internal/metals/Configs.scala index 4e9b040388c..ece48f7a172 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Configs.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Configs.scala @@ -40,6 +40,7 @@ object Configs { new FileSystemWatcher( Either.forLeft(s"$root/.metals/.reports/bloop/*/*") ), + new FileSystemWatcher(Either.forLeft(s"$root/.bsp/*.json")), ).asJava ) } diff --git a/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala b/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala index 57ddb7b2006..4ecf03eb904 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala @@ -19,6 +19,7 @@ import scala.meta.dialects._ import scala.meta.inputs.Input import scala.meta.internal.bsp.BspSession import scala.meta.internal.bsp.BuildChange +import scala.meta.internal.builds.BspOnly import scala.meta.internal.builds.BuildTool import scala.meta.internal.builds.BuildTools import scala.meta.internal.builds.Digest.Status @@ -88,25 +89,33 @@ final case class Indexer( buildTool: BuildTool, checksum: String, importBuild: BspSession => Future[Unit], + reconnectToBuildServer: () => Future[BuildChange], ): Future[BuildChange] = { def reloadAndIndex(session: BspSession): Future[BuildChange] = { workspaceReload().persistChecksumStatus(Status.Started, buildTool) - session - .workspaceReload() - .flatMap(_ => importBuild(session)) - .map { _ => - scribe.info("Correctly reloaded workspace") - profiledIndexWorkspace(doctorCheck) - workspaceReload().persistChecksumStatus(Status.Installed, buildTool) - BuildChange.Reloaded - } - .recoverWith { case NonFatal(e) => - scribe.error(s"Unable to reload workspace: ${e.getMessage()}") - workspaceReload().persistChecksumStatus(Status.Failed, buildTool) - languageClient.showMessage(Messages.ReloadProjectFailed) - Future.successful(BuildChange.Failed) - } + buildTool match { + case _: BspOnly => reconnectToBuildServer() + case _ => + session + .workspaceReload() + .flatMap(_ => importBuild(session)) + .map { _ => + scribe.info("Correctly reloaded workspace") + profiledIndexWorkspace(doctorCheck) + workspaceReload().persistChecksumStatus( + Status.Installed, + buildTool, + ) + BuildChange.Reloaded + } + .recoverWith { case NonFatal(e) => + scribe.error(s"Unable to reload workspace: ${e.getMessage()}") + workspaceReload().persistChecksumStatus(Status.Failed, buildTool) + languageClient.showMessage(Messages.ReloadProjectFailed) + Future.successful(BuildChange.Failed) + } + } } bspSession() match { 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 d52e544d83c..84437fc2a15 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -2153,6 +2153,7 @@ class MetalsLspService( found.buildTool, found.digest, importBuild, + quickConnectToBuildServer, ) case None => Future.successful(BuildChange.None) From a6e1ac4c4e93cb3cf53e56928973aea3b4bcc6a5 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Mon, 20 Nov 2023 17:25:49 +0100 Subject: [PATCH 3/5] look for `.bsp` also in the project root --- .../meta/internal/bsp/BspConnector.scala | 22 +++++++++++++- .../scala/meta/internal/bsp/BspServers.scala | 11 +++---- .../scala/meta/internal/builds/BspOnly.scala | 5 ++-- .../meta/internal/builds/BuildTools.scala | 30 +++++++++---------- .../scala/meta/internal/metals/Configs.scala | 2 +- .../internal/metals/UserConfiguration.scala | 12 ++++++++ .../src/main/scala/tests/TestingClient.scala | 2 -- .../src/test/scala/tests/BillLspSuite.scala | 16 +++++----- 8 files changed, 65 insertions(+), 35 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala index b0b29ee14ef..ce3ec891312 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala @@ -10,6 +10,7 @@ import scala.meta.internal.builds.BuildServerProvider import scala.meta.internal.builds.BuildTool import scala.meta.internal.builds.BuildTools import scala.meta.internal.builds.SbtBuildTool +import scala.meta.internal.builds.ScalaCliBuildTool import scala.meta.internal.builds.ShellRunner import scala.meta.internal.metals.BloopServers import scala.meta.internal.metals.BuildServerConnection @@ -19,6 +20,7 @@ import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.StatusBar import scala.meta.internal.metals.Tables import scala.meta.internal.metals.UserConfiguration +import scala.meta.internal.metals.scalacli.ScalaCli import scala.meta.internal.semver.SemVer import scala.meta.io.AbsolutePath @@ -58,7 +60,10 @@ class BspConnector( else bspServers .findAvailableServers() - .find(_.getName == sel) + .find(buildServer => + (ScalaCli.names(buildServer.getName()) && ScalaCli.names(sel)) || + buildServer.getName == sel + ) .map(ResolvedBspOne) } } @@ -129,6 +134,7 @@ class BspConnector( .map(Some(_)) case ResolvedBspOne(details) => tables.buildServers.chooseServer(details.getName()) + optSetBuildTool(details.getName()) bspServers .newServer(projectRoot, bspTraceRoot, details, bspStatusOpt) .map(Some(_)) @@ -163,6 +169,7 @@ class BspConnector( ) ) _ = tables.buildServers.chooseServer(item.getName()) + _ = optSetBuildTool(item.getName()) conn <- bspServers.newServer( projectRoot, bspTraceRoot, @@ -197,6 +204,19 @@ class BspConnector( } } + private def optSetBuildTool(buildServerName: String): Unit = + buildTools.loadSupported + .find { + case _: ScalaCliBuildTool if ScalaCli.names(buildServerName) => true + case buildTool: BuildServerProvider + if buildTool.buildServerName.contains(buildServerName) => + true + case buildTool => buildTool.executableName == buildServerName + } + .foreach(buildTool => + tables.buildTool.chooseBuildTool(buildTool.executableName) + ) + private def sbtMetaWorkspaces(root: AbsolutePath): List[AbsolutePath] = { def recursive( p: AbsolutePath, diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspServers.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspServers.scala index f252c6abafb..838a6c185f8 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspServers.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspServers.scala @@ -47,6 +47,8 @@ final class BspServers( config: MetalsServerConfig, userConfig: () => UserConfiguration, )(implicit ec: ExecutionContextExecutorService) { + private def customProjectRoot = + userConfig().getCustomProjectRoot(mainWorkspace) def resolve(): BspResolvedResult = { findAvailableServers() match { @@ -153,7 +155,7 @@ final class BspServers( * may be a server in the current workspace */ def findAvailableServers(): List[BspConnectionDetails] = { - val jsonFiles = findJsonFiles(mainWorkspace) + val jsonFiles = findJsonFiles() val gson = new Gson() for { candidate <- jsonFiles @@ -172,9 +174,7 @@ final class BspServers( } } - private def findJsonFiles( - projectDirectory: AbsolutePath - ): List[AbsolutePath] = { + private def findJsonFiles(): List[AbsolutePath] = { val buf = List.newBuilder[AbsolutePath] def visit(dir: AbsolutePath): Unit = dir.list.foreach { p => @@ -182,7 +182,8 @@ final class BspServers( buf += p } } - visit(projectDirectory.resolve(".bsp")) + visit(mainWorkspace.resolve(".bsp")) + customProjectRoot.map(_.resolve(".bsp")).foreach(visit) bspGlobalInstallDirectories.foreach(visit) buf.result() } diff --git a/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala b/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala index fe6ef611789..7347a536600 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala @@ -7,16 +7,17 @@ import scala.meta.internal.mtags.MD5 import scala.meta.io.AbsolutePath /** - * Build tool for custom bsp detected in `.bsp/.json` + * Build tool for custom bsp detected in `.bsp/.json` or `bspGlobalDirectories` */ case class BspOnly( override val executableName: String, override val projectRoot: AbsolutePath, + pathToBspConfig: AbsolutePath, ) extends BuildTool { override def digest(workspace: AbsolutePath): Option[String] = { val digest = MessageDigest.getInstance("MD5") val isSuccess = - Digest.digestJson(workspace.resolve(s"$executableName.json"), digest) + Digest.digestJson(pathToBspConfig, digest) if (isSuccess) Some(MD5.bytesToHex(digest.digest())) else None } 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 9d7c72a7cb0..bc36c2fdc34 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala @@ -55,6 +55,7 @@ final class BuildTools( def isBloop: Boolean = bloopProject.isDefined def isBsp: Boolean = { hasJsonFile(workspace.resolve(".bsp")) || + customProjectRoot.exists(root => hasJsonFile(root.resolve(".bsp"))) || bspGlobalDirectories.exists(hasJsonFile) } private def hasJsonFile(dir: AbsolutePath): Boolean = { @@ -123,34 +124,33 @@ final class BuildTools( def isBazel: Boolean = bazelProject.isDefined private def customBsps: List[BspOnly] = { - val bspFolder = workspace.resolve(".bsp") + val bspFolders = + (workspace :: customProjectRoot.toList).distinct + .map(_.resolve(".bsp")) ++ bspGlobalDirectories val root = customProjectRoot.getOrElse(workspace) - if (bspFolder.exists && bspFolder.isDirectory) { - bspFolder.toFile + for { + bspFolder <- bspFolders + 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"), root) + BspOnly( + file.getName().stripSuffix(".json"), + root, + AbsolutePath(file.toPath()), + ) } .toList - } else Nil + } yield buildTool } private def knownBsps = Set(SbtBuildTool.name, MillBuildTool.name) ++ ScalaCli.names - private def customProjectRoot = - userConfig().customProjectRoot - .map(relativePath => workspace.resolve(relativePath.trim())) - .filter { projectRoot => - val exists = projectRoot.exists - if (!exists) { - scribe.error(s"custom project root $projectRoot does not exist") - } - exists - } + private def customProjectRoot = userConfig().getCustomProjectRoot(workspace) private def searchForBuildTool( isProjectRoot: AbsolutePath => Boolean diff --git a/metals/src/main/scala/scala/meta/internal/metals/Configs.scala b/metals/src/main/scala/scala/meta/internal/metals/Configs.scala index ece48f7a172..874e9cacc33 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Configs.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Configs.scala @@ -40,7 +40,7 @@ object Configs { new FileSystemWatcher( Either.forLeft(s"$root/.metals/.reports/bloop/*/*") ), - new FileSystemWatcher(Either.forLeft(s"$root/.bsp/*.json")), + new FileSystemWatcher(Either.forLeft(s"$root/**/.bsp/*.json")), ).asJava ) } diff --git a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala index 4361ec15ea4..c7ce292b43d 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala @@ -68,6 +68,18 @@ case class UserConfiguration( ) && this.showInferredType.nonEmpty showImplicitArguments || showInferredType || showImplicitConversionsAndClasses } + + def getCustomProjectRoot(workspace: AbsolutePath): Option[AbsolutePath] = + customProjectRoot + .map(relativePath => workspace.resolve(relativePath.trim())) + .filter { projectRoot => + val exists = projectRoot.toFile.exists + if (!exists) { + scribe.error(s"custom project root $projectRoot does not exist") + } + exists + } + } object UserConfiguration { diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index 9792c7821cf..687be471531 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -13,8 +13,6 @@ import scala.concurrent.Promise import scala.meta.inputs.Input import scala.meta.internal.bsp.ConnectionBspStatus -import scala.meta.internal.builds.BuildTool -import scala.meta.internal.builds.BspErrorHandler import scala.meta.internal.builds.BuildTools import scala.meta.internal.decorations.PublishDecorationsParams import scala.meta.internal.metals.Buffers diff --git a/tests/unit/src/test/scala/tests/BillLspSuite.scala b/tests/unit/src/test/scala/tests/BillLspSuite.scala index 35c26fe2b87..7c906d39578 100644 --- a/tests/unit/src/test/scala/tests/BillLspSuite.scala +++ b/tests/unit/src/test/scala/tests/BillLspSuite.scala @@ -187,9 +187,7 @@ class BillLspSuite extends BaseLspSuite("bill") { testRoundtripCompilation() } - def testSelectServerDialogue( - additionalMessages: List[String] = Nil - ): Future[Unit] = { + def testSelectServerDialogue(): Future[Unit] = { // when asked, choose the Bob build tool client.selectBspServer = { actions => actions.find(_.getTitle == "Bob").get @@ -203,11 +201,11 @@ class BillLspSuite extends BaseLspSuite("bill") { ) _ = assertNoDiff( client.workspaceMessageRequests, - (additionalMessages ++ - List( - Messages.BspSwitch.message, - Messages.CheckDoctor.allProjectsMisconfigured, - )).mkString("\n"), + List( + Messages.ChooseBuildTool.message, + Messages.BspSwitch.message, + Messages.CheckDoctor.allProjectsMisconfigured, + ).mkString("\n"), ) } yield () } @@ -216,7 +214,7 @@ class BillLspSuite extends BaseLspSuite("bill") { cleanWorkspace() Bill.installWorkspace(workspace.toNIO, "Bill") Bill.installWorkspace(workspace.toNIO, "Bob") - testSelectServerDialogue(List(Messages.ChooseBuildTool.message)) + testSelectServerDialogue() } test("mix") { From 253a1719f98a62d0b74aad3786a0ca4973d19bc9 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Thu, 30 Nov 2023 12:35:33 +0100 Subject: [PATCH 4/5] delete additional bsp config discovery, now discovered as a new build tool --- .../internal/metals/MetalsLspService.scala | 74 +++++++++---------- .../scala/tests/scalacli/ScalaCliSuite.scala | 13 ++++ .../src/main/scala/tests/TestingClient.scala | 3 - 3 files changed, 48 insertions(+), 42 deletions(-) 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 84437fc2a15..9b27d7311af 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -17,7 +17,6 @@ import scala.concurrent.TimeoutException import scala.concurrent.duration._ import scala.util.Failure import scala.util.Success -import scala.util.Try import scala.util.control.NonFatal import scala.meta.internal.bsp.BspConfigGenerationStatus._ @@ -287,7 +286,7 @@ class MetalsLspService( ) private val onBuildChanged = - BatchedFunction.fromFuture[AbsolutePath, BuildChange]( + BatchedFunction.fromFuture[AbsolutePath, Unit]( onBuildChangedUnbatched, "onBuildChanged", ) @@ -912,7 +911,8 @@ class MetalsLspService( val isInitialized = new AtomicBoolean(false) - def fullConnect(): Future[Unit] = + def fullConnect(): Future[Unit] = { + buildTools.initialize() for { found <- supportedBuildTool() chosenBuildServer = found match { @@ -934,6 +934,7 @@ class MetalsLspService( ) ) } yield () + } def initialized(): Future[Unit] = { loadFingerPrints() @@ -946,8 +947,7 @@ class MetalsLspService( Future .sequence( List[Future[Unit]]( - Future(buildTools.initialize()), - fullConnect().ignoreValue, + fullConnect(), Future(workspaceSymbols.indexClasspath()), Future(formattingProvider.load()), ) @@ -1358,14 +1358,6 @@ class MetalsLspService( ): CompletableFuture[Unit] = { val path = AbsolutePath(event.path) val isScalaOrJava = path.isScalaOrJava - event.eventType match { - case EventType.CreateOrModify - if path.isInBspDirectory(folder) && path.extension == "json" - && isValidBspFile(path) => - scribe.info(s"Detected new build tool in $path") - quickConnectToBuildServer() - case _ => - } if (isScalaOrJava && event.eventType == EventType.Delete) { onDelete(path).asJava } else if ( @@ -1393,15 +1385,12 @@ class MetalsLspService( } }.asJava } else if (path.isBuild) { - onBuildChanged(List(path)).ignoreValue.asJava + onBuildChanged(List(path)).asJava } else { CompletableFuture.completedFuture(()) } } - private def isValidBspFile(path: AbsolutePath): Boolean = - path.readTextOpt.exists(text => Try(ujson.read(text)).toOption.nonEmpty) - private def onChange(paths: Seq[AbsolutePath]): Future[Unit] = { paths.foreach { path => fingerprints.add(path, FileIO.slurp(path, charset)) @@ -1413,8 +1402,7 @@ class MetalsLspService( Future(indexer.reindexWorkspaceSources(paths)), compilations .compileFiles(paths, Option(focusedDocumentBuildTarget.get())), - onBuildChanged(paths).ignoreValue, - Future.sequence(paths.map(onBuildToolAdded)), + onBuildChanged(paths), ) ++ paths.map(f => Future(interactiveSemanticdbs.textDocument(f))) ) .ignoreValue @@ -2078,8 +2066,6 @@ class MetalsLspService( if (!buildTools.isAutoConnectable()) { warnings.noBuildTool() } - // wait for a bsp file to show up - fileWatcher.start(Set(folder.resolve(".bsp"))) Future(None) } case buildTools => @@ -2515,30 +2501,40 @@ class MetalsLspService( private def onBuildChangedUnbatched( paths: Seq[AbsolutePath] - ): Future[BuildChange] = { + ): Future[Unit] = { val changedBuilds = paths.flatMap(buildTools.isBuildRelated) - val buildChange = for { - chosenBuildTool <- tables.buildTool.selectedBuildTool() - if (changedBuilds.contains(chosenBuildTool)) - } yield slowConnectToBuildServer(forceImport = false) - buildChange.getOrElse(Future.successful(BuildChange.None)) + tables.buildTool.selectedBuildTool() match { + // no build tool and new added + case None if changedBuilds.nonEmpty => + scribe.info(s"Detected new build tool in $path") + fullConnect() + // used build tool changed + case Some(chosenBuildTool) if changedBuilds.contains(chosenBuildTool) => + slowConnectToBuildServer(forceImport = false).ignoreValue + // maybe new build tool added + case Some(chosenBuildTool) if changedBuilds.nonEmpty => + onBuildToolsAdded(chosenBuildTool, changedBuilds) + case _ => Future.unit + } } - private def onBuildToolAdded( - path: AbsolutePath - ): Future[BuildChange] = { + private def onBuildToolsAdded( + currentBuildToolName: String, + newBuildToolsChanged: Seq[String], + ): Future[Unit] = { 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 - ) + newBuildTool <- newBuildToolsChanged + .filter(buildTools.newBuildTool) + .flatMap(addedBuildName => + supportedBuildTools.find( + _.executableName == addedBuildName + ) + ) + .headOption } yield { buildToolSelector .onNewBuildToolAdded(newBuildTool, currentBuildTool) @@ -2546,8 +2542,8 @@ class MetalsLspService( if (switch) slowConnectToBuildServer(forceImport = false) else Future.successful(BuildChange.None) } - } - maybeBuildChange.getOrElse(Future.successful(BuildChange.None)) + }.ignoreValue + maybeBuildChange.getOrElse(Future.unit) } /** diff --git a/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala b/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala index e0269f152fe..640fbbf1b5d 100644 --- a/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala +++ b/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala @@ -12,6 +12,7 @@ import scala.meta.internal.metals.SlowTaskConfig import scala.meta.internal.metals.scalacli.ScalaCli import scala.meta.internal.metals.{BuildInfo => V} +import org.eclipse.{lsp4j => l} import tests.FileLayout class ScalaCliSuite extends BaseScalaCliSuite(V.scala3) { @@ -267,6 +268,18 @@ class ScalaCliSuite extends BaseScalaCliSuite(V.scala3) { _ <- server.initialized() _ = FileLayout.fromString(simpleFileLayout, workspace) _ = FileLayout.fromString(bspLayout, workspace) + _ <- server.fullServer + .didChangeWatchedFiles( + new l.DidChangeWatchedFilesParams( + List( + new l.FileEvent( + workspace.resolve(".bsp/scala-cli.json").toURI.toString(), + l.FileChangeType.Created, + ) + ).asJava + ) + ) + .asScala _ <- server.server.indexingPromise.future _ <- server.didOpen("MyTests.scala") _ <- assertDefinitionAtLocation( diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index 687be471531..00beb8840ff 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -302,9 +302,6 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) .map(tool => createParams(tool.toString())) .contains(params) } - // NOTE: (ckipp01) Just for easiness of testing, we are going to just look - // for sbt and mill builds together, which are most common. The logic however - // is identical for all build tools. def isNewBuildToolDetectedMessage(): Boolean = { val buildTools = BuildTools.default().allAvailable From 67269f0bdeacd1e3861924af458d3ecdd5d49a40 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Mon, 18 Dec 2023 11:52:53 +0100 Subject: [PATCH 5/5] add doc for `optSetBuildTool` --- .../src/main/scala/scala/meta/internal/bsp/BspConnector.scala | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala index ce3ec891312..a208dbb12e3 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala @@ -204,6 +204,10 @@ class BspConnector( } } + /** + * Looks for a build tool matching the chosen build server, and sets it as the chosen build server. + * Only for `bloop` there will be no matching build tool and the previously chosen one remains. + */ private def optSetBuildTool(buildServerName: String): Unit = buildTools.loadSupported .find {