Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle missing bsp info better #5813

Merged
merged 1 commit into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,20 @@ class BuildServerConnection private (

def isAmmonite: Boolean = name == Ammonite.name

def supportsLanguage(id: String): Boolean =
Option(capabilities.getCompileProvider())
.exists(_.getLanguageIds().contains(id)) ||
Option(capabilities.getDebugProvider())
.exists(_.getLanguageIds().contains(id)) ||
Option(capabilities.getRunProvider())
.exists(_.getLanguageIds().contains(id)) ||
Option(capabilities.getTestProvider())
.exists(_.getLanguageIds().contains(id))

def supportsScala: Boolean = supportsLanguage("scala")

def supportsJava: Boolean = supportsLanguage("java")

def isDebuggingProvider: Boolean =
Option(capabilities.getDebugProvider())
.exists(_.getLanguageIds().contains("scala"))
Expand Down Expand Up @@ -186,13 +200,38 @@ class BuildServerConnection private (
def mainClasses(
params: ScalaMainClassesParams
): Future[ScalaMainClassesResult] = {
register(server => server.buildTargetScalaMainClasses(params)).asScala
val resultOnUnsupported = new ScalaMainClassesResult(Collections.emptyList)
if (supportsScala) {
val onFail = Some(
(
resultOnUnsupported,
"Scala main classes not supported by server",
)
)
register(
server => server.buildTargetScalaMainClasses(params),
onFail,
).asScala
} else Future.successful(resultOnUnsupported)

}

def testClasses(
params: ScalaTestClassesParams
): Future[ScalaTestClassesResult] = {
register(server => server.buildTargetScalaTestClasses(params)).asScala
val resultOnUnsupported = new ScalaTestClassesResult(Collections.emptyList)
if (supportsScala) {
val onFail = Some(
(
resultOnUnsupported,
"Scala test classes not supported by server",
)
)
register(
server => server.buildTargetScalaTestClasses(params),
onFail,
).asScala
} else Future.successful(resultOnUnsupported)
}

def startDebugSession(
Expand Down Expand Up @@ -240,13 +279,18 @@ class BuildServerConnection private (
)
if (isSbt || isAmmonite) Future.successful(resultOnJavacOptionsUnsupported)
else {
val onFail = Some(
(
resultOnJavacOptionsUnsupported,
"Java targets not supported by server",
if (supportsJava) {
val onFail = Some(
(
resultOnJavacOptionsUnsupported,
"Java targets not supported by server",
)
)
)
register(server => server.buildTargetJavacOptions(params), onFail).asScala
register(
server => server.buildTargetJavacOptions(params),
onFail,
).asScala
} else Future.successful(resultOnJavacOptionsUnsupported)
}
}

Expand All @@ -256,10 +300,18 @@ class BuildServerConnection private (
val resultOnScalaOptionsUnsupported = new ScalacOptionsResult(
List.empty[ScalacOptionsItem].asJava
)
val onFail = Some(
(resultOnScalaOptionsUnsupported, "Scala targets not supported by server")
)
register(server => server.buildTargetScalacOptions(params), onFail).asScala
if (supportsScala) {
val onFail = Some(
(
resultOnScalaOptionsUnsupported,
"Scala targets not supported by server",
)
)
register(
server => server.buildTargetScalacOptions(params),
onFail,
).asScala
} else Future.successful(resultOnScalaOptionsUnsupported)
}

def buildTargetSources(params: SourcesParams): Future[SourcesResult] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class BuildTargetInfo(buildTargets: BuildTargets) {

def buildTargetDetails(targetName: String, uri: String): String = {
buildTargets.all
.find(_.getDisplayName == targetName)
.find(_.getName == targetName)
.orElse(buildTargets.all.find(_.getId.getUri.toString == uri))
.map(target => buildTargetDetail(target.getId()))
.getOrElse(s"Build target $targetName not found")
Expand All @@ -32,7 +32,7 @@ class BuildTargetInfo(buildTargets: BuildTargets) {

commonInfo.foreach(info => {
output += "Target"
output += s" ${info.getDisplayName}"
output += s" ${info.getName}"

if (!info.getTags.isEmpty)
output ++= getSection("Tags", info.getTags.asScala.toList)
Expand Down Expand Up @@ -91,7 +91,10 @@ class BuildTargetInfo(buildTargets: BuildTargets) {
"Scala Binary Version",
List(info.scalaBinaryVersion),
)
output ++= getSection("Scala Platform", List(info.scalaPlatform))
output ++= getSection(
"Scala Platform",
List(info.scalaPlatform.toString()),
)
info.jvmVersion.foreach(jvmVersion =>
output ++= getSection("JVM Version", List(jvmVersion))
)
Expand Down Expand Up @@ -136,18 +139,19 @@ class BuildTargetInfo(buildTargets: BuildTargets) {

private def getSection(
sectionName: String,
sectionText: List[_],
sectionText: List[String],
): List[String] =
"" :: sectionName :: {
if (sectionText.isEmpty) List(" NONE")
else sectionText.map(text => s" $text")
else
sectionText.map(text => s" ${if (text.isEmpty) "[BLANK]" else text}")
}

private def translateCapability(
capability: String,
hasCapability: Boolean,
): String =
if (hasCapability) s" $capability" else s" $capability <- NOT SUPPORTED"
if (hasCapability) capability else s"$capability <- NOT SUPPORTED"

private def jarHasSource(jarName: String): Boolean = {
val sourceJarName = jarName.replace(".jar", "-sources.jar")
Expand Down Expand Up @@ -194,15 +198,15 @@ class BuildTargetInfo(buildTargets: BuildTargets) {
)
)
} else
List(" NONE")
List("NONE")
}

private def getDependencies(target: BuildTarget): List[String] = {
target.getDependencies.asScala
.map(f =>
buildTargets
.info(f)
.map(_.getDisplayName())
.map(_.getName())
.getOrElse("Unknown target")
)
.toList
Expand All @@ -213,7 +217,7 @@ class BuildTargetInfo(buildTargets: BuildTargets) {
.filter(dependentTarget =>
dependentTarget.getDependencies.contains(target.getId())
)
.map(_.getDisplayName())
.map(_.getName())
.toList
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ case class JavaTarget(
javac: JavacOptionsItem,
bspConnection: Option[BuildServerConnection],
) {
def displayName: String = info.getDisplayName()
def displayName: String = info.getName

def dataKind: String = info.dataKind

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ object MetalsEnrichments

def isSbtBuild: Boolean = dataKind == "sbt"

def isScalaBuild: Boolean = dataKind == "scala"

def baseDirectory: String =
Option(buildTarget.getBaseDirectory()).getOrElse("")

Expand All @@ -96,7 +98,11 @@ object MetalsEnrichments
def asScalaBuildTarget: Option[b.ScalaBuildTarget] = {
asSbtBuildTarget
.map(_.getScalaBuildTarget)
.orElse(decodeJson(buildTarget.getData, classOf[b.ScalaBuildTarget]))
.orElse(
if (isScalaBuild)
decodeJson(buildTarget.getData, classOf[b.ScalaBuildTarget])
else None
)
}

def asSbtBuildTarget: Option[b.SbtBuildTarget] = {
Expand All @@ -105,6 +111,22 @@ object MetalsEnrichments
else
None
}

def getName(): String = {
if (buildTarget.getDisplayName == null) {
val name =
if (buildTarget.getBaseDirectory() != null)
buildTarget
.getId()
.getUri()
.stripPrefix(buildTarget.getBaseDirectory())
else
buildTarget.getId().getUri()

name.replaceAll("[^a-zA-Z0-9]+", "-")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tgodzik Should I change this so the non A->Z chars are always replaced with -, not just when DisplayName is not supplied?
The issue is that in FileDecoderProvider Metals uses the display name to build up a URI to then display build target info to the user. If there is a non URI valid character then this URI is built incorrectly. You can see that issue also in #5816.
The other option would be to change the way build target info is displayed but that would probably involve changing each individual clients (vscode, vim etc) as well.

} else
buildTarget.getDisplayName
}
}

implicit class XtensionTaskStart(task: b.TaskStartParams) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ case class ScalaTarget(
}
}

def displayName: String = info.getDisplayName()
def displayName: String = info.getName

def dataKind: String = info.dataKind

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,10 @@ final class TargetData {
}

def targetClassDirectories(id: BuildTargetIdentifier): List[String] = {
val scalacData = scalaTarget(id).map(_.scalac.getClassDirectory).toList
val javacData = javaTarget(id).map(_.javac.getClassDirectory).toList
val scalacData =
scalaTarget(id).map(_.scalac.getClassDirectory).filter(_.nonEmpty).toList
val javacData =
javaTarget(id).map(_.javac.getClassDirectory).filter(_.nonEmpty).toList
(scalacData ++ javacData).distinct
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ final class Doctor(
javaTarget.displayName,
gotoBuildTargetCommand(workspace, javaTarget.displayName),
javaTarget.dataKind,
javaTarget.baseDirectory,
Option(javaTarget.baseDirectory),
"Java",
compilationStatus,
diagnosticsStatus,
Expand Down Expand Up @@ -612,7 +612,7 @@ final class Doctor(
scalaTarget.displayName,
gotoBuildTargetCommand(workspace, scalaTarget.displayName),
scalaTarget.dataKind,
scalaTarget.baseDirectory,
Option(scalaTarget.baseDirectory),
targetType,
compilationStatus,
diagnosticsStatus,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ final case class DoctorTargetInfo(
name: String,
gotoCommand: String,
dataKind: String,
baseDirectory: String,
baseDirectory: Option[String],
targetType: String,
compilationStatus: DoctorStatus,
diagnosticsStatus: DoctorStatus,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ class FolderTreeViewProvider(
_.getId(),
_.getUri(),
uri => new BuildTargetIdentifier(uri),
_.getDisplayName(),
_.getName(),
_.baseDirectory,
{ () =>
buildTargets.all.filter(target =>
Expand Down Expand Up @@ -447,7 +447,7 @@ class FolderTreeViewProvider(
} yield TreeViewNode(
TreeViewProvider.Compile,
id.getUri,
s"${info.getDisplayName()} - ${compilation.timer.toStringSeconds} (${compilation.progressPercentage}%)",
s"${info.getName()} - ${compilation.timer.toStringSeconds} (${compilation.progressPercentage}%)",
icon = "compile",
)
}
Expand Down
1 change: 1 addition & 0 deletions tests/unit/src/main/scala/bill/Bill.scala
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ object Bill {
)
result.setDisplayName("id")
result.setData(scalaTarget)
result.setDataKind("scala")
result
}
val reporter = new StoreReporter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ object MetalsTestEnrichments {
val gson = new Gson
val data = gson.toJsonTree(scalaTarget)
buildTarget.setData(data)
buildTarget.setDataKind("scala")
val result = new WorkspaceBuildTargetsResult(List(buildTarget).asJava)
val data0 = new m.internal.metals.TargetData
data0.addWorkspaceBuildTargets(result)
Expand Down
Loading