make sure found implementations are within build target

scalameta · Jan 16, 2024 · 1d86b8e · 1d86b8e
1 parent ecdc166
commit 1d86b8e
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 32 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
@@ -11,6 +11,7 @@ import scala.concurrent.Future
 
 import scala.meta.internal.io.FileIO
 import scala.meta.internal.metals.Buffers
+import scala.meta.internal.metals.BuildTargets
 import scala.meta.internal.metals.Compilers
 import scala.meta.internal.metals.DefinitionProvider
 import scala.meta.internal.metals.MetalsEnrichments._
@@ -42,18 +43,20 @@ import scala.meta.internal.semanticdb.TypeRef
 import scala.meta.internal.semanticdb.TypeSignature
 import scala.meta.io.AbsolutePath
 
+import ch.epfl.scala.bsp4j.BuildTargetIdentifier
 import org.eclipse.lsp4j.Location
 import org.eclipse.lsp4j.TextDocumentPositionParams
 
 final class ImplementationProvider(
-    semanticdbs: Semanticdbs, 
+    semanticdbs: Semanticdbs,
     workspace: AbsolutePath,
     index: GlobalSymbolIndex,
     buffer: Buffers,
     definitionProvider: DefinitionProvider,
     trees: Trees,
     scalaVersionSelector: ScalaVersionSelector,
     compilers: Compilers,
+    buildTargets: BuildTargets
 )(implicit ec: ExecutionContext, rc: ReportContext)
     extends SemanticdbFeatureProvider {
   import ImplementationProvider._
@@ -169,21 +172,22 @@ final class ImplementationProvider(
               _.path.isWorkspaceSource(workspace)
             )
 
+        val workspaceInheritanceContext: InheritanceContext =
+          InheritanceContext.fromDefinitions(
+            implementationsInPath.asScala.toMap
+          )
+
         val inheritanceContext: InheritanceContext =
-          if (!isWorkspaceSymbol) {
-            // symbol is not in workspace, we search both workspace and dependencies for it
-            InheritanceContext
-              .fromDefinitions(implementationsInPath.asScala.toMap)
+          if (isWorkspaceSymbol) workspaceInheritanceContext
+          else
+            // symbol is not defined in the workspace, we search both workspace and dependencies for it
+            workspaceInheritanceContext
               .toGlobal(
                 compilers,
                 implementationsInDependencySources.asScala.toMap,
+                source,
               )
-          } else {
-            // symbol in workspace we search the workspace for it only
-            InheritanceContext.fromDefinitions(
-              implementationsInPath.asScala.toMap
-            )
-          }
+
         symbolLocationsFromContext(
           dealisedSymbol,
           currentDocument,
@@ -210,6 +214,7 @@ final class ImplementationProvider(
         classSymbol: String,
         textDocument: TextDocument,
         implReal: ClassLocation,
+        source: AbsolutePath,
     ): Option[Future[String]] = {
       if (classLikeKinds(info.kind)) Some(Future(implReal.symbol))
       else {
@@ -231,18 +236,14 @@ final class ImplementationProvider(
         def pcSearch = {
           val symbol =
             s"${implReal.symbol}${info.symbol.stripPrefix(classSymbol)}"
-          implReal.file
-            .map(path =>
-              compilers
-                .infoAll(AbsolutePath(path), symbol)
-                .map { allFound =>
-                  allFound
-                    .find(implInfo => implInfo.overridden.contains(info.symbol))
-                    .map(_.symbol)
-                    .getOrElse(symbol)
-                }
-            )
-            .getOrElse(Future.successful(symbol))
+          compilers
+            .infoAll(source, symbol)
+            .map { allFound =>
+              allFound
+                .find(implInfo => implInfo.overridden.contains(info.symbol))
+                .map(_.symbol)
+                .getOrElse(symbol)
+            }
         }
         tryFromDoc.orElse {
           if (implReal.symbol.isLocal) None
@@ -258,11 +259,13 @@ final class ImplementationProvider(
         locationsByFile: Map[Path, Set[ClassLocation]],
         parentSymbol: PcSymbolInformation,
         classSymbol: String,
+        buildTarget: BuildTargetIdentifier
     ) = Future.sequence({
       for {
         file <- files
         locations = locationsByFile(file)
         implPath = AbsolutePath(file)
+        if(buildTargets.belongsToBuildTarget(buildTarget, implPath))
         implDocument <- findSemanticdb(implPath).toList
       } yield {
         for {
@@ -273,6 +276,7 @@ final class ImplementationProvider(
                 classSymbol,
                 implDocument,
                 _,
+                source,
               )
             )
           )
@@ -285,7 +289,7 @@ final class ImplementationProvider(
               implDocument,
               sym,
               implPath,
-              scalaVersionSelector
+              scalaVersionSelector,
             ).toList
             range <- implOccurrence.range
             revised <-
@@ -310,6 +314,7 @@ final class ImplementationProvider(
         (for {
           symbolInfo <- optSymbolInfo
           symbolClass <- classFromSymbol(symbolInfo)
+          target <- buildTargets.inverseSources(source)
         } yield {
           for {
             locationsByFile <- findImplementation(
@@ -328,6 +333,7 @@ final class ImplementationProvider(
                     locationsByFile,
                     symbolInfo,
                     symbolClass,
+                    target
                   )
                 )
               )

diff --git a/metals/src/main/scala/scala/meta/internal/implementation/InheritanceContext.scala b/metals/src/main/scala/scala/meta/internal/implementation/InheritanceContext.scala
@@ -25,17 +25,20 @@ class InheritanceContext(inheritance: Map[String, Set[ClassLocation]]) {
   def toGlobal(
       compilers: Compilers,
       implementationsInDependencySources: Map[String, Set[ClassLocation]],
+      source: AbsolutePath
   ) = new GlobalInheritanceContext(
     compilers,
     implementationsInDependencySources,
     inheritance,
+    source
   )
 }
 
 class GlobalInheritanceContext(
     compilers: Compilers,
     implementationsInDependencySources: Map[String, Set[ClassLocation]],
     localInheritance: Map[String, Set[ClassLocation]],
+    source: AbsolutePath
 ) extends InheritanceContext(localInheritance) {
   override def getLocations(
       symbol: String
@@ -48,8 +51,8 @@ class GlobalInheritanceContext(
     val resolveGlobal =
       implementationsInDependencySources
         .getOrElse(shortName, Set.empty)
-        .collect { case loc @ ClassLocation(sym, Some(file)) =>
-          compilers.info(AbsolutePath(file), sym).map { 
+        .collect { case loc @ ClassLocation(sym, _) =>
+          compilers.info(source, sym).map { 
             case Some(symInfo) if symInfo.parents.contains(symbol) => Some(loc)
             case Some(symInfo) if symInfo.dealisedSymbol == symbol && symInfo.symbol != symbol => Some(loc)
             case _ => None

diff --git a/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala b/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala
@@ -240,6 +240,16 @@ final class BuildTargets private (
     }
   }
 
+  def inverseSourcesAll(
+      source: AbsolutePath
+  ): Iterable[BuildTargetIdentifier] = {
+    val buildTargets = sourceBuildTargets(source)
+    val orSbtBuildTarget =
+      buildTargets.getOrElse(sbtBuildScalaTarget(source).toIterable)
+    if (orSbtBuildTarget.isEmpty) inferBuildTargets(source)
+    else orSbtBuildTarget
+  }
+
   def inverseSourcesBsp(
       source: AbsolutePath
   )(implicit ec: ExecutionContext): Future[Option[BuildTargetIdentifier]] = {
@@ -308,12 +318,16 @@ final class BuildTargets private (
    */
   def inferBuildTarget(
       source: AbsolutePath
-  ): Option[BuildTargetIdentifier] = {
+  ): Option[BuildTargetIdentifier] = inferBuildTargets(source).headOption
+
+  def inferBuildTargets(
+      source: AbsolutePath
+  ): Iterable[BuildTargetIdentifier] = {
     if (source.isJarFileSystem) {
       for {
-        jarName <- source.jarPath.map(_.filename)
-        sourceJarFile <- sourceJarFile(jarName)
-        buildTargetId <- inverseDependencySource(sourceJarFile).headOption
+        jarName <- source.jarPath.map(_.filename).toIterable
+        sourceJarFile <- sourceJarFile(jarName).toIterable
+        buildTargetId <- inverseDependencySource(sourceJarFile)
       } yield buildTargetId
     } else {
       val readonly = workspace.resolve(Directories.readonly)
@@ -323,8 +337,8 @@ final class BuildTargets private (
           names match {
             case Directories.dependenciesName :: jarName :: _ =>
               // match build target by source jar name
-              sourceJarFile(jarName)
-                .flatMap(inverseDependencySource(_).headOption)
+              sourceJarFile(jarName).toList
+                .flatMap(inverseDependencySource(_))
             case _ => None
           }
         case None =>
@@ -343,6 +357,14 @@ final class BuildTargets private (
     }
   }
 
+  def belongsToBuildTarget(
+    target: BuildTargetIdentifier,
+    path: AbsolutePath
+  ): Boolean = {
+    val possibleBuildTargets = target :: buildTargetTransitiveDependencies(target).toList
+    inverseSourcesAll(path).exists(possibleBuildTargets.contains)
+  }
+
   def findByDisplayName(name: String): Option[BuildTarget] = {
     data
       .fromIterators(_.buildTargetInfo.valuesIterator)

diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala
@@ -655,6 +655,7 @@ class MetalsLspService(
       trees,
       scalaVersionSelector,
       compilers,
+      buildTargets
     )
 
   private val symbolHierarchyOps: SymbolHierarchyOps =