Skip to content

Commit

Permalink
improvement: on new build tool added suggest switching
Browse files Browse the repository at this point in the history
  • Loading branch information
kasiaMarek committed Jun 21, 2023
1 parent 06815b4 commit 3d89198
Show file tree
Hide file tree
Showing 9 changed files with 155 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
}
19 changes: 16 additions & 3 deletions metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -84,6 +86,7 @@ final class BuildTools(
GradleBuildTool(userConfig),
MavenBuildTool(userConfig),
MillBuildTool(userConfig),
ScalaCliBuildTool(workspace),
)
}

Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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)

Expand All @@ -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)
}
}

Expand All @@ -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)
)
}
}
19 changes: 19 additions & 0 deletions metals/src/main/scala/scala/meta/internal/metals/Messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,7 @@ class MetalsLspService(
Future
.sequence(
List[Future[Unit]](
Future(buildTools.initialize()),
quickConnectToBuildServer().ignoreValue,
slowConnectToBuildServer(forceImport = false).ignoreValue,
Future(workspaceSymbols.indexClasspath()),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -2220,14 +2222,40 @@ 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))
} yield slowConnectToBuildServer(forceImport = false)
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -298,6 +297,7 @@ class ScalaCli(
}

object ScalaCli {
val minVersion = "0.1.3"

private def socketConn(
command: Seq[String],
Expand Down
36 changes: 36 additions & 0 deletions tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down Expand Up @@ -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 ()
}

}
12 changes: 12 additions & 0 deletions tests/unit/src/main/scala/tests/TestingClient.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 = { _ =>
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
Expand Down

0 comments on commit 3d89198

Please sign in to comment.