From bef83a50e2f6c8a13214324d49c19eda834afb5f Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Wed, 12 Aug 2026 13:46:42 +0200 Subject: [PATCH] bugfix: Index properly when dependencyModules is empty There were two issues here, which together broke go to def for dependency sources in Scala CLI: - we had a check to not run the request for Scala CLI (due to some previous native image issues) - without dependency modules nothing would be indexed Now, we remove the workaround as the Scala CLI issue is now solved, but also added a fallback just in case --- .../scala/meta/internal/metals/Indexer.scala | 69 ++++++++++++++++-- .../buildserver/BuildServerConnection.scala | 3 +- tests/unit/src/main/scala/bill/Bill.scala | 21 +++++- .../src/test/scala/tests/BillLspSuite.scala | 71 +++++++++++++++++++ 4 files changed, 155 insertions(+), 9 deletions(-) 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 423341cce57..6e61d053c07 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala @@ -248,12 +248,16 @@ case class Indexer(indexProviders: IndexProviders, mbtBuild: () => MbtBuild)( ) progress.message = s"indexing ${buildTool.importedBuild.dependencyModules.getItems().size()} dependencies" - if (indexProviders.clientConfig.definitionIndexStrategy().isClasspath) { - usedJars ++= indexDependencyModules( - buildTool.importedBuild.dependencyModules, - progress, - ) - } + val isClasspathIndexing = + indexProviders.clientConfig.definitionIndexStrategy().isClasspath + val indexedDependencyModules = + if (isClasspathIndexing) + indexDependencyModules( + buildTool.importedBuild.dependencyModules, + progress, + ) + else Set.empty[AbsolutePath] + usedJars ++= indexedDependencyModules if (shouldFallbackToFileMbt) { val build = MbtBuild.fromWorkspace(indexProviders.folder) indexDependencyModules(build.asBspModules, progress) @@ -263,6 +267,13 @@ case class Indexer(indexProviders: IndexProviders, mbtBuild: () => MbtBuild)( buildTool.importedBuild.dependencySources, progress, ) + // If no dependency modules are found, index normal classpath jars + if (indexedDependencyModules.isEmpty && isClasspathIndexing) { + usedJars ++= indexClasspathJarsFallback( + buildTool.data, + progress, + ) + } scribe.debug(s"indexed ${usedJars.size} dependency source jars") } @@ -534,6 +545,52 @@ case class Indexer(indexProviders: IndexProviders, mbtBuild: () => MbtBuild)( usedJars.toSet } + /** + * Index classpath jars if none are included in the dependency modules + */ + private def indexClasspathJarsFallback( + data: TargetData, + progress: TaskProgress, + ): Set[AbsolutePath] = { + val usedJars = mutable.HashSet.empty[AbsolutePath] + val isVisited = new ju.HashSet[AbsolutePath]() + scribe.info("Dependency modules empty, falling back to classpath jars") + for { + targetId <- data.allBuildTargetIds + jars <- data.targetJarClasspath(targetId).toList + jar <- jars + if jar.isJar && !isVisited.contains(jar) + } { + progress.progress = progress.progress + 1 + isVisited.add(jar) + usedJars += jar + + val sourcesJarName = jar.filename.stripSuffix(".jar") + "-sources.jar" + val sources = data.sourceJarNameToJarFile.get(sourcesJarName) + + if (sources.isEmpty) { + scribe.warn(s"sources jar not found for $jar") + } + + val jarName = jar.filename.stripSuffix(".jar") + // coordinates are not used anywhere currently in definitionIndex + val coordinates = MavenCoordinates("unknown", jarName, "unknown") + + val dependencyModule = DependencyModule(coordinates, jar, sources) + val dialect = buildTargets + .scalaTarget(targetId) + .map(scalaTarget => + ScalaVersions.dialectForScalaVersion( + scalaTarget.scalaVersion, + includeSource3 = true, + ) + ) + .getOrElse(Scala213) + definitionIndex.addDependencyModule(dependencyModule, dialect) + } + usedJars.toSet + } + private def processDependencyPath( path: AbsolutePath, target: b.BuildTargetIdentifier, diff --git a/metals/src/main/scala/scala/meta/internal/metals/buildserver/BuildServerConnection.scala b/metals/src/main/scala/scala/meta/internal/metals/buildserver/BuildServerConnection.scala index 81340570ab6..4455b504511 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/buildserver/BuildServerConnection.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/buildserver/BuildServerConnection.scala @@ -159,9 +159,8 @@ class BuildServerConnection private[metals] ( def isDependencySourcesSupported: Boolean = capabilities.getDependencySourcesProvider() - // Scala CLI breaks when we try to use the `buildTarget/dependencyModules` request def isDependencyModulesSupported: Boolean = - capabilities.getDependencyModulesProvider() && !isScalaCLI + capabilities.getDependencyModulesProvider() def supportsSyncMethod: Boolean = initialConnection.syncModes.isDefined diff --git a/tests/unit/src/main/scala/bill/Bill.scala b/tests/unit/src/main/scala/bill/Bill.scala index 3628e8b2229..e2f8a39ae82 100644 --- a/tests/unit/src/main/scala/bill/Bill.scala +++ b/tests/unit/src/main/scala/bill/Bill.scala @@ -169,6 +169,7 @@ object Bill { val capabilities = new BuildServerCapabilities capabilities.setCompileProvider(new CompileProvider(languages)) capabilities.setCanReload(true) + capabilities.setDependencyModulesProvider(true) new InitializeBuildResult("Bill", "1.0", "2.0.0-M2", capabilities) }.logError("initialize").asJava } @@ -401,9 +402,27 @@ object Bill { Future.successful(new ScalaMainClassesResult(List.empty.asJava)).asJava } + // Returns true when tests request a nonempty DependencyModulesResult + // whose per-target module lists are empty (classpath fallback path). + def hasEmptyDependencyModuleLists(): Boolean = { + Files.isRegularFile( + workspace.resolve("bill-empty-dependency-module-lists") + ) + } + override def buildTargetDependencyModules( params: DependencyModulesParams - ): CompletableFuture[DependencyModulesResult] = ??? + ): CompletableFuture[DependencyModulesResult] = { + CompletableFuture.completedFuture { + if (hasEmptyDependencyModuleLists()) { + val item = + new DependencyModulesItem(target.getId, Collections.emptyList()) + new DependencyModulesResult(Collections.singletonList(item)) + } else { + new DependencyModulesResult(Collections.emptyList()) + } + } + } override def debugSessionStart( params: DebugSessionParams diff --git a/tests/unit/src/test/scala/tests/BillLspSuite.scala b/tests/unit/src/test/scala/tests/BillLspSuite.scala index e8a7c0c4754..9b155ed98c8 100644 --- a/tests/unit/src/test/scala/tests/BillLspSuite.scala +++ b/tests/unit/src/test/scala/tests/BillLspSuite.scala @@ -313,4 +313,75 @@ class BillLspSuite extends BaseLspSuite("bill") { _ = assertEquals(compileReport.getStatusCode(), StatusCode.OK) } yield () } + + test("definition-without-dependency-modules") { + cleanWorkspace() + Bill.installWorkspace(workspace) + for { + _ <- initialize( + """|/src/com/App.scala + |object App { + | val list: List[Int] = List(1, 2, 3) + |} + |""".stripMargin + ) + _ <- server.didOpen("src/com/App.scala") + _ = assertNoDiff(client.workspaceDiagnostics, "") + locations <- server.definition( + "src/com/App.scala", + """|object App { + | val list: Li@@st[Int] = List(1, 2, 3) + |} + |""".stripMargin, + workspace, + ) + _ = assert( + locations.nonEmpty, + "Expected definition location for List but got none. " + + "This tests the fallback to classpath jars when dependency modules are empty.", + ) + uri = locations.head.getUri() + _ = assert( + uri.contains("scala-library"), + s"Expected definition to be in scala-library, but got: $uri", + ) + } yield () + } + + test("definition-with-empty-dependency-module-lists") { + cleanWorkspace() + Bill.installWorkspace(workspace) + for { + _ <- initialize( + """|/src/com/App.scala + |object App { + | val list: List[Int] = List(1, 2, 3) + |} + |/bill-empty-dependency-module-lists + |true + |""".stripMargin + ) + _ <- server.didOpen("src/com/App.scala") + _ = assertNoDiff(client.workspaceDiagnostics, "") + locations <- server.definition( + "src/com/App.scala", + """|object App { + | val list: Li@@st[Int] = List(1, 2, 3) + |} + |""".stripMargin, + workspace, + ) + _ = assert( + locations.nonEmpty, + "Expected definition location for List but got none. " + + "This tests classpath jar fallback when DependencyModulesResult " + + "has items but empty module lists.", + ) + uri = locations.head.getUri() + _ = assert( + uri.contains("scala-library"), + s"Expected definition to be in scala-library, but got: $uri", + ) + } yield () + } }