Skip to content

Commit

Permalink
bugfix: choose correct workspace folder for jars and build target files
Browse files Browse the repository at this point in the history
  • Loading branch information
kasiaMarek committed Oct 20, 2023
1 parent 9ed092b commit 338af7d
Show file tree
Hide file tree
Showing 6 changed files with 191 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import java.io.ByteArrayOutputStream
import java.io.File
import java.io.PrintStream
import java.net.URI
import java.net.URLEncoder
import javax.annotation.Nullable

import scala.annotation.tailrec
Expand Down Expand Up @@ -296,12 +295,14 @@ final class FileDecoderProvider(
}

private def decodeBuildTarget(uri: URI): DecoderResponse = {
val targetName =
URIEncoderDecoder.decode(
uri.toString.stripSuffix(".metals-buildtarget").split('/').last
)
val text =
new BuildTargetInfo(buildTargets).buildTargetDetails(targetName)
val text = uri
.toString()
.toAbsolutePathSafe
.map { path =>
val targetName = path.filename.stripSuffix(".metals-buildtarget")
new BuildTargetInfo(buildTargets).buildTargetDetails(targetName)
}
.getOrElse(s"Error transforming $uri to path")
DecoderResponse.success(uri, text)
}

Expand Down Expand Up @@ -771,10 +772,10 @@ final class FileDecoderProvider(

object FileDecoderProvider {
def createBuildTargetURI(
workspace: AbsolutePath,
workspaceFolder: AbsolutePath,
buildTargetName: String,
): URI =
URI.create(
s"metalsDecode:${URLEncoder.encode(s"file:///${workspace.filename}/${buildTargetName}.metals-buildtarget")}"
s"metalsDecode:${workspaceFolder.resolve(s"${buildTargetName}.metals-buildtarget").toURI}"
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -242,11 +242,17 @@ class WorkspaceLspService(
def getServiceFor(path: AbsolutePath): MetalsLspService =
getServiceForOpt(path).getOrElse(fallbackService)

def getServiceFor(uri: String): MetalsLspService =
def getServiceFor(uri: String): MetalsLspService = {
// "metalsDecode" prefix is used for showing special files and is not an actual file system
val strippedUri = uri.stripPrefix("metalsDecode:")
(for {
// "metalsDecode" prefix is used for showing special files and is not an actual file system
path <- uri.stripPrefix("metalsDecode:").toAbsolutePathSafe()
} yield getServiceFor(path)).getOrElse(fallbackService)
path <- strippedUri.toAbsolutePathSafe()
service <-
if (strippedUri.startsWith("jar:"))
folderServices.find(_.buildTargets.inverseSources(path).nonEmpty)
else Some(getServiceFor(path))
} yield service).getOrElse(fallbackService)
}

def currentFolder: Option[MetalsLspService] =
focusedDocument.flatMap(getServiceForOpt)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,32 +323,46 @@ class FileDecoderProviderLspSuite

check(
"semanticdb-jar-compact",
s"""
|/metals.json
|{
| "a": {
| "scalaVersion": "${scala.meta.internal.metals.BuildInfo.scala213}",
| "libraryDependencies": ["org.scalameta::munit:0.7.29"]
| }
|}
|/a/src/main/scala/a/Main.scala
|package a
|import munit.Printable
|
|object Main {
| val a : Printable = null
| println(a)
|}
|""".stripMargin,
"a/src/main/scala/a/Main.scala",
Map(
"otherfolder" ->
s"""
|/metals.json
|{
| "b": {
| "scalaVersion": "${scala.meta.internal.metals.BuildInfo.scala3}"
| }
|}
|/b/src/main/scala/b/Main.scala
|package b
|
|object Main { }
|""".stripMargin,
"somefolder" ->
s"""
|/metals.json
|{
| "a": {
| "scalaVersion": "${scala.meta.internal.metals.BuildInfo.scala213}",
| "libraryDependencies": ["org.scalameta::munit:0.7.29"]
| }
|}
|/a/src/main/scala/a/Main.scala
|package a
|import munit.Printable
|
|object Main {
| val a : Printable = null
| println(a)
|}
|""".stripMargin,
),
"somefolder/a/src/main/scala/a/Main.scala",
None,
"decode",
Right(
FileDecoderProviderLspSuite.PrintableSemanticDBFile(coursierCacheDir)
),
customUri = Some(
s"metalsDecode:jar:${coursierCacheDir.toUri}v1/https/repo1.maven.org/maven2/org/scalameta/munit_2.13/0.7.29/munit_2.13-0.7.29-sources.jar!/munit/Printable.scala.semanticdb-compact"
),
identity(_),
s"metalsDecode:jar:${coursierCacheDir.toUri}v1/https/repo1.maven.org/maven2/org/scalameta/munit_2.13/0.7.29/munit_2.13-0.7.29-sources.jar!/munit/Printable.scala.semanticdb-compact",
)

check(
Expand Down Expand Up @@ -433,19 +447,37 @@ class FileDecoderProviderSbtLspSuite

checkBuildTarget(
"sbt-buildtarget",
SbtBuildLayout(
s"""|/a/src/main/scala/Main.scala
|package a
|class A {
| def foo(): Unit = ()
|}
|/b/src/main/scala/Main.scala
|package b
|class B {
| def foo(): Unit = ()
|}
|""".stripMargin,
V.scala3,
Map(
"otherfolder" ->
SbtBuildLayout(
s"""/a/src/main/scala/OtherMain.scala
|package a
|class A {
| def foo(): Unit = ()
|}
|/b/src/main/scala/OtherMain.scala
|package b
|class B {
| def foo(): Unit = ()
|}
|""".stripMargin,
V.scala3,
),
"somefolder" ->
SbtBuildLayout(
s"""|/a/src/main/scala/Main.scala
|package a
|class A {
| def foo(): Unit = ()
|}
|/b/src/main/scala/Main.scala
|package b
|class B {
| def foo(): Unit = ()
|}
|""".stripMargin,
V.scala3,
),
),
"a", // buildTarget, see: SbtBuildLayout
Right(FileDecoderProviderLspSuite.sbtBuildTargetResponse),
Expand All @@ -454,6 +486,7 @@ class FileDecoderProviderSbtLspSuite
result,
Set("Target", "Scala Version", "Base Directory", "Source Directories"),
),
"somefolder",
)

check(
Expand Down Expand Up @@ -497,23 +530,39 @@ class FileDecoderProviderSbtLspSuite

trait FileDecoderProviderLspSpec { self: BaseLspSuite =>

def checkBuildTarget(
testName: TestOptions,
input: String,
buildTarget: String,
expected: Either[String, String],
transformResult: String => String,
): Unit = checkBuildTarget(
testName,
Map("somefolder" -> input),
buildTarget,
expected,
transformResult,
"somefolder",
)

/**
* @param expected - we can use "@workspace" to represent the workspace directory
* (can't receive it via params because it will be set at "initialize" request)
*/
def checkBuildTarget(
testName: TestOptions,
input: String,
input: Map[String, String],
buildTarget: String,
expected: Either[String, String],
transformResult: String => String = identity,
transformResult: String => String,
folderName: String,
): Unit = {
test(testName) {
for {
_ <- initialize(input)
_ <- initialize(input, expectError = false)
result <- server.executeDecodeFileCommand(
FileDecoderProvider
.createBuildTargetURI(workspace, buildTarget)
.createBuildTargetURI(workspace.resolve(folderName), buildTarget)
.toString
)
} yield {
Expand All @@ -538,6 +587,26 @@ trait FileDecoderProviderLspSpec { self: BaseLspSuite =>
expected: Either[String, String],
transformResult: String => String = identity,
customUri: Option[String] = None,
): Unit = check(
testName,
Map("somefolder" -> input),
s"somefolder/$filePath",
picked,
expected,
transformResult,
customUri.getOrElse(
s"metalsDecode:file://$workspace/somefolder/$filePath.$extension"
),
)

def check(
testName: TestOptions,
input: Map[String, String],
filePath: String,
picked: Option[String],
expected: Either[String, String],
transformResult: String => String,
uri: => String,
): Unit = {
test(testName) {
cleanWorkspace()
Expand All @@ -547,13 +616,9 @@ trait FileDecoderProviderLspSpec { self: BaseLspSuite =>
}
}
for {
_ <- initialize(input)
_ <- initialize(input, expectError = false)
_ <- server.didOpen(filePath)
result <- server.executeDecodeFileCommand(
customUri.getOrElse(
s"metalsDecode:file://$workspace/$filePath.$extension"
)
)
result <- server.executeDecodeFileCommand(uri)
} yield {
assertEquals(
if (result.value != null) Right(transformResult(result.value))
Expand Down Expand Up @@ -1195,7 +1260,7 @@ object FileDecoderProviderLspSuite {
| ${V.scala3}
|
|Base Directory
| file://@workspace/a[2.13.8]/""".stripMargin
| file://@workspace/somefolder/a[2.13.8]/""".stripMargin

def sbtBuildTargetResponse: String =
s"""|Target
Expand All @@ -1205,11 +1270,11 @@ object FileDecoderProviderLspSuite {
| ${V.scala3}
|
|Base Directory
| file:@workspace/a/
| file:@workspace/somefolder/a/
|
|Source Directories
| @workspace/a/src/main/java
| @workspace/a/src/main/scala
| @workspace/a/src/main/scala-3
| @workspace/a/target/scala-${V.scala3}/src_managed/main (generated)""".stripMargin
| @workspace/somefolder/a/src/main/java
| @workspace/somefolder/a/src/main/scala
| @workspace/somefolder/a/src/main/scala-3
| @workspace/somefolder/a/target/scala-${V.scala3}/src_managed/main (generated)""".stripMargin
}
25 changes: 18 additions & 7 deletions tests/unit/src/main/scala/tests/BuildServerInitializer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,18 @@ object SbtServerInitializer extends BuildServerInitializer {
expectError: Boolean,
workspaceFolders: List[String] = Nil,
)(implicit ec: ExecutionContext): Future[InitializeResult] = {
val sbtVersion =
SbtBuildTool
.loadVersion(workspace)
.getOrElse(V.sbtVersion)
generateBspConfig(workspace, sbtVersion)
val paths =
if (workspaceFolders.isEmpty) List(workspace)
else workspaceFolders.map(workspace.resolve)
paths.foreach { path =>
val sbtVersion =
SbtBuildTool
.loadVersion(path)
.getOrElse(V.sbtVersion)
generateBspConfig(path, sbtVersion)
}
for {
initializeResult <- server.initialize()
initializeResult <- server.initialize(workspaceFolders)
_ <- server.initialized()
// choose sbt as the Bsp Server
_ = client.selectBspServer = { items =>
Expand All @@ -113,7 +118,13 @@ object SbtServerInitializer extends BuildServerInitializer {
)
}
}
_ <- server.executeCommand(ServerCommands.BspSwitch)
_ <- paths.zipWithIndex.foldLeft(Future.successful(())) {
case (future, (_, i)) =>
future.flatMap { _ =>
client.chooseWorkspaceFolder = { _(i) }
server.executeCommand(ServerCommands.BspSwitch).ignoreValue
}
}
} yield {
if (!expectError) {
server.assertBuildServerConnection()
Expand Down
6 changes: 6 additions & 0 deletions tests/unit/src/main/scala/tests/TestingClient.scala
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers)
var selectBspServer: Seq[MessageActionItem] => MessageActionItem = { _ =>
null
}
var chooseWorkspaceFolder: Seq[MessageActionItem] => MessageActionItem =
_.head
var chooseBuildTool: Seq[MessageActionItem] => MessageActionItem = {
actions =>
actions
Expand Down Expand Up @@ -378,6 +380,10 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers)
.logMessage(Icons.default)
) {
new MessageActionItem("ok")
} else if (
params.getMessage().startsWith("For which folder would you like to")
) {
chooseWorkspaceFolder(params.getActions().asScala.toSeq)
} else {
throw new IllegalArgumentException(params.toString)
}
Expand Down
Loading

0 comments on commit 338af7d

Please sign in to comment.