Add option --allow-exporting-global-symbols-from-directory-entries

Previously, scip-java only allowed cross-index (or cross-repo)
navigation between symbols that are compiled to classfiles inside jar
files. This behavior mixed an intentional design decision with an
accidental implementation detail. The intentional design decision was
that the ideal cross-repo navigation experience is from repositories to
packages. To achieve this behavior, scip-java didn't assign a package
name and package version to symbols that are compiled to directory
entries on the classpath.  The accidental implementation detail was that
Gradle, Maven, and sbt compile to directories while packages only have
jar files.

This commit changes the behavior to treat symbols the same regardless if
they are compiled to jar files or directories. The accidental
implemenation detail was a hack, and this behavior has come up as a
surprise in at least twice cases of trying to integrate scip-java with
big propriatery codebases. To get the default old behavior, the flag
`--allow-exporting-global-symbols-from-directory-entries` can be set to
false.

Author: olafurpg

scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/ClasspathEntry.scala

@@ -6,6 +6,7 @@ import java.nio.file.Files
 import java.nio.file.Path
 import java.nio.file.Paths
 
+import scala.annotation.tailrec
 import scala.jdk.CollectionConverters._
 
 import com.sourcegraph.scip_semanticdb.MavenPackage
@@ -15,15 +16,15 @@ import com.sourcegraph.scip_semanticdb.MavenPackage
  * "packageInformation" nodes.
  */
 case class ClasspathEntry(
-    jar: Path,
+    entry: Path,
     sources: Option[Path],
     groupId: String,
     artifactId: String,
     version: String
 ) {
   def toPackageHubId: String = s"maven:$groupId:$artifactId:$version"
   def toPackageInformation: MavenPackage =
-    new MavenPackage(jar, groupId, artifactId, version)
+    new MavenPackage(entry, groupId, artifactId, version)
 }
 
 object ClasspathEntry {
@@ -39,13 +40,16 @@ object ClasspathEntry {
    * @param targetroot
    *   @return
    */
-  def fromTargetroot(targetroot: Path): List[ClasspathEntry] = {
+  def fromTargetroot(
+      targetroot: Path,
+      sourceroot: Path
+  ): List[ClasspathEntry] = {
     val javacopts = targetroot.resolve("javacopts.txt")
     val dependencies = targetroot.resolve("dependencies.txt")
     if (Files.isRegularFile(dependencies)) {
       fromDependencies(dependencies)
     } else if (Files.isRegularFile(javacopts)) {
-      fromJavacopts(javacopts)
+      fromJavacopts(javacopts, sourceroot)
     } else {
       Nil
     }
@@ -55,17 +59,18 @@ object ClasspathEntry {
    * Parses ClasspathEntry from a "dependencies.txt" file in the targetroot.
    *
    * Every line of the file is a tab separated value with the following columns:
-   * groupId, artifactId, version, path to the jar file.
+   * groupId, artifactId, version, path to the jar file OR classes directory
+   * path.
    */
   private def fromDependencies(dependencies: Path): List[ClasspathEntry] = {
     Files
       .readAllLines(dependencies, StandardCharsets.UTF_8)
       .asScala
       .iterator
       .map(_.split("\t"))
-      .collect { case Array(groupId, artifactId, version, jar) =>
+      .collect { case Array(groupId, artifactId, version, entry) =>
         ClasspathEntry(
-          jar = Paths.get(jar),
+          entry = Paths.get(entry),
           sources = None,
           groupId = groupId,
           artifactId = artifactId,
@@ -81,36 +86,69 @@ object ClasspathEntry {
    * Every line of the file represents a Java compiler options, such as
    * "-classpath" or "-encoding".
    */
-  private def fromJavacopts(javacopts: Path): List[ClasspathEntry] = {
+  private def fromJavacopts(
+      javacopts: Path,
+      sourceroot: Path
+  ): List[ClasspathEntry] = {
     Files
       .readAllLines(javacopts, StandardCharsets.UTF_8)
       .asScala
       .iterator
       .map(_.stripPrefix("\"").stripSuffix("\""))
       .sliding(2)
-      .collect { case Seq("-cp" | "-classpath", classpath) =>
-        classpath.split(File.pathSeparator).iterator
+      .collect {
+        case Seq("-d", classesDirectory) =>
+          fromClassesDirectory(Paths.get(classesDirectory), sourceroot).toList
+        case Seq("-cp" | "-classpath", classpath) =>
+          classpath
+            .split(File.pathSeparator)
+            .iterator
+            .map(Paths.get(_))
+            .flatMap(ClasspathEntry.fromClasspathJarFile)
+            .toList
       }
       .flatten
-      .map(Paths.get(_))
-      .toSet
-      .iterator
-      .flatMap(ClasspathEntry.fromPom)
       .toList
   }
 
+  private def fromClassesDirectory(
+      classesDirectory: Path,
+      sourceroot: Path
+  ): Option[ClasspathEntry] = {
+    @tailrec
+    def loop(dir: Path): Option[ClasspathEntry] = {
+      if (dir == null || !dir.startsWith(sourceroot))
+        return None
+      fromPomXml(dir.resolve("pom.xml"), classesDirectory, None) match {
+        case None =>
+          loop(dir.getParent())
+        case Some(value) =>
+          Some(value)
+      }
+    }
+    loop(classesDirectory.getParent())
+  }
+
   /**
    * Tries to parse a ClasspathEntry from the POM file that lies next to the
    * given jar file.
    */
-  private def fromPom(jar: Path): Option[ClasspathEntry] = {
+  private def fromClasspathJarFile(jar: Path): Option[ClasspathEntry] = {
     val pom = jar
       .resolveSibling(jar.getFileName.toString.stripSuffix(".jar") + ".pom")
     val sources = Option(
       jar.resolveSibling(
         jar.getFileName.toString.stripSuffix(".jar") + ".sources"
       )
     ).filter(Files.isRegularFile(_))
+    fromPomXml(pom, jar, sources)
+  }
+
+  private def fromPomXml(
+      pom: Path,
+      classpathEntry: Path,
+      sources: Option[Path]
+  ): Option[ClasspathEntry] = {
     if (Files.isRegularFile(pom)) {
       val xml = scala.xml.XML.loadFile(pom.toFile)
       def xmlValue(key: String): String = {
@@ -123,7 +161,9 @@ object ClasspathEntry {
       val groupId = xmlValue("groupId")
       val artifactId = xmlValue("artifactId")
       val version = xmlValue("version")
-      Some(ClasspathEntry(jar, sources, groupId, artifactId, version))
+      Some(
+        ClasspathEntry(classpathEntry, sources, groupId, artifactId, version)
+      )
     } else {
       None
     }

scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexSemanticdbCommand.scala

@@ -52,6 +52,13 @@ final case class IndexSemanticdbCommand(
       "If true, don't report an error when no documents have been indexed. " +
         "The resulting SCIP index will silently be empty instead."
     ) allowEmptyIndex: Boolean = false,
+    @Description(
+      "Determines how to index symbols that are compiled to classfiles inside directories. " +
+        "If true, symbols inside directory entries are allowed to be publicly visible outside of the generated SCIP index. " +
+        "If false, symbols inside directory entries are only visible inside the generated SCIP index. " +
+        "The practical consequences of making this flag false is that cross-index (or cross-repository) navigation does not work between " +
+        "Maven->Maven or Gradle->Gradle projects because those build tools compile sources to classfiles izelinside directories."
+    ) allowExportingGlobalSymbolsFromDirectoryEntries: Boolean = true,
     @Inline() app: Application = Application.default
 ) extends Command {
   def sourceroot: Path = AbsolutePath.of(app.env.workingDirectory)
@@ -75,7 +82,9 @@ final case class IndexSemanticdbCommand(
     val packages =
       absoluteTargetroots
         .iterator
-        .flatMap(ClasspathEntry.fromTargetroot)
+        .flatMap(targetroot =>
+          ClasspathEntry.fromTargetroot(targetroot, sourceroot)
+        )
         .distinct
         .toList
     val options =
@@ -95,7 +104,8 @@ final case class IndexSemanticdbCommand(
         packages.map(_.toPackageInformation).asJava,
         buildKind,
         emitInverseRelationships,
-        allowEmptyIndex
+        allowEmptyIndex,
+        allowExportingGlobalSymbolsFromDirectoryEntries
       )
     ScipSemanticdb.run(options)
     postPackages(packages)

scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/BazelBuildTool.java

@@ -65,7 +65,9 @@ public boolean hasErrors() {
             mavenPackages,
             /* buildKind */ "",
             /* emitInverseRelationships */ true,
-            /* allowEmptyIndex */ true);
+            /* allowEmptyIndex */ true,
+            /* indexDirectoryEntries */ false // because Bazel only compiles to jar files.
+            );
     ScipSemanticdb.run(scipOptions);
 
     if (!scipOptions.reporter.hasErrors()) {

scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/PackageTable.java

@@ -3,11 +3,8 @@
 import java.io.File;
 import java.io.IOException;
 import java.net.URL;
-import java.nio.file.FileSystems;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.PathMatcher;
-import java.nio.file.Paths;
+import java.nio.file.*;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -28,13 +25,17 @@ public class PackageTable implements Function<Package, Integer> {
   private final Map<Package, Integer> scip = new ConcurrentHashMap<>();
   private final JavaVersion javaVersion;
   private final ScipWriter writer;
+  private final boolean indexDirectoryEntries;
 
+  private static final PathMatcher CLASS_PATTERN =
+      FileSystems.getDefault().getPathMatcher("glob:**.class");
   private static final PathMatcher JAR_PATTERN =
       FileSystems.getDefault().getPathMatcher("glob:**.jar");
 
   public PackageTable(ScipSemanticdbOptions options, ScipWriter writer) throws IOException {
     this.writer = writer;
     this.javaVersion = new JavaVersion();
+    this.indexDirectoryEntries = options.allowExportingGlobalSymbolsFromDirectoryEntries;
     // NOTE: it's important that we index the JDK before maven packages. Some maven packages
     // redefine classes from the JDK and we want those maven packages to take precedence over
     // the JDK. The motivation to prioritize maven packages over the JDK is that we only want
@@ -68,13 +69,29 @@ private Optional<Package> packageForClassfile(String classfile) {
   }
 
   private void indexPackage(MavenPackage pkg) throws IOException {
-    if (!JAR_PATTERN.matches(pkg.jar)) {
-      return;
+    if (JAR_PATTERN.matches(pkg.jar) && Files.isRegularFile(pkg.jar)) {
+      indexJarFile(pkg.jar, pkg);
+    } else if (this.indexDirectoryEntries && Files.isDirectory(pkg.jar)) {
+      indexDirectoryPackage(pkg);
     }
-    if (!Files.isRegularFile(pkg.jar)) {
-      return;
-    }
-    indexJarFile(pkg.jar, pkg);
+  }
+
+  private void indexDirectoryPackage(MavenPackage pkg) throws IOException {
+    Files.walkFileTree(
+        pkg.jar,
+        new SimpleFileVisitor<Path>() {
+          @Override
+          public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
+              throws IOException {
+            if (CLASS_PATTERN.matches(file)) {
+              String classfile = pkg.jar.relativize(file).toString();
+              if (!classfile.contains("$")) {
+                byClassfile.put(classfile, pkg);
+              }
+            }
+            return super.visitFile(file, attrs);
+          }
+        });
   }
 
   private void indexJarFile(Path file, Package pkg) throws IOException {

scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/ScipSemanticdbOptions.java

@@ -19,6 +19,7 @@ public class ScipSemanticdbOptions {
   public final String buildKind;
   public final boolean emitInverseRelationships;
   public final boolean allowEmptyIndex;
+  public final boolean allowExportingGlobalSymbolsFromDirectoryEntries;
 
   public ScipSemanticdbOptions(
       List<Path> targetroots,
@@ -32,7 +33,8 @@ public ScipSemanticdbOptions(
       List<MavenPackage> packages,
       String buildKind,
       boolean emitInverseRelationships,
-      boolean allowEmptyIndex) {
+      boolean allowEmptyIndex,
+      boolean allowExportingGlobalSymbolsFromDirectoryEntries) {
     this.targetroots = targetroots;
     this.output = output;
     this.sourceroot = sourceroot;
@@ -45,5 +47,7 @@ public ScipSemanticdbOptions(
     this.buildKind = buildKind;
     this.emitInverseRelationships = emitInverseRelationships;
     this.allowEmptyIndex = allowEmptyIndex;
+    this.allowExportingGlobalSymbolsFromDirectoryEntries =
+        allowExportingGlobalSymbolsFromDirectoryEntries;
   }
 }

semanticdb-gradle-plugin/src/main/scala/SemanticdbGradlePlugin.scala

@@ -5,13 +5,17 @@ import java.nio.file.Paths
 import java.{util => ju}
 
 import scala.jdk.CollectionConverters._
+import scala.util.Try
 
 import com.sourcegraph.scip_java.BuildInfo
 import org.gradle.api.DefaultTask
 import org.gradle.api.Plugin
 import org.gradle.api.Project
 import org.gradle.api.artifacts.component.ModuleComponentIdentifier
 import org.gradle.api.provider.Property
+import org.gradle.api.publish.PublishingExtension
+import org.gradle.api.publish.maven.MavenPublication
+import org.gradle.api.tasks.SourceSetContainer
 import org.gradle.api.tasks.TaskAction
 import org.gradle.api.tasks.compile.JavaCompile
 import org.gradle.api.tasks.scala.ScalaCompile
@@ -396,8 +400,43 @@ class WriteDependencies extends DefaultTask {
       .foreach(path => java.nio.file.Files.createDirectories(path.getParent()))
 
     val deps = List.newBuilder[String]
+    val project = getProject()
+
+    // List the project itself as a dependency so that we can assign project name/version to symbols that are defined in this project.
+    // The code below is roughly equivalent to the following with Groovy:
+    //   deps += "$publication.groupId $publication.artifactId $publication.version $sourceSets.main.output.classesDirectory"
+    Try(
+      project
+        .getExtensions()
+        .getByType(classOf[SourceSetContainer])
+        .getByName("main")
+        .getOutput()
+        .getClassesDirs()
+        .getFiles()
+        .asScala
+        .toList
+        .map(_.getAbsolutePath())
+        .sorted
+        .headOption
+    ).collect { case Some(classesDirectory) =>
+      project
+        .getExtensions()
+        .findByType(classOf[PublishingExtension])
+        .getPublications()
+        .withType(classOf[MavenPublication])
+        .asScala
+        .foreach { publication =>
+          deps +=
+            List(
+              publication.getGroupId(),
+              publication.getArtifactId(),
+              publication.getVersion(),
+              classesDirectory
+            ).mkString("\t")
+        }
+    }
 
-    getProject()
+    project
       .getConfigurations()
       .forEach { conf =>
         if (conf.isCanBeResolved()) {

tests/buildTools/src/test/scala/tests/BaseBuildToolSuite.scala

@@ -111,7 +111,7 @@ abstract class BaseBuildToolSuite extends MopedSuite(ScipJava.app) {
       }
       if (expectedPackages.nonEmpty) {
         val obtainedPackages = ClasspathEntry
-          .fromTargetroot(targetroot)
+          .fromTargetroot(targetroot, workingDirectory)
           .map(_.toPackageHubId)
           .sorted
           .distinct