From 9202ed22d68185c71ee4aacadeffa3e4c061b1bf Mon Sep 17 00:00:00 2001 From: Arthur McGibbon Date: Sat, 4 Nov 2023 17:34:55 +0000 Subject: [PATCH] Handle missing bsp info better --- .../metals/BuildServerConnection.scala | 76 ++++++++++++++++--- .../internal/metals/BuildTargetInfo.scala | 22 +++--- .../meta/internal/metals/JavaTarget.scala | 2 +- .../internal/metals/MetalsEnrichments.scala | 24 +++++- .../meta/internal/metals/ScalaTarget.scala | 2 +- .../meta/internal/metals/TargetData.scala | 6 +- .../meta/internal/metals/doctor/Doctor.scala | 4 +- .../metals/doctor/DoctorResults.scala | 2 +- .../internal/tvp/MetalsTreeViewProvider.scala | 4 +- tests/unit/src/main/scala/bill/Bill.scala | 1 + .../scala/tests/MetalsTestEnrichments.scala | 1 + 11 files changed, 113 insertions(+), 31 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/BuildServerConnection.scala b/metals/src/main/scala/scala/meta/internal/metals/BuildServerConnection.scala index 7ea3bdc19a1..051e8a6a223 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/BuildServerConnection.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/BuildServerConnection.scala @@ -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")) @@ -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( @@ -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) } } @@ -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] = { diff --git a/metals/src/main/scala/scala/meta/internal/metals/BuildTargetInfo.scala b/metals/src/main/scala/scala/meta/internal/metals/BuildTargetInfo.scala index a34865652c2..0f4b946ad1a 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/BuildTargetInfo.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/BuildTargetInfo.scala @@ -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") @@ -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) @@ -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)) ) @@ -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") @@ -194,7 +198,7 @@ class BuildTargetInfo(buildTargets: BuildTargets) { ) ) } else - List(" NONE") + List("NONE") } private def getDependencies(target: BuildTarget): List[String] = { @@ -202,7 +206,7 @@ class BuildTargetInfo(buildTargets: BuildTargets) { .map(f => buildTargets .info(f) - .map(_.getDisplayName()) + .map(_.getName()) .getOrElse("Unknown target") ) .toList @@ -213,7 +217,7 @@ class BuildTargetInfo(buildTargets: BuildTargets) { .filter(dependentTarget => dependentTarget.getDependencies.contains(target.getId()) ) - .map(_.getDisplayName()) + .map(_.getName()) .toList } diff --git a/metals/src/main/scala/scala/meta/internal/metals/JavaTarget.scala b/metals/src/main/scala/scala/meta/internal/metals/JavaTarget.scala index 1e2cffa9096..b25cb98f82a 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/JavaTarget.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/JavaTarget.scala @@ -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 diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala index 36c9f258afc..0129b27ed2e 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala @@ -88,6 +88,8 @@ object MetalsEnrichments def isSbtBuild: Boolean = dataKind == "sbt" + def isScalaBuild: Boolean = dataKind == "scala" + def baseDirectory: String = Option(buildTarget.getBaseDirectory()).getOrElse("") @@ -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] = { @@ -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]+", "-") + } else + buildTarget.getDisplayName + } } implicit class XtensionTaskStart(task: b.TaskStartParams) { diff --git a/metals/src/main/scala/scala/meta/internal/metals/ScalaTarget.scala b/metals/src/main/scala/scala/meta/internal/metals/ScalaTarget.scala index f67ce8173e9..eea0ea8a1c3 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ScalaTarget.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ScalaTarget.scala @@ -52,7 +52,7 @@ case class ScalaTarget( } } - def displayName: String = info.getDisplayName() + def displayName: String = info.getName def dataKind: String = info.dataKind diff --git a/metals/src/main/scala/scala/meta/internal/metals/TargetData.scala b/metals/src/main/scala/scala/meta/internal/metals/TargetData.scala index 028f6decfbc..0735e018909 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/TargetData.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/TargetData.scala @@ -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 } diff --git a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala index 7b9cc7a8302..4ce840c85c0 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala @@ -532,7 +532,7 @@ final class Doctor( javaTarget.displayName, gotoBuildTargetCommand(workspace, javaTarget.displayName), javaTarget.dataKind, - javaTarget.baseDirectory, + Option(javaTarget.baseDirectory), "Java", compilationStatus, diagnosticsStatus, @@ -612,7 +612,7 @@ final class Doctor( scalaTarget.displayName, gotoBuildTargetCommand(workspace, scalaTarget.displayName), scalaTarget.dataKind, - scalaTarget.baseDirectory, + Option(scalaTarget.baseDirectory), targetType, compilationStatus, diagnosticsStatus, diff --git a/metals/src/main/scala/scala/meta/internal/metals/doctor/DoctorResults.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/DoctorResults.scala index 71ff6b8b056..977c122d57e 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/doctor/DoctorResults.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/doctor/DoctorResults.scala @@ -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, diff --git a/metals/src/main/scala/scala/meta/internal/tvp/MetalsTreeViewProvider.scala b/metals/src/main/scala/scala/meta/internal/tvp/MetalsTreeViewProvider.scala index 90c15f7d527..3fd12d4b333 100644 --- a/metals/src/main/scala/scala/meta/internal/tvp/MetalsTreeViewProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/tvp/MetalsTreeViewProvider.scala @@ -293,7 +293,7 @@ class FolderTreeViewProvider( _.getId(), _.getUri(), uri => new BuildTargetIdentifier(uri), - _.getDisplayName(), + _.getName(), _.baseDirectory, { () => buildTargets.all.filter(target => @@ -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", ) } diff --git a/tests/unit/src/main/scala/bill/Bill.scala b/tests/unit/src/main/scala/bill/Bill.scala index 276eb293ebf..ea17771c5fd 100644 --- a/tests/unit/src/main/scala/bill/Bill.scala +++ b/tests/unit/src/main/scala/bill/Bill.scala @@ -140,6 +140,7 @@ object Bill { ) result.setDisplayName("id") result.setData(scalaTarget) + result.setDataKind("scala") result } val reporter = new StoreReporter diff --git a/tests/unit/src/main/scala/tests/MetalsTestEnrichments.scala b/tests/unit/src/main/scala/tests/MetalsTestEnrichments.scala index 06cb7f64e54..bd36280e4ac 100644 --- a/tests/unit/src/main/scala/tests/MetalsTestEnrichments.scala +++ b/tests/unit/src/main/scala/tests/MetalsTestEnrichments.scala @@ -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)