From 7062b8c9aa9ae46243fcba55ed7a13922784aa58 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Tue, 7 Nov 2023 18:06:04 +0100 Subject: [PATCH 1/5] improvement: fetch missing dependency sources --- .../meta/internal/metals/ImportedBuild.scala | 35 +++++- .../internal/metals/JarSourcesProvider.scala | 106 ++++++++++++++++++ .../test/scala/tests/sbt/SbtServerSuite.scala | 31 ++++- tests/unit/src/main/scala/tests/Library.scala | 23 ++-- .../scala/tests/JarSourcesProviderSuite.scala | 19 ++++ 5 files changed, 202 insertions(+), 12 deletions(-) create mode 100644 metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala create mode 100644 tests/unit/src/test/scala/tests/JarSourcesProviderSuite.scala diff --git a/metals/src/main/scala/scala/meta/internal/metals/ImportedBuild.scala b/metals/src/main/scala/scala/meta/internal/metals/ImportedBuild.scala index eabee1e5835..a3213bc66f0 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ImportedBuild.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ImportedBuild.scala @@ -81,9 +81,14 @@ object ImportedBuild { new JavacOptionsParams(ids) ) sources <- conn.buildTargetSources(new SourcesParams(ids)) - dependencySources <- conn.buildTargetDependencySources( + bspProvidedDependencySources <- conn.buildTargetDependencySources( new DependencySourcesParams(ids) ) + dependencySources <- resolveMissingDependencySources( + bspProvidedDependencySources, + javacOptions, + scalacOptions, + ) wrappedSources <- conn.buildTargetWrappedSources( new WrappedSourcesParams(ids) ) @@ -98,6 +103,34 @@ object ImportedBuild { ) } + private def resolveMissingDependencySources( + dependencySources: DependencySourcesResult, + javacOptions: JavacOptionsResult, + scalacOptions: ScalacOptionsResult, + )(implicit ec: ExecutionContext): Future[DependencySourcesResult] = Future { + val dependencySourcesItems = dependencySources.getItems().asScala.toList + val idsLookup = dependencySourcesItems.map(_.getTarget()).toSet + val classpaths = javacOptions + .getItems() + .asScala + .map(item => (item.getTarget(), item.getClasspath())) ++ + scalacOptions + .getItems() + .asScala + .map(item => (item.getTarget(), item.getClasspath())) + + val newItems = + classpaths.collect { + case (id, classpath) if !idsLookup(id) => + val items = JarSourcesProvider.fetchSources( + classpath.asScala.filter(_.endsWith(".jar")).toSeq + ) + new DependencySourcesItem(id, items.asJava) + } + + new DependencySourcesResult((dependencySourcesItems ++ newItems).asJava) + } + def fromList(data: Seq[ImportedBuild]): ImportedBuild = if (data.isEmpty) empty else if (data.lengthCompare(1) == 0) data.head diff --git a/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala new file mode 100644 index 00000000000..d7abafae8b8 --- /dev/null +++ b/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala @@ -0,0 +1,106 @@ +package scala.meta.internal.metals + +import java.nio.file.Path + +import scala.util.Success +import scala.util.Try +import scala.xml.XML + +import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.semver.SemVer +import scala.meta.io.AbsolutePath + +import coursierapi.Dependency +import coursierapi.Fetch + +object JarSourcesProvider { + + private val sbtRegex = "sbt-(.*)".r + + def fetchSources(jars: Seq[String]): Seq[String] = { + def sourcesPath(jar: String) = s"${jar.stripSuffix(".jar")}-sources.jar" + + val (haveSources, toDownload) = jars.partition { jar => + sourcesPath(jar).toAbsolutePathSafe.exists(_.exists) + } + + val dependencies = toDownload.flatMap { jarPath => + val pomPath = s"${jarPath.stripSuffix(".jar")}.pom" + val dependency = + for { + pom <- pomPath.toAbsolutePathSafe + if pom.exists + dependency <- getDependency(pom) + } yield dependency + + dependency.orElse { jarPath.toAbsolutePathSafe.flatMap(sbtFallback) } + }.distinct + + val fetchedSources = + dependencies.flatMap { dep => + Try(fetchDependencySources(dep)).toEither match { + case Right(fetched) => fetched.map(_.toUri().toString()) + case Left(error) => + scribe.warn( + s"could not fetch dependency sources for $dep, error: $error" + ) + None + } + } + val existingSources = haveSources.map(sourcesPath) + fetchedSources ++ existingSources + + } + + private def sbtFallback(jar: AbsolutePath): Option[Dependency] = { + val filename = jar.filename.stripSuffix(".jar") + filename match { + case sbtRegex(versionStr) if Try().isSuccess => + Try(SemVer.Version.fromString(versionStr)) match { + case Success(version) if version.toString == versionStr => + Some(Dependency.of("org.scala-sbt", "sbt", versionStr)) + case _ => None + } + case _ => None + } + } + + private def getDependency(pom: AbsolutePath) = { + val xml = XML.loadFile(pom.toFile) + val groupId = (xml \ "groupId").text + val version = (xml \ "version").text + val artifactId = (xml \ "artifactId").text + Option + .when(groupId.nonEmpty && version.nonEmpty && artifactId.nonEmpty) { + Dependency.of(groupId, artifactId, version) + } + .filterNot(dep => isSbtDap(dep) || isMetalsPlugin(dep)) + } + + private def isSbtDap(dependency: Dependency) = { + dependency.getModule().getOrganization() == "ch.epfl.scala" && + dependency.getModule().getName() == "sbt-debug-adapter" && + dependency.getVersion() == BuildInfo.debugAdapterVersion + } + + private def isMetalsPlugin(dependency: Dependency) = { + dependency.getModule().getOrganization() == "org.scalameta" && + dependency.getModule().getName() == "sbt-metals" && + dependency.getVersion() == BuildInfo.metalsVersion + } + + private def fetchDependencySources( + dependency: Dependency + ): List[Path] = { + Fetch + .create() + .withDependencies(dependency) + .addClassifiers("sources") + .fetchResult() + .getFiles() + .asScala + .map(_.toPath()) + .toList + } + +} diff --git a/tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala b/tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala index 75b47c71884..27c2e4b3e8a 100644 --- a/tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala +++ b/tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala @@ -23,13 +23,15 @@ import scribe.writer.Writer import tests.BaseImportSuite import tests.SbtBuildLayout import tests.SbtServerInitializer +import tests.ScriptsAssertions import tests.TestSemanticTokens /** * Basic suite to ensure that a connection to sbt server can be made. */ class SbtServerSuite - extends BaseImportSuite("sbt-server", SbtServerInitializer) { + extends BaseImportSuite("sbt-server", SbtServerInitializer) + with ScriptsAssertions { val preBspVersion = "1.3.13" val supportedMetaBuildVersion = "1.6.0-M1" @@ -497,4 +499,31 @@ class SbtServerSuite } yield () } + test("build-sbt") { + cleanWorkspace() + for { + _ <- initialize( + s"""|/project/build.properties + |sbt.version=${V.sbtVersion} + |/build.sbt + |${SbtBuildLayout.commonSbtSettings} + |ThisBuild / scalaVersion := "${V.scala213}" + |val a = project.in(file("a")) + |/a/src/main/scala/a/A.scala + |package a + |object A { + | val a = 1 + |} + |""".stripMargin + ) + _ <- server.didOpen("build.sbt") + res <- definitionsAt( + "build.sbt", + s"ThisBuild / sc@@alaVersion := \"${V.scala213}\"", + ) + _ = assert(res.length == 1) + _ = assertNoDiff(res.head.getUri().toAbsolutePath.filename, "Keys.scala") + } yield () + } + } diff --git a/tests/unit/src/main/scala/tests/Library.scala b/tests/unit/src/main/scala/tests/Library.scala index 8e66808ee66..4cbd7d416d4 100644 --- a/tests/unit/src/main/scala/tests/Library.scala +++ b/tests/unit/src/main/scala/tests/Library.scala @@ -25,8 +25,11 @@ object Library { Classpath(PackageIndex.bootClasspath.map(AbsolutePath.apply)), Classpath(JdkSources().right.get :: Nil), ) - def catsSources: Seq[AbsolutePath] = - fetchSources("org.typelevel", "cats-core_2.12", "2.0.0-M4") + + def catsDependency: Dependency = + Dependency.of("org.typelevel", "cats-core_2.12", "2.0.0-M4") + def catsSources: Seq[AbsolutePath] = fetchSources(catsDependency) + def cats: Seq[AbsolutePath] = fetch(catsDependency) def scala3: Library = { val binaryVersion = @@ -102,17 +105,17 @@ object Library { ) } - def fetchSources( - org: String, - artifact: String, - version: String, + def fetchSources(dependency: Dependency): Seq[AbsolutePath] = + fetch(dependency, Set("sources")) + + def fetch( + dependency: Dependency, + classifiers: Set[String] = Set.empty, ): Seq[AbsolutePath] = Fetch .create() - .withDependencies( - Dependency.of(org, artifact, version).withTransitive(false) - ) - .withClassifiers(Set("sources").asJava) + .withDependencies(dependency.withTransitive(false)) + .withClassifiers(classifiers.asJava) .fetch() .asScala .toSeq diff --git a/tests/unit/src/test/scala/tests/JarSourcesProviderSuite.scala b/tests/unit/src/test/scala/tests/JarSourcesProviderSuite.scala new file mode 100644 index 00000000000..0fe891455a3 --- /dev/null +++ b/tests/unit/src/test/scala/tests/JarSourcesProviderSuite.scala @@ -0,0 +1,19 @@ +package tests + +import scala.meta.internal.metals.JarSourcesProvider +import scala.meta.internal.metals.MetalsEnrichments._ + +class JarSourcesProviderSuite extends BaseSuite { + + test("download-deps") { + val downloadedSources = + JarSourcesProvider.fetchSources(Library.cats.map(_.toURI.toString())) + assert(downloadedSources.nonEmpty) + downloadedSources.foreach { pathStr => + val path = pathStr.toAbsolutePath + assert(path.exists) + assert(path.filename.endsWith("-sources.jar")) + } + } + +} From 6c67b4bd83940001018343e8a713e1402e53171b Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Mon, 20 Nov 2023 20:45:16 +0100 Subject: [PATCH 2/5] add preferred dialect to workspace symbol search --- .../main/scala/bench/ClasspathFuzzBench.scala | 2 +- .../main/scala/bench/WorkspaceFuzzBench.scala | 2 +- .../internal/metals/DefinitionProvider.scala | 3 ++- .../internal/metals/MetalsLspService.scala | 8 ++++++-- .../metals/WorkspaceSearchVisitor.scala | 16 ++++++++++++--- .../metals/WorkspaceSymbolProvider.scala | 20 +++++++++++++++---- .../tests/BaseWorkspaceSymbolSuite.scala | 2 +- .../scala/tests/debug/BaseStepDapSuite.scala | 4 ++++ 8 files changed, 44 insertions(+), 13 deletions(-) diff --git a/metals-bench/src/main/scala/bench/ClasspathFuzzBench.scala b/metals-bench/src/main/scala/bench/ClasspathFuzzBench.scala index d60bea30ca9..8239900417a 100644 --- a/metals-bench/src/main/scala/bench/ClasspathFuzzBench.scala +++ b/metals-bench/src/main/scala/bench/ClasspathFuzzBench.scala @@ -46,7 +46,7 @@ class ClasspathFuzzBench { @BenchmarkMode(Array(Mode.SingleShotTime)) @OutputTimeUnit(TimeUnit.MILLISECONDS) def run(): Seq[SymbolInformation] = { - symbols.search(query) + symbols.search(query, None) } } diff --git a/metals-bench/src/main/scala/bench/WorkspaceFuzzBench.scala b/metals-bench/src/main/scala/bench/WorkspaceFuzzBench.scala index df8a0f7e38f..c1328e1d62a 100644 --- a/metals-bench/src/main/scala/bench/WorkspaceFuzzBench.scala +++ b/metals-bench/src/main/scala/bench/WorkspaceFuzzBench.scala @@ -37,7 +37,7 @@ class WorkspaceFuzzBench { @BenchmarkMode(Array(Mode.SingleShotTime)) @OutputTimeUnit(TimeUnit.MILLISECONDS) def upper(): Seq[SymbolInformation] = { - symbols.search(query) + symbols.search(query, None) } } diff --git a/metals/src/main/scala/scala/meta/internal/metals/DefinitionProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/DefinitionProvider.scala index 249a1b53847..cd76c4ce507 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/DefinitionProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/DefinitionProvider.scala @@ -175,8 +175,9 @@ final class DefinitionProvider( else true } + val dialect = scalaVersionSelector.dialectFromBuildTarget(path) val locs = workspaceSearch - .searchExactFrom(ident.value, path, token) + .searchExactFrom(ident.value, path, token, dialect) val reducedGuesses = if (locs.size > 1) 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 22e08c5ab8e..65b050432b4 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -1677,7 +1677,8 @@ class MetalsLspService( ): Future[List[SymbolInformation]] = indexingPromise.future.map { _ => val timer = new Timer(time) - val result = workspaceSymbols.search(params.getQuery, token).toList + val result = + workspaceSymbols.search(params.getQuery, token, currentDialect).toList if (clientConfig.initialConfig.statistics.isWorkspaceSymbol) { scribe.info( s"time: found ${result.length} results for query '${params.getQuery}' in $timer" @@ -1687,9 +1688,12 @@ class MetalsLspService( } def workspaceSymbol(query: String): Seq[SymbolInformation] = { - workspaceSymbols.search(query) + workspaceSymbols.search(query, currentDialect) } + private def currentDialect = + focusedDocument().flatMap(scalaVersionSelector.dialectFromBuildTarget) + def indexSources(): Future[Unit] = Future { indexer.indexWorkspaceSources(buildTargets.allWritableData) } diff --git a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceSearchVisitor.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceSearchVisitor.scala index 796b86ed4cb..437fbdda27d 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceSearchVisitor.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceSearchVisitor.scala @@ -5,6 +5,8 @@ import java.{util => ju} import scala.collection.mutable +import scala.meta.Dialect +import scala.meta.dialects import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.mtags.GlobalSymbolIndex import scala.meta.internal.mtags.Symbol @@ -32,6 +34,7 @@ class WorkspaceSearchVisitor( token: CancelChecker, index: GlobalSymbolIndex, saveClassFileToDisk: Boolean, + preferredDialect: Option[Dialect], )(implicit rc: ReportContext) extends SymbolSearchVisitor { private val fromWorkspace = new ju.ArrayList[l.SymbolInformation]() @@ -88,14 +91,21 @@ class WorkspaceSearchVisitor( ): Option[SymbolDefinition] = { val nme = Classfile.name(filename) val tpe = Symbol(Symbols.Global(pkg, Descriptor.Type(nme))) - + val preferredDialects = preferredDialect match { + case Some(dialects.Scala213) => + Set(dialects.Scala213, dialects.Scala213Source3) + case Some(dialects.Scala212) => + Set(dialects.Scala212, dialects.Scala212Source3) + case opt => opt.toSet + } val forTpe = index.definitions(tpe) val defs = if (forTpe.isEmpty) { val term = Symbol(Symbols.Global(pkg, Descriptor.Term(nme))) index.definitions(term) } else forTpe - - defs.sortBy(_.path.toURI.toString).headOption + defs.sortBy { defn => + (!preferredDialects(defn.dialect), defn.path.toURI.toString) + }.headOption } override def shouldVisitPackage(pkg: String): Boolean = true override def visitWorkspaceSymbol( diff --git a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceSymbolProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceSymbolProvider.scala index ee90b0900da..c9cde2b03f6 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceSymbolProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceSymbolProvider.scala @@ -6,6 +6,7 @@ import java.nio.file.Path import scala.collection.concurrent.TrieMap import scala.util.control.NonFatal +import scala.meta.Dialect import scala.meta.internal.mtags.GlobalSymbolIndex import scala.meta.internal.pc.InterruptException import scala.meta.io.AbsolutePath @@ -39,14 +40,21 @@ final class WorkspaceSymbolProvider( var inDependencies: ClasspathSearch = ClasspathSearch.empty - def search(query: String): Seq[l.SymbolInformation] = { - search(query, () => ()) + def search( + query: String, + preferredDialect: Option[Dialect], + ): Seq[l.SymbolInformation] = { + search(query, () => (), preferredDialect) } - def search(query: String, token: CancelChecker): Seq[l.SymbolInformation] = { + def search( + query: String, + token: CancelChecker, + preferredDialect: Option[Dialect], + ): Seq[l.SymbolInformation] = { if (query.isEmpty) return Nil try { - searchUnsafe(query, token) + searchUnsafe(query, token, preferredDialect) } catch { case InterruptException() => Nil @@ -57,6 +65,7 @@ final class WorkspaceSymbolProvider( queryString: String, path: AbsolutePath, token: CancelToken, + preferredDialect: Option[Dialect], ): Seq[l.SymbolInformation] = { val query = WorkspaceSymbolQuery.exact(queryString) val visistor = @@ -66,6 +75,7 @@ final class WorkspaceSymbolProvider( token, index, saveClassFileToDisk, + preferredDialect, ) val targetId = buildTargets.inverseSources(path) search(query, visistor, targetId) @@ -205,6 +215,7 @@ final class WorkspaceSymbolProvider( private def searchUnsafe( textQuery: String, token: CancelChecker, + preferredDialect: Option[Dialect], ): Seq[l.SymbolInformation] = { val query = WorkspaceSymbolQuery.fromTextQuery(textQuery) val visitor = @@ -214,6 +225,7 @@ final class WorkspaceSymbolProvider( token, index, saveClassFileToDisk, + preferredDialect, ) search(query, visitor, None) visitor.allResults() diff --git a/tests/unit/src/main/scala/tests/BaseWorkspaceSymbolSuite.scala b/tests/unit/src/main/scala/tests/BaseWorkspaceSymbolSuite.scala index c58bc898a33..d09f8e5e7c0 100644 --- a/tests/unit/src/main/scala/tests/BaseWorkspaceSymbolSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseWorkspaceSymbolSuite.scala @@ -25,7 +25,7 @@ abstract class BaseWorkspaceSymbolSuite extends BaseSuite { expected: String, )(implicit loc: Location): Unit = { test(query) { - val result = symbols.search(query) + val result = symbols.search(query, None) val obtained = if (result.length > 100) s"${result.length} results" else { diff --git a/tests/unit/src/main/scala/tests/debug/BaseStepDapSuite.scala b/tests/unit/src/main/scala/tests/debug/BaseStepDapSuite.scala index ac56b5dd688..a598c8ae062 100644 --- a/tests/unit/src/main/scala/tests/debug/BaseStepDapSuite.scala +++ b/tests/unit/src/main/scala/tests/debug/BaseStepDapSuite.scala @@ -89,6 +89,7 @@ abstract class BaseStepDapSuite( .at("a/src/main/scala/a/ScalaMain.scala", line = 5)(StepIn) .at("a/src/main/java/a/JavaClass.java", line = 5)(StepOut) .at("a/src/main/scala/a/ScalaMain.scala", line = 6)(Continue), + focusFile = "a/src/main/scala/a/ScalaMain.scala", ) assertSteps("step-into-scala-lib", withoutVirtualDocs = true)( @@ -162,12 +163,14 @@ abstract class BaseStepDapSuite( steps .at("a/src/main/scala/a/Main.scala", line = 6)(Continue) .at("a/src/main/scala/a/Main.scala", line = 13)(Continue), + focusFile = "a/src/main/scala/a/Main.scala", ) def assertSteps(name: TestOptions, withoutVirtualDocs: Boolean = false)( sources: String, main: String, instrument: StepNavigator => StepNavigator, + focusFile: String = "a/src/main/scala/Main.scala", )(implicit loc: Location): Unit = { test(name, withoutVirtualDocs) { cleanWorkspace() @@ -176,6 +179,7 @@ abstract class BaseStepDapSuite( for { _ <- initialize(workspaceLayout) + _ <- server.didFocus(focusFile) navigator = instrument(StepNavigator(workspace)) debugger <- debugMain("a", main, navigator) _ <- debugger.initialize From 8778a3463c16e24a95a0f107f24527cc0ff1eaef Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Wed, 22 Nov 2023 13:04:09 +0100 Subject: [PATCH 3/5] add timeout to fetching dependency sources --- .../meta/internal/metals/ImportedBuild.scala | 23 ++++++---- .../internal/metals/JarSourcesProvider.scala | 42 ++++++++++++------- .../scala/tests/JarSourcesProviderSuite.scala | 22 ++++++---- 3 files changed, 57 insertions(+), 30 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/ImportedBuild.scala b/metals/src/main/scala/scala/meta/internal/metals/ImportedBuild.scala index a3213bc66f0..17229c95e09 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ImportedBuild.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ImportedBuild.scala @@ -107,7 +107,7 @@ object ImportedBuild { dependencySources: DependencySourcesResult, javacOptions: JavacOptionsResult, scalacOptions: ScalacOptionsResult, - )(implicit ec: ExecutionContext): Future[DependencySourcesResult] = Future { + )(implicit ec: ExecutionContext): Future[DependencySourcesResult] = { val dependencySourcesItems = dependencySources.getItems().asScala.toList val idsLookup = dependencySourcesItems.map(_.getTarget()).toSet val classpaths = javacOptions @@ -119,16 +119,21 @@ object ImportedBuild { .asScala .map(item => (item.getTarget(), item.getClasspath())) - val newItems = - classpaths.collect { - case (id, classpath) if !idsLookup(id) => - val items = JarSourcesProvider.fetchSources( - classpath.asScala.filter(_.endsWith(".jar")).toSeq - ) - new DependencySourcesItem(id, items.asJava) + val newItemsFuture = + Future.sequence { + classpaths.collect { + case (id, classpath) if !idsLookup(id) => + for { + items <- JarSourcesProvider.fetchSources( + classpath.asScala.filter(_.endsWith(".jar")).toSeq + ) + } yield new DependencySourcesItem(id, items.asJava) + } } - new DependencySourcesResult((dependencySourcesItems ++ newItems).asJava) + newItemsFuture.map { newItems => + new DependencySourcesResult((dependencySourcesItems ++ newItems).asJava) + } } def fromList(data: Seq[ImportedBuild]): ImportedBuild = diff --git a/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala index d7abafae8b8..2ba2c3796bb 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala @@ -1,9 +1,14 @@ package scala.meta.internal.metals import java.nio.file.Path +import java.util.concurrent.TimeUnit +import java.util.concurrent.TimeoutException +import scala.concurrent.ExecutionContext +import scala.concurrent.Future import scala.util.Success import scala.util.Try +import scala.util.control.NonFatal import scala.xml.XML import scala.meta.internal.metals.MetalsEnrichments._ @@ -12,12 +17,13 @@ import scala.meta.io.AbsolutePath import coursierapi.Dependency import coursierapi.Fetch +import coursierapi.error.CoursierError object JarSourcesProvider { private val sbtRegex = "sbt-(.*)".r - def fetchSources(jars: Seq[String]): Seq[String] = { + def fetchSources(jars: Seq[String])(implicit ec: ExecutionContext): Future[Seq[String]] = { def sourcesPath(jar: String) = s"${jar.stripSuffix(".jar")}-sources.jar" val (haveSources, toDownload) = jars.partition { jar => @@ -37,25 +43,33 @@ object JarSourcesProvider { }.distinct val fetchedSources = - dependencies.flatMap { dep => - Try(fetchDependencySources(dep)).toEither match { - case Right(fetched) => fetched.map(_.toUri().toString()) - case Left(error) => - scribe.warn( - s"could not fetch dependency sources for $dep, error: $error" - ) - None + Future.sequence { + dependencies.map{ dep => + fetchDependencySources(dep) + .recover { + case error : CoursierError => + scribe.warn(s"Could not fetch dependency sources for $dep, error: $error") + Nil + } } - } + }.recover { + case _: TimeoutException => + scribe.warn(s"Timeout when fetching dependency sources.") + Nil + case NonFatal(e) => + scribe.warn(s"Could not fetch dependency sources, error: $e.") + Nil + }.map(_.flatten.map(_.toUri().toString())) + val existingSources = haveSources.map(sourcesPath) - fetchedSources ++ existingSources + fetchedSources.map(_ ++ existingSources) } private def sbtFallback(jar: AbsolutePath): Option[Dependency] = { val filename = jar.filename.stripSuffix(".jar") filename match { - case sbtRegex(versionStr) if Try().isSuccess => + case sbtRegex(versionStr) => Try(SemVer.Version.fromString(versionStr)) match { case Success(version) if version.toString == versionStr => Some(Dependency.of("org.scala-sbt", "sbt", versionStr)) @@ -91,7 +105,7 @@ object JarSourcesProvider { private def fetchDependencySources( dependency: Dependency - ): List[Path] = { + )(implicit ec: ExecutionContext): Future[List[Path]] = Future { Fetch .create() .withDependencies(dependency) @@ -101,6 +115,6 @@ object JarSourcesProvider { .asScala .map(_.toPath()) .toList - } + }.withTimeout(5, TimeUnit.MINUTES) } diff --git a/tests/unit/src/test/scala/tests/JarSourcesProviderSuite.scala b/tests/unit/src/test/scala/tests/JarSourcesProviderSuite.scala index 0fe891455a3..0cb72eba492 100644 --- a/tests/unit/src/test/scala/tests/JarSourcesProviderSuite.scala +++ b/tests/unit/src/test/scala/tests/JarSourcesProviderSuite.scala @@ -1,18 +1,26 @@ package tests +import scala.concurrent.ExecutionContext + import scala.meta.internal.metals.JarSourcesProvider import scala.meta.internal.metals.MetalsEnrichments._ class JarSourcesProviderSuite extends BaseSuite { + private implicit val ctx: ExecutionContext = this.munitExecutionContext + test("download-deps") { - val downloadedSources = - JarSourcesProvider.fetchSources(Library.cats.map(_.toURI.toString())) - assert(downloadedSources.nonEmpty) - downloadedSources.foreach { pathStr => - val path = pathStr.toAbsolutePath - assert(path.exists) - assert(path.filename.endsWith("-sources.jar")) + for { + downloadedSources <- JarSourcesProvider.fetchSources( + Library.cats.map(_.toURI.toString()) + ) + } yield { + assert(downloadedSources.nonEmpty) + downloadedSources.foreach { pathStr => + val path = pathStr.toAbsolutePath + assert(path.exists) + assert(path.filename.endsWith("-sources.jar")) + } } } From 2c3950f00474d084774227c4e5d8194787c1aba8 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Wed, 22 Nov 2023 13:14:12 +0100 Subject: [PATCH 4/5] correctly fetch sbt plugins try resolving debug adapter plugin --- build.sbt | 5 +- .../internal/metals/JarSourcesProvider.scala | 115 ++++++++++-------- .../internal/metals/scalacli/ScalaCli.scala | 2 +- .../tests/scalacli/ScalaCliActionsSuite.scala | 2 +- .../scala/tests/JarSourcesProviderSuite.scala | 35 ++++++ 5 files changed, 107 insertions(+), 52 deletions(-) diff --git a/build.sbt b/build.sbt index caa8ff03879..e1460344b8c 100644 --- a/build.sbt +++ b/build.sbt @@ -487,8 +487,9 @@ lazy val metals = project // For reading classpaths. // for fetching ch.epfl.scala:bloop-frontend and other library dependencies "io.get-coursier" % "interface" % V.coursierInterfaces, - // for comparing versions - "io.get-coursier" %% "versions" % "0.3.2", + // for comparing versions && fetching from sbt maven repository + "io.get-coursier" %% "coursier" % V.coursier, + "io.get-coursier" %% "coursier-sbt-maven-repository" % V.coursier, // for logging "com.outr" %% "scribe" % V.scribe, "com.outr" %% "scribe-file" % V.scribe, diff --git a/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala index 2ba2c3796bb..f245a6b6188 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala @@ -15,15 +15,23 @@ import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.semver.SemVer import scala.meta.io.AbsolutePath -import coursierapi.Dependency -import coursierapi.Fetch -import coursierapi.error.CoursierError +import coursier.Fetch +import coursier.Repositories +import coursier.core.Classifier +import coursier.core.Dependency +import coursier.core.Module +import coursier.core.ModuleName +import coursier.core.Organization +import coursier.error.CoursierError +import coursier.maven.SbtMavenRepository object JarSourcesProvider { private val sbtRegex = "sbt-(.*)".r - def fetchSources(jars: Seq[String])(implicit ec: ExecutionContext): Future[Seq[String]] = { + def fetchSources( + jars: Seq[String] + )(implicit ec: ExecutionContext): Future[Seq[String]] = { def sourcesPath(jar: String) = s"${jar.stripSuffix(".jar")}-sources.jar" val (haveSources, toDownload) = jars.partition { jar => @@ -43,23 +51,27 @@ object JarSourcesProvider { }.distinct val fetchedSources = - Future.sequence { - dependencies.map{ dep => - fetchDependencySources(dep) - .recover { - case error : CoursierError => - scribe.warn(s"Could not fetch dependency sources for $dep, error: $error") - Nil + Future + .sequence { + dependencies.map { dep => + fetchDependencySources(dep) + .recover { case error: CoursierError => + scribe.warn( + s"Could not fetch dependency sources for $dep, error: $error" + ) + Nil + } } } - }.recover { - case _: TimeoutException => - scribe.warn(s"Timeout when fetching dependency sources.") - Nil - case NonFatal(e) => - scribe.warn(s"Could not fetch dependency sources, error: $e.") - Nil - }.map(_.flatten.map(_.toUri().toString())) + .recover { + case _: TimeoutException => + scribe.warn(s"Timeout when fetching dependency sources.") + Nil + case NonFatal(e) => + scribe.warn(s"Could not fetch dependency sources, error: $e.") + Nil + } + .map(_.flatten.map(_.toUri().toString())) val existingSources = haveSources.map(sourcesPath) fetchedSources.map(_ ++ existingSources) @@ -72,7 +84,12 @@ object JarSourcesProvider { case sbtRegex(versionStr) => Try(SemVer.Version.fromString(versionStr)) match { case Success(version) if version.toString == versionStr => - Some(Dependency.of("org.scala-sbt", "sbt", versionStr)) + val module = Module( + Organization("org.scala-sbt"), + ModuleName("sbt"), + Map.empty, + ) + Some(Dependency(module, versionStr)) case _ => None } case _ => None @@ -84,37 +101,39 @@ object JarSourcesProvider { val groupId = (xml \ "groupId").text val version = (xml \ "version").text val artifactId = (xml \ "artifactId").text - Option - .when(groupId.nonEmpty && version.nonEmpty && artifactId.nonEmpty) { - Dependency.of(groupId, artifactId, version) - } - .filterNot(dep => isSbtDap(dep) || isMetalsPlugin(dep)) - } - - private def isSbtDap(dependency: Dependency) = { - dependency.getModule().getOrganization() == "ch.epfl.scala" && - dependency.getModule().getName() == "sbt-debug-adapter" && - dependency.getVersion() == BuildInfo.debugAdapterVersion + val properties = (xml \ "properties") + + def getProperty(name: String) = + properties.map(node => (node \ name).text).find(_.nonEmpty).map(name -> _) + + Option.when(groupId.nonEmpty && version.nonEmpty && artifactId.nonEmpty) { + val scalaVersion = getProperty("scalaVersion").toMap + val sbtVersion = getProperty("sbtVersion").toMap + val attributes = (scalaVersion ++ sbtVersion) + Dependency( + Module(Organization(groupId), ModuleName(artifactId), attributes), + version, + ) + } } - private def isMetalsPlugin(dependency: Dependency) = { - dependency.getModule().getOrganization() == "org.scalameta" && - dependency.getModule().getName() == "sbt-metals" && - dependency.getVersion() == BuildInfo.metalsVersion - } + private val sbtMaven = SbtMavenRepository(Repositories.central) + private val metalsPluginSnapshots = SbtMavenRepository( + Repositories.sonatype("public") + ) - private def fetchDependencySources( + def fetchDependencySources( dependency: Dependency - )(implicit ec: ExecutionContext): Future[List[Path]] = Future { - Fetch - .create() - .withDependencies(dependency) - .addClassifiers("sources") - .fetchResult() - .getFiles() - .asScala - .map(_.toPath()) - .toList - }.withTimeout(5, TimeUnit.MINUTES) + )(implicit ec: ExecutionContext): Future[List[Path]] = { + val repositories = + List(Repositories.central, sbtMaven, metalsPluginSnapshots) + Fetch() + .withRepositories(repositories) + .withDependencies(Seq(dependency)) + .addClassifiers(Classifier.sources) + .future() + .map(_.map(_.toPath()).toList) + .withTimeout(5, TimeUnit.MINUTES) + } } 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..74c44c2ea9f 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 @@ -41,7 +41,7 @@ import scala.meta.internal.metals.clients.language.MetalsLanguageClient import scala.meta.internal.process.SystemProcess import scala.meta.io.AbsolutePath -import coursier.version.Version +import coursier.core.Version // todo https://github.com/scalameta/metals/issues/4788 // clean () =>, use plain values diff --git a/tests/slow/src/test/scala/tests/scalacli/ScalaCliActionsSuite.scala b/tests/slow/src/test/scala/tests/scalacli/ScalaCliActionsSuite.scala index 6663593eb76..d91fea9bc41 100644 --- a/tests/slow/src/test/scala/tests/scalacli/ScalaCliActionsSuite.scala +++ b/tests/slow/src/test/scala/tests/scalacli/ScalaCliActionsSuite.scala @@ -6,7 +6,7 @@ import scala.meta.internal.metals.codeactions.ImportMissingSymbol import scala.meta.internal.mtags.BuildInfo.scalaCompilerVersion import scala.meta.internal.mtags.CoursierComplete -import coursier.version.Version +import coursier.core.Version class ScalaCliActionsSuite extends BaseScalaCLIActionSuite("actionableDiagnostic") { diff --git a/tests/unit/src/test/scala/tests/JarSourcesProviderSuite.scala b/tests/unit/src/test/scala/tests/JarSourcesProviderSuite.scala index 0cb72eba492..23bd43f3fd6 100644 --- a/tests/unit/src/test/scala/tests/JarSourcesProviderSuite.scala +++ b/tests/unit/src/test/scala/tests/JarSourcesProviderSuite.scala @@ -1,10 +1,16 @@ package tests import scala.concurrent.ExecutionContext +import scala.concurrent.Future import scala.meta.internal.metals.JarSourcesProvider import scala.meta.internal.metals.MetalsEnrichments._ +import coursier.core.Dependency +import coursier.core.Module +import coursier.core.ModuleName +import coursier.core.Organization + class JarSourcesProviderSuite extends BaseSuite { private implicit val ctx: ExecutionContext = this.munitExecutionContext @@ -24,4 +30,33 @@ class JarSourcesProviderSuite extends BaseSuite { } } + private val sbtDap = { + val attributes = Map("scalaVersion" -> "2.12", "sbtVersion" -> "1.0") + val module = Module( + Organization("ch.epfl.scala"), + ModuleName("sbt-debug-adapter"), + attributes, + ) + Dependency(module, "3.1.4") + } + + private val metalsSbt = { + val attributes = Map("scalaVersion" -> "2.12", "sbtVersion" -> "1.0") + val module = Module( + Organization("org.scalameta"), + ModuleName("sbt-metals"), + attributes, + ) + Dependency(module, "1.1.0") + } + + test("download-sbt-plugins") { + for { + sources <- Future.sequence( + List(sbtDap, metalsSbt).map(JarSourcesProvider.fetchDependencySources) + ) + _ = sources.foreach(sources => assert(sources.nonEmpty)) + } yield () + } + } From 43019c646ed344e0a8abb6b9c816109a74b18bc9 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Mon, 11 Dec 2023 15:20:17 +0100 Subject: [PATCH 5/5] fail fast if repositories cannot be resolved --- .../internal/metals/JarSourcesProvider.scala | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala index f245a6b6188..d27810e723e 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/JarSourcesProvider.scala @@ -1,5 +1,6 @@ package scala.meta.internal.metals +import java.net.UnknownHostException import java.nio.file.Path import java.util.concurrent.TimeUnit import java.util.concurrent.TimeoutException @@ -17,6 +18,7 @@ import scala.meta.io.AbsolutePath import coursier.Fetch import coursier.Repositories +import coursier.cache.ArtifactError import coursier.core.Classifier import coursier.core.Dependency import coursier.core.Module @@ -56,9 +58,20 @@ object JarSourcesProvider { dependencies.map { dep => fetchDependencySources(dep) .recover { case error: CoursierError => - scribe.warn( - s"Could not fetch dependency sources for $dep, error: $error" - ) + Option(error.getCause()).map(e => (e, e.getCause())) match { + case Some( + ( + _: ArtifactError.DownloadError, + e: UnknownHostException, + ) + ) => + // Fail all if repositories cannot be resolved, e.g. no internet connection. + throw e + case _ => + scribe.warn( + s"Could not fetch dependency sources for $dep, error: $error" + ) + } Nil } } @@ -67,6 +80,9 @@ object JarSourcesProvider { case _: TimeoutException => scribe.warn(s"Timeout when fetching dependency sources.") Nil + case e: UnknownHostException => + scribe.warn(s"Repository `${e.getMessage()}` is not available.") + Nil case NonFatal(e) => scribe.warn(s"Could not fetch dependency sources, error: $e.") Nil @@ -83,6 +99,8 @@ object JarSourcesProvider { filename match { case sbtRegex(versionStr) => Try(SemVer.Version.fromString(versionStr)) match { + // Since `SemVer.Version.fromString` might be able to parse strings, that are not versions, + // we check if the result version is a correct parse of the input string. case Success(version) if version.toString == versionStr => val module = Module( Organization("org.scala-sbt"), @@ -133,7 +151,7 @@ object JarSourcesProvider { .addClassifiers(Classifier.sources) .future() .map(_.map(_.toPath()).toList) - .withTimeout(5, TimeUnit.MINUTES) + .withTimeout(1, TimeUnit.MINUTES) } }