From 2d053550f07388b47ce1f9bbef6ec232ef059dbc Mon Sep 17 00:00:00 2001 From: kasiaMarek Date: Wed, 21 Aug 2024 15:14:32 +0200 Subject: [PATCH 1/3] improvement: if no binary version in jar path try using build target info --- .../ImplementationProvider.scala | 3 ++- .../scala/meta/internal/metals/Indexer.scala | 4 +-- .../meta/internal/metals/ScalaVersions.scala | 27 ++++++++++++++----- .../metals/StandaloneSymbolSearch.scala | 5 +++- .../internal/tvp/MetalsTreeViewProvider.scala | 7 +++-- .../scala/tests/MetalsTestEnrichments.scala | 6 ++++- .../test/scala/tests/DefinitionSuite.scala | 8 +++++- .../test/scala/tests/ScalaVersionsSuite.scala | 2 +- 8 files changed, 44 insertions(+), 18 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/implementation/ImplementationProvider.scala b/metals/src/main/scala/scala/meta/internal/implementation/ImplementationProvider.scala index 7174d333902..1ae18259e75 100644 --- a/metals/src/main/scala/scala/meta/internal/implementation/ImplementationProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/implementation/ImplementationProvider.scala @@ -396,7 +396,8 @@ final class ImplementationProvider( } private def semanticdbForJarFile(fileSource: AbsolutePath) = { - val dialect = ScalaVersions.dialectForDependencyJar(fileSource.filename) + val dialect = + ScalaVersions.dialectForDependencyJar(fileSource, buildTargets) FileIO.slurp(fileSource, StandardCharsets.UTF_8) val textDocument = Mtags.index(fileSource, dialect) textDocument diff --git a/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala b/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala index d9d8dcfa93b..90922ae0f92 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala @@ -415,7 +415,7 @@ case class Indexer(indexProviders: IndexProviders)(implicit rc: ReportContext) { case Right(zip) => scribe.debug(s"Indexing JDK sources from $zip") usedJars += zip - val dialect = ScalaVersions.dialectForDependencyJar(zip.filename) + val dialect = ScalaVersions.dialectForDependencyJar(zip, buildTargets) sharedIndices.jvmTypeHierarchy.getTypeHierarchy(zip) match { case Some(overrides) => definitionIndex.addIndexedSourceJar(zip, Nil, dialect) @@ -540,7 +540,7 @@ case class Indexer(indexProviders: IndexProviders)(implicit rc: ReportContext) { * @param path JAR path */ private def addSourceJarSymbols(path: AbsolutePath): Unit = { - val dialect = ScalaVersions.dialectForDependencyJar(path.filename) + val dialect = ScalaVersions.dialectForDependencyJar(path, buildTargets) tables.jarSymbols.getTopLevels(path) match { case Some(toplevels) => tables.jarSymbols.getTypeHierarchy(path) match { diff --git a/metals/src/main/scala/scala/meta/internal/metals/ScalaVersions.scala b/metals/src/main/scala/scala/meta/internal/metals/ScalaVersions.scala index ea87d13847e..4eec0386ef2 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ScalaVersions.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ScalaVersions.scala @@ -3,6 +3,7 @@ package scala.meta.internal.metals import scala.meta.Dialect import scala.meta.dialects._ import scala.meta.internal.semver.SemVer +import scala.meta.io.AbsolutePath class ScalaVersions( deprecatedScalaVersions: Seq[String], @@ -157,7 +158,7 @@ class ScalaVersions( * `some-library_2.13-4.5.0` -> 2.13 * `some-library_2.13-2.11` -> 2.13 */ - def scalaBinaryVersionFromJarName(filename: String): String = { + def scalaBinaryVersionFromJarName(filename: String): Option[String] = { val dropEnding = filename .stripSuffix(".jar") @@ -178,14 +179,26 @@ class ScalaVersions( .sortBy(_._2)(Ordering.Boolean.reverse) .headOption .map { case (version, _) => scalaBinaryVersionFromFullVersion(version) } - .getOrElse("2.13") } - def dialectForDependencyJar(filename: String): Dialect = - dialectForScalaVersion( - scalaBinaryVersionFromJarName(filename), - includeSource3 = true, - ) + def dialectForDependencyJar( + jar: AbsolutePath, + buildTargets: BuildTargets, + ): Dialect = { + val scalaVersion = + scalaBinaryVersionFromJarName(jar.toNIO.getFileName().toString()) + .orElse( + buildTargets + .inverseDependencySource(jar) + .flatMap(id => buildTargets.scalaTarget(id)) + .map(_.scalaBinaryVersion) + .toList + .sorted + .headOption + ) + .getOrElse("2.13") + dialectForScalaVersion(scalaVersion, includeSource3 = true) + } } diff --git a/metals/src/main/scala/scala/meta/internal/metals/StandaloneSymbolSearch.scala b/metals/src/main/scala/scala/meta/internal/metals/StandaloneSymbolSearch.scala index c9f257dabe1..77834b033cf 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/StandaloneSymbolSearch.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/StandaloneSymbolSearch.scala @@ -44,7 +44,10 @@ class StandaloneSymbolSearch( private val index = OnDemandSymbolIndex.empty() sources.foreach(s => - index.addSourceJar(s, ScalaVersions.dialectForDependencyJar(s.filename)) + index.addSourceJar( + s, + ScalaVersions.dialectForDependencyJar(s, buildTargets), + ) ) private val docs = new Docstrings(index) 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 7ffb0429971..dc53d989fed 100644 --- a/metals/src/main/scala/scala/meta/internal/tvp/MetalsTreeViewProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/tvp/MetalsTreeViewProvider.scala @@ -132,9 +132,8 @@ class MetalsTreeViewProvider( val dialect = if (path.isJarFileSystem) { path.jarPath - .map { p => - ScalaVersions.dialectForDependencyJar(p.filename) - } + .flatMap(p => ScalaVersions.scalaBinaryVersionFromJarName(p.filename)) + .map(ScalaVersions.dialectForScalaVersion(_, includeSource3 = true)) .getOrElse(dialects.Scala3) } else dialectFromWorkspace val input = path.toInput @@ -264,7 +263,7 @@ class FolderTreeViewProvider( valueTooltip = _.toString, toplevels = () => buildTargets.allSourceJars.filter(_.exists), loadSymbols = (path, symbol) => { - val dialect = ScalaVersions.dialectForDependencyJar(path.filename) + val dialect = ScalaVersions.dialectForDependencyJar(path, buildTargets) classpath.jarSymbols(path, symbol, dialect) }, toplevelIcon = "package", diff --git a/tests/unit/src/main/scala/tests/MetalsTestEnrichments.scala b/tests/unit/src/main/scala/tests/MetalsTestEnrichments.scala index bd36280e4ac..024d90653d6 100644 --- a/tests/unit/src/main/scala/tests/MetalsTestEnrichments.scala +++ b/tests/unit/src/main/scala/tests/MetalsTestEnrichments.scala @@ -81,7 +81,11 @@ object MetalsTestEnrichments { } libraries.foreach( _.sources.entries.foreach { s => - val dialect = ScalaVersions.dialectForDependencyJar(s.filename) + val version = ScalaVersions + .scalaBinaryVersionFromJarName(s.filename) + .getOrElse("2.13") + val dialect = + ScalaVersions.dialectForScalaVersion(version, includeSource3 = true) wsp.index.addSourceJar(s, dialect) } ) diff --git a/tests/unit/src/test/scala/tests/DefinitionSuite.scala b/tests/unit/src/test/scala/tests/DefinitionSuite.scala index a9f838bd8de..3a2b2f98d59 100644 --- a/tests/unit/src/test/scala/tests/DefinitionSuite.scala +++ b/tests/unit/src/test/scala/tests/DefinitionSuite.scala @@ -45,9 +45,15 @@ abstract class DefinitionSuiteBase( // Step 2. Index dependency sources index.addSourceJar(JdkSources().right.get, dialect) input.dependencySources.entries.foreach { jar => + val scalaVersion = ScalaVersions + .scalaBinaryVersionFromJarName(jar.filename) + .getOrElse("2.13") index.addSourceJar( jar, - ScalaVersions.dialectForDependencyJar(jar.filename), + ScalaVersions.dialectForScalaVersion( + scalaVersion, + includeSource3 = true, + ), ) } diff --git a/tests/unit/src/test/scala/tests/ScalaVersionsSuite.scala b/tests/unit/src/test/scala/tests/ScalaVersionsSuite.scala index 86777b76a1f..57c0740793c 100644 --- a/tests/unit/src/test/scala/tests/ScalaVersionsSuite.scala +++ b/tests/unit/src/test/scala/tests/ScalaVersionsSuite.scala @@ -316,7 +316,7 @@ class ScalaVersionsSuite extends BaseSuite { checkJar("tested-3.0-sources.jar", "2.13") def checkJar(jar: String, expected: String): Unit = test(jar) { - val out = ScalaVersions.scalaBinaryVersionFromJarName(jar) + val out = ScalaVersions.scalaBinaryVersionFromJarName(jar).getOrElse("2.13") assertEquals(out, expected, jar) } From 087940c40b8a2de372bdc4bc760209ab037fff6e Mon Sep 17 00:00:00 2001 From: kasiaMarek Date: Thu, 22 Aug 2024 11:21:18 +0200 Subject: [PATCH 2/3] special case for scala lib and look for tasty files when discovering scala version for source jar --- .../meta/internal/metals/BuildTargets.scala | 23 +++++-- .../meta/internal/metals/ScalaVersions.scala | 69 +++++++++++++------ .../meta/internal/metals/TargetData.scala | 11 +-- 3 files changed, 74 insertions(+), 29 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala b/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala index 9226f663cff..2f9b1aa188d 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala @@ -434,10 +434,14 @@ final class BuildTargets private ( * @return path to the source jar for that jar */ private def sourceJarPathFallback( - sourceJarPath: AbsolutePath + sourceJarPath: AbsolutePath, + inputIsSourceJar: Boolean = false, ): Option[AbsolutePath] = { + val (from, to) = + if (inputIsSourceJar) ("-sources.jar", ".jar") + else (".jar", "-sources.jar") val fallback = sourceJarPath.parent.resolve( - sourceJarPath.filename.replace(".jar", "-sources.jar") + sourceJarPath.filename.replace(from, to) ) if (fallback.exists) Some(fallback) else None @@ -578,7 +582,7 @@ final class BuildTargets private ( jar: AbsolutePath, ): Option[AbsolutePath] = { data - .fromOptions(_.findSourceJarOf(jar, Some(id))) + .fromOptions(_.findConnectedArtifact(jar, Some(id))) .orElse(sourceJarPathFallback(jar)) } @@ -586,10 +590,21 @@ final class BuildTargets private ( jar: AbsolutePath ): Option[AbsolutePath] = { data - .fromOptions(_.findSourceJarOf(jar, targetId = None)) + .fromOptions(_.findConnectedArtifact(jar, targetId = None)) .orElse(sourceJarPathFallback(jar)) } + def findJarFor( + id: BuildTargetIdentifier, + sourceJar: AbsolutePath, + ): Option[AbsolutePath] = { + data + .fromOptions( + _.findConnectedArtifact(sourceJar, Some(id), classifier = null) + ) + .orElse(sourceJarPathFallback(sourceJar, inputIsSourceJar = true)) + } + def inverseDependencySource( sourceJar: AbsolutePath ): collection.Set[BuildTargetIdentifier] = { diff --git a/metals/src/main/scala/scala/meta/internal/metals/ScalaVersions.scala b/metals/src/main/scala/scala/meta/internal/metals/ScalaVersions.scala index 4eec0386ef2..c3477fa3977 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ScalaVersions.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ScalaVersions.scala @@ -1,7 +1,13 @@ package scala.meta.internal.metals +import java.io.UncheckedIOException + +import scala.collection.concurrent.TrieMap + import scala.meta.Dialect import scala.meta.dialects._ +import scala.meta.internal.io.FileIO +import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.semver.SemVer import scala.meta.io.AbsolutePath @@ -14,6 +20,8 @@ class ScalaVersions( scala3: String, ) { + private val jarScalaVersionIndex = TrieMap[String, String]() + def isScala3Milestone(version: String): Boolean = version.startsWith("3.0.0-M") || version.startsWith("3.0.0-RC") @@ -148,7 +156,9 @@ class ScalaVersions( } private val scalaVersionRegex = - raw"(_)(\d)(\.\d{1,2})?(\.\d(-(RC|M)\d)?)?".r + raw"_(\d)(\.\d{1,2})?(\.\d(-(RC|M)\d)?)?".r + private val scalaLibraryRegex = + raw"scala-library-(\d)(\.\d{1,2})(\.\d(-(RC|M)\d)?)".r /** * Extract scala binary version from dependency jar name. @@ -162,40 +172,59 @@ class ScalaVersions( val dropEnding = filename .stripSuffix(".jar") - scalaVersionRegex - .findAllMatchIn(dropEnding) - .toList + List(scalaLibraryRegex, scalaVersionRegex) + .flatMap(_.findAllMatchIn(dropEnding).toList) .flatMap { m => - val hasUnderscorePrefix = Option(m.group(1)).isDefined - val major = m.group(2) - val minor = Option(m.group(3)).getOrElse("") - val ending = Option(m.group(4)).getOrElse("") + val major = m.group(1) + val minor = Option(m.group(2)).getOrElse("") + val ending = Option(m.group(3)).getOrElse("") val version = s"$major$minor$ending" if (isSupportedScalaBinaryVersion(version)) - Some(version -> hasUnderscorePrefix) + Some(version) else None } - .sortBy(_._2)(Ordering.Boolean.reverse) .headOption - .map { case (version, _) => scalaBinaryVersionFromFullVersion(version) } + .map(scalaBinaryVersionFromFullVersion) } def dialectForDependencyJar( jar: AbsolutePath, buildTargets: BuildTargets, ): Dialect = { + lazy val buildTargetAndScalaVersion = + buildTargets + .inverseDependencySource(jar) + .flatMap(id => buildTargets.scalaTarget(id)) + .map(target => (target.scalaBinaryVersion, target.id)) + .toList + .sortBy(_._1) + .headOption + + def fromTastyExistance = { + val fromTasty = buildTargetAndScalaVersion + .flatMap { case (_, id) => buildTargets.findJarFor(id, jar) } + .flatMap( + FileIO.withJarFileSystem(_, create = false) { root => + try { + root.listRecursive + .find(f => f.isFile && f.filename.endsWith(".tasty")) + .map(_ => "3") + .orElse(Some("2.13")) + } catch { + case _: UncheckedIOException => None + } + } + ) + fromTasty.foreach(jarScalaVersionIndex.put(jar.toURI.toString(), _)) + fromTasty + } + val scalaVersion = scalaBinaryVersionFromJarName(jar.toNIO.getFileName().toString()) - .orElse( - buildTargets - .inverseDependencySource(jar) - .flatMap(id => buildTargets.scalaTarget(id)) - .map(_.scalaBinaryVersion) - .toList - .sorted - .headOption - ) + .orElse(jarScalaVersionIndex.get(jar.toURI.toString())) + .orElse(fromTastyExistance) + .orElse(buildTargetAndScalaVersion.map(_._1)) .getOrElse("2.13") dialectForScalaVersion(scalaVersion, includeSource3 = true) } 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 b3ee4f757f5..bb868f60409 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/TargetData.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/TargetData.scala @@ -206,9 +206,10 @@ final class TargetData { } } - def findSourceJarOf( + def findConnectedArtifact( jar: AbsolutePath, targetId: Option[BuildTargetIdentifier], + classifier: String = "sources", ): Option[AbsolutePath] = { val jarUri = jar.toURI.toString() def depModules: Iterator[MavenDependencyModule] = targetId match { @@ -228,10 +229,10 @@ final class TargetData { module <- depModules artifacts = module.getArtifacts().asScala if artifacts.exists(artifact => isUriEqual(artifact.getUri(), jarUri)) - sourceJar <- artifacts.find(_.getClassifier() == "sources") - sourceJarPath = sourceJar.getUri().toAbsolutePath - if sourceJarPath.exists - } yield sourceJarPath + foundJar <- artifacts.find(_.getClassifier() == classifier) + foundJarPath = foundJar.getUri().toAbsolutePath + if foundJarPath.exists + } yield foundJarPath allFound.headOption } From 4cb42b6a73e627edfff7648597be5d65c4ece954 Mon Sep 17 00:00:00 2001 From: kasiaMarek Date: Mon, 2 Dec 2024 10:26:55 +0100 Subject: [PATCH 3/3] refactor: create new method --- .../meta/internal/metals/BuildTargets.scala | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala b/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala index 2f9b1aa188d..476c37a6ba6 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala @@ -430,18 +430,31 @@ final class BuildTargets private ( * in other capacity than as a fallback, since both source jar * and normal jar might not be in the same directory. * - * @param sourceJarPath path to the nromaljar + * @param sourceJarPath path to the normal jar * @return path to the source jar for that jar */ private def sourceJarPathFallback( - sourceJarPath: AbsolutePath, - inputIsSourceJar: Boolean = false, - ): Option[AbsolutePath] = { - val (from, to) = - if (inputIsSourceJar) ("-sources.jar", ".jar") - else (".jar", "-sources.jar") - val fallback = sourceJarPath.parent.resolve( - sourceJarPath.filename.replace(from, to) + sourceJarPath: AbsolutePath + ): Option[AbsolutePath] = + findInTheSameDirectoryWithReplace(sourceJarPath, ".jar", "-sources.jar") + + /** + * Try to resolve path to normal jar from source jar, + * should only be used as fallback, since jar and source jar + * do not have to be in the same directory. + */ + private def jarFromSourceJarFallback( + jarPath: AbsolutePath + ): Option[AbsolutePath] = + findInTheSameDirectoryWithReplace(jarPath, "-sources.jar", ".jar") + + private def findInTheSameDirectoryWithReplace( + path: AbsolutePath, + fromPattern: String, + toPattern: String, + ) = { + val fallback = path.parent.resolve( + path.filename.replace(fromPattern, toPattern) ) if (fallback.exists) Some(fallback) else None @@ -602,7 +615,7 @@ final class BuildTargets private ( .fromOptions( _.findConnectedArtifact(sourceJar, Some(id), classifier = null) ) - .orElse(sourceJarPathFallback(sourceJar, inputIsSourceJar = true)) + .orElse(jarFromSourceJarFallback(sourceJar)) } def inverseDependencySource(