Skip to content

Commit

Permalink
fix: use semanticdb also for local and when stale if scala version do…
Browse files Browse the repository at this point in the history
…esn't support finding refs using pc
  • Loading branch information
kasiaMarek committed Aug 8, 2024
1 parent 074cf7a commit e56266a
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,8 @@ object IdentifierIndex {
) {
def asStale: MaybeStaleIndexEntry =
MaybeStaleIndexEntry(id, bloom, isStale = true)

def shouldUsePc(buildTargets: BuildTargets): Boolean =
isStale && buildTargets.supportsPcRefs(id)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand All @@ -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)
Expand All @@ -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)
}
}
Expand Down
36 changes: 36 additions & 0 deletions tests/unit/src/test/scala/tests/ReferenceLspSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit e56266a

Please sign in to comment.