From e56266a4fdaf11ab71bc64f6e66ac2c1131bdb1b Mon Sep 17 00:00:00 2001 From: kasiaMarek Date: Tue, 6 Aug 2024 10:44:43 +0200 Subject: [PATCH] fix: use semanticdb also for local and when stale if scala version doesn't support finding refs using pc --- .../meta/internal/metals/BuildTargets.scala | 13 ++++++ .../internal/metals/IdentifierIndex.scala | 3 ++ .../meta/internal/metals/MtagsResolver.scala | 2 +- .../internal/metals/ReferenceProvider.scala | 41 ++++++++++++------- .../test/scala/tests/ReferenceLspSuite.scala | 36 ++++++++++++++++ 5 files changed, 80 insertions(+), 15 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 e866d300e91..d5e670811a6 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala @@ -15,6 +15,7 @@ import scala.util.control.NonFatal import scala.meta.internal.io.PathIO import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.mtags.Symbol +import scala.meta.internal.semver.SemVer.Version import scala.meta.io.AbsolutePath import ch.epfl.scala.bsp4j.BuildTarget @@ -627,6 +628,18 @@ final class BuildTargets private ( BuildTargets.DataSeq(list.filterNot(_ == data)) } } + + def supportsPcRefs(id: BuildTargetIdentifier): Boolean = { + def scalaVersionSupportsPcReferences(scalaVersion: String) = + !scalaVersion.startsWith("3.4") && + !MtagsResolver.removedScalaVersions + .get(scalaVersion) + .exists(Version.fromString(_) < Version.fromString("1.3.2")) + + scalaTarget(id).exists(scalaTaget => + scalaVersionSupportsPcReferences(scalaTaget.scalaVersion) + ) + } } object BuildTargets { diff --git a/metals/src/main/scala/scala/meta/internal/metals/IdentifierIndex.scala b/metals/src/main/scala/scala/meta/internal/metals/IdentifierIndex.scala index bfc5747e36d..1663a20d380 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/IdentifierIndex.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/IdentifierIndex.scala @@ -68,5 +68,8 @@ object IdentifierIndex { ) { def asStale: MaybeStaleIndexEntry = MaybeStaleIndexEntry(id, bloom, isStale = true) + + def shouldUsePc(buildTargets: BuildTargets): Boolean = + isStale && buildTargets.supportsPcRefs(id) } } diff --git a/metals/src/main/scala/scala/meta/internal/metals/MtagsResolver.scala b/metals/src/main/scala/scala/meta/internal/metals/MtagsResolver.scala index 7b0f607d572..a234cc502ff 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MtagsResolver.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MtagsResolver.scala @@ -47,7 +47,7 @@ object MtagsResolver { * Map of removed Scala versions since 0.11.10. * Points to the last Metals version that supported it. */ - private val removedScalaVersions = Map( + val removedScalaVersions: Map[String, String] = Map( "2.13.1" -> "0.11.10", "2.13.2" -> "0.11.10", "2.13.3" -> "0.11.12", diff --git a/metals/src/main/scala/scala/meta/internal/metals/ReferenceProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/ReferenceProvider.scala index 7c4050d02be..6ca106c9466 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ReferenceProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ReferenceProvider.scala @@ -191,7 +191,11 @@ final class ReferenceProvider( )(implicit report: ReportContext): Future[List[ReferencesResult]] = { val source = params.getTextDocument.getUri.toAbsolutePath val textDoc = semanticdbs().textDocument(source) - textDoc.toOption match { + val supportsPcRefs = + buildTargets.inverseSources(source).exists(buildTargets.supportsPcRefs(_)) + val textDocOpt = + if (supportsPcRefs) textDoc.toOption else textDoc.documentIncludingStale + textDocOpt match { case Some(doc) => val results: List[ResolvedSymbolOccurrence] = { val posOccurrences = @@ -222,6 +226,7 @@ final class ReferenceProvider( params.getContext.isIncludeDeclaration, findRealRange, includeSynthetics, + supportsPcRefs, ).map { locations => // It's possible to return nothing is we exclude declaration if ( @@ -427,7 +432,9 @@ final class ReferenceProvider( } yield { val searchFiles = pathsMap(id).filter { searchFile => val indexedFile = index.get(searchFile.toNIO) - (indexedFile.isEmpty || indexedFile.get.isStale) && !visited( + (indexedFile.isEmpty || indexedFile.get.shouldUsePc( + buildTargets + )) && !visited( searchFile ) }.distinct @@ -509,7 +516,9 @@ final class ReferenceProvider( val visited = scala.collection.mutable.Set.empty[AbsolutePath] val result = for { (path, entry) <- index.iterator - if includeStale || !entry.isStale || path.filename.isJavaFilename + if includeStale || !entry.shouldUsePc( + buildTargets + ) || path.filename.isJavaFilename if allowedBuildTargets(entry.id) && isSymbol.exists(entry.bloom.mightContain) sourcePath = AbsolutePath(path) @@ -628,10 +637,11 @@ final class ReferenceProvider( isIncludeDeclaration: Boolean, findRealRange: AdjustRange, includeSynthetics: Synthetic => Boolean, + supportsPcRefs: Boolean, ): Future[Seq[Location]] = { val isSymbol = alternatives + occ.symbol val isLocal = occ.symbol.isLocal - if (isLocal) + if (isLocal && supportsPcRefs) compilers .references(params, EmptyCancelToken, findRealRange) .map(_.flatMap(_.locations)) @@ -643,7 +653,8 @@ final class ReferenceProvider( */ val searchLocal = source.isDependencySource(workspace) || - buildTargets.inverseSources(source).isEmpty + buildTargets.inverseSources(source).isEmpty || + isLocal val local = if (searchLocal) @@ -659,19 +670,21 @@ final class ReferenceProvider( ) else Seq.empty - val sourceContainsDefinition = + def sourceContainsDefinition = occ.role.isDefinition || snapshot.symbols.exists( _.symbol == occ.symbol ) val workspaceRefs = - workspaceReferences( - source, - isSymbol, - isIncludeDeclaration, - findRealRange, - includeSynthetics, - sourceContainsDefinition, - ) + if (isLocal) Seq.empty + else + workspaceReferences( + source, + isSymbol, + isIncludeDeclaration, + findRealRange, + includeSynthetics, + sourceContainsDefinition, + ) Future.successful(local ++ workspaceRefs) } } diff --git a/tests/unit/src/test/scala/tests/ReferenceLspSuite.scala b/tests/unit/src/test/scala/tests/ReferenceLspSuite.scala index 3157c217059..3400016e66e 100644 --- a/tests/unit/src/test/scala/tests/ReferenceLspSuite.scala +++ b/tests/unit/src/test/scala/tests/ReferenceLspSuite.scala @@ -507,6 +507,42 @@ class ReferenceLspSuite extends BaseRangesSuite("reference") { } yield () } + test("local-3.4.x") { + for { + _ <- initialize(""" + |/metals.json + |{ + | "a": { "scalaVersion": "3.4.2" } + |} + |/a/src/main/scala/a/A.scala + |package a + | + |object A { + | def foo = { + | val bar = 1 + | val g = bar + | bar + | } + |} + |""".stripMargin) + _ <- server.didOpen("a/src/main/scala/a/A.scala") + references <- server.references("a/src/main/scala/a/A.scala", "bar") + _ = assertNoDiff( + references, + """|a/src/main/scala/a/A.scala:5:9: info: reference + | val bar = 1 + | ^^^ + |a/src/main/scala/a/A.scala:6:13: info: reference + | val g = bar + | ^^^ + |a/src/main/scala/a/A.scala:7:5: info: reference + | bar + | ^^^ + |""".stripMargin, + ) + } yield () + } + override def assertCheck( filename: String, edit: String,