Merge pull request #60 from lucidsoftware/expose-semanticdb-provider

Expose a SemanticDB provider in the semanticdb rule phase

rules/private/phases/phase_semanticdb.bzl

@@ -1,5 +1,21 @@
 load("@bazel_skylib//lib:paths.bzl", "paths")
-load("@rules_scala_annex//rules:providers.bzl", _ScalaConfiguration = "ScalaConfiguration")
+load(
+    "@rules_scala_annex//rules:providers.bzl",
+    _ScalaConfiguration = "ScalaConfiguration",
+    _SemanticDbInfo = "SemanticDbInfo",
+)
+
+def _semanticdb_directory_from_file(file):
+    """
+    This is janky, but we're limited in what we can do in this function. From the
+    [documentation](https://bazel.build/rules/lib/builtins/Args#add_all) on `Args#add_all`:
+
+    To avoid unintended retention of large analysis-phase data structures into the execution phase,
+    the `map_each` function must be declared by a top-level `def` statement; it may not be a
+    nested function closure by default.
+    """
+
+    return "{}/{}".format(file.root.path, file.short_path[:file.short_path.find("META-INF") - 1])
 
 #
 # PHASE: semanticdb
@@ -11,32 +27,49 @@ def phase_semanticdb(ctx, g):
     scala_configuration = ctx.attr.scala[_ScalaConfiguration]
 
     if scala_configuration.semanticdb_bundle:
-        return struct(outputs = [], scalacopts = [])
+        return struct(outputs = [], arguments_modifier = lambda _: None)
 
+    directory_name = "{}/semanticdb".format(ctx.label.name)
     outputs = []
-    semanticdb_directory = paths.join("_semanticdb/", ctx.label.name)
-    semanticdb_target_root = paths.join(paths.dirname(ctx.outputs.jar.path), semanticdb_directory)
 
     for source in ctx.files.srcs:
         if source.extension == "scala":
-            output_filename = paths.join(
-                semanticdb_directory,
+            path = paths.join(
+                directory_name,
                 "META-INF",
                 "semanticdb",
                 "{}.semanticdb".format(source.path),
             )
 
-            outputs.append(ctx.actions.declare_file(output_filename))
+            outputs.append(ctx.actions.declare_file(path))
+
+    def add_scalacopts(arguments):
+        if len(outputs) == 0:
+            return
+
+        if scala_configuration.version.startswith("2"):
+            arguments.add("--compiler_option=-P:semanticdb:failures:error")
+            arguments.add("--compiler_option_referencing_path=-P:semanticdb:sourceroot:${workDir}")
+            arguments.add_all(
+                [outputs[0]],
+                format_each = "--compiler_option_referencing_path=-P:semanticdb:targetroot:${path} %s",
+                map_each = _semanticdb_directory_from_file,
+            )
+        else:
+            arguments.add_all(
+                [outputs[0]],
+                format_each = "--compiler_option_referencing_path=-semanticdb-target:${path} %s",
+                map_each = _semanticdb_directory_from_file,
+            )
+
+            arguments.add("--compiler_option_referencing_path=-sourceroot:${workDir}")
+            arguments.add("--compiler_option=-Ysemanticdb")
 
-    if scala_configuration.version.startswith("2"):
-        scalacopts = [
-            "-P:semanticdb:failures:error",
-            "-P:semanticdb:targetroot:{}".format(semanticdb_target_root),
-        ]
-    else:
-        scalacopts = [
-            "-semanticdb-target:{}".format(semanticdb_target_root),
-            "-Ysemanticdb",
-        ]
+    g.out.providers.append(
+        _SemanticDbInfo(
+            target_root = "{}/{}".format(ctx.label.package, directory_name),
+            semanticdb_files = outputs,
+        ),
+    )
 
-    return struct(outputs = outputs, scalacopts = scalacopts)
+    return struct(outputs = outputs, arguments_modifier = add_scalacopts)

rules/private/phases/phase_zinc_compile.bzl

@@ -34,10 +34,9 @@ def phase_zinc_compile(ctx, g):
     ]
 
     zincs = [dep[_ZincInfo] for dep in ctx.attr.deps if _ZincInfo in dep]
-    common_scalacopts = scala_configuration.global_scalacopts + ctx.attr.scalacopts + g.semanticdb.scalacopts
+    common_scalacopts = scala_configuration.global_scalacopts + ctx.attr.scalacopts
 
     args = ctx.actions.args()
-
     args.add_all(depset(transitive = [zinc.deps for zinc in zincs]), map_each = _compile_analysis)
     args.add("--compiler_bridge", zinc_configuration.compiler_bridge)
     args.add_all("--compiler_classpath", g.classpaths.compiler)
@@ -52,8 +51,10 @@ def phase_zinc_compile(ctx, g):
     args.add_all("--plugins", g.classpaths.plugin)
     args.add_all("--source_jars", g.classpaths.src_jars)
     args.add("--tmp", tmp.path)
-
     args.add("--log_level", zinc_configuration.log_level)
+
+    g.semanticdb.arguments_modifier(args)
+
     args.add_all("--", g.classpaths.srcs)
     args.set_param_file_format("multiline")
     args.use_param_file("@%s", use_always = True)

rules/providers.bzl

@@ -207,3 +207,11 @@ LabeledJars = provider(
         "values": "The preorder depset of label and jars.",
     },
 )
+
+SemanticDbInfo = provider(
+    doc = "Provided by the Scala rules when `semanticdb_bundle` is set to `False`.",
+    fields = {
+        "target_root": "The directory in which SemanticDB files were outputted.",
+        "semanticdb_files": "The SemanticDB files.",
+    },
+)

src/main/scala/higherkindness/rules_scala/workers/common/CommonArguments.scala

@@ -7,6 +7,7 @@ import common.sandbox.SandboxUtil
 import net.sourceforge.argparse4j.impl.{Arguments => ArgumentsImpl}
 import net.sourceforge.argparse4j.inf.{Argument, ArgumentParser, ArgumentType, Namespace}
 import java.util.{Collections, List as JList}
+import scala.annotation.nowarn
 import scala.collection.mutable.Buffer
 import scala.jdk.CollectionConverters.*
 import java.io.File
@@ -17,6 +18,18 @@ class CommonArguments private (
   val compilerBridge: Path,
   val compilerClasspath: List[Path],
   val compilerOptions: List[String],
+
+  /**
+   * With [[https://bazel.build/remote/multiplex#multiplex_sandboxing multiplex sandboxing]], Bazel generates a separate
+   * sandbox directory for each worker invocation, which means the paths to build artifacts won't be known until the
+   * execution phase. However, compiler options are usually generated during the analysis phase.
+   *
+   * Some compiler options (currently, only those regarding SemanticDB) reference paths to build artifacts and need to
+   * be adjusted to be relative to the sandbox directory. It's more performant for those options to be passed in a
+   * separate list so we don't have to scan through every compiler option and potentially modify it. In our experience,
+   * this resulted in a ~10% worker speedup.
+   */
+  val compilerOptionsReferencingPaths: List[String],
   val classpath: List[Path],
   val debug: Boolean,
   val javaCompilerOptions: List[String],
@@ -49,18 +62,22 @@ object Analysis {
 }
 
 object CommonArguments {
-  private def adjustCompilerOptions(workDir: Path, options: List[String]) = {
-    def adjustStringPath(path: String) = SandboxUtil.getSandboxPath(workDir, Paths.get(path)).toString
-
-    options.flatMap {
-      case s"-P:semanticdb:targetroot:$path" =>
-        List(s"-P:semanticdb:sourceroot:${workDir.toString}", s"-P:semanticdb:targetroot:${adjustStringPath(path)}")
-
-      case s"-semanticdb-target:$path" =>
-        List(s"-semanticdb-target:${adjustStringPath(path)}", s"-sourceroot:${workDir.toString}")
+  private def adjustCompilerOptions(workDir: Path, options: List[String]) = options.map { option =>
+    val i = option.lastIndexOf(' ')
+    val withPathReplaced = if (i == -1) {
+      option
+    } else {
+      val template = option.slice(0, i)
+      val path = option.slice(i + 1, option.length)
 
-      case option => List(option)
+      template.replace(
+        "${path}": @nowarn("cat=lint-missing-interpolator"),
+        SandboxUtil.getSandboxPath(workDir, Paths.get(path)).toString,
+      )
     }
+
+    withPathReplaced
+      .replace("${workDir}": @nowarn("cat=lint-missing-interpolator"), workDir.toString)
   }
 
   /**
@@ -91,6 +108,11 @@ object CommonArguments {
       .help("Compiler option")
       .action(ArgumentsImpl.append)
       .metavar("option")
+    parser
+      .addArgument("--compiler_option_referencing_path")
+      .help("Compiler option referencing the paths to build artifact(s)")
+      .action(ArgumentsImpl.append)
+      .metavar("option")
     parser
       .addArgument("--classpath")
       .help("Compilation classpath")
@@ -191,9 +213,14 @@ object CommonArguments {
       analyses = analyses,
       compilerBridge = SandboxUtil.getSandboxPath(workDir, namespace.get[Path]("compiler_bridge")),
       compilerClasspath = SandboxUtil.getSandboxPaths(workDir, namespace.getList[Path]("compiler_classpath")),
-      compilerOptions = adjustCompilerOptions(
+      compilerOptions = Option(namespace.getList[String]("compiler_option"))
+        .map(_.asScala.toList)
+        .getOrElse(List.empty),
+      compilerOptionsReferencingPaths = adjustCompilerOptions(
         workDir,
-        Option(namespace.getList[String]("compiler_option")).map(_.asScala.toList).getOrElse(List.empty),
+        Option(namespace.getList[String]("compiler_option_referencing_path"))
+          .map(_.asScala.toList)
+          .getOrElse(List.empty),
       ),
       classpath = SandboxUtil.getSandboxPaths(workDir, namespace.getList[Path]("classpath")),
       debug = namespace.getBoolean("debug"),

src/main/scala/higherkindness/rules_scala/workers/zinc/compile/ZincRunner.scala

@@ -214,10 +214,11 @@ object ZincRunner extends WorkerMain[ZincRunnerWorkerConfig] {
         .withClassesDirectory(classesOutputDir)
         .withJavacOptions(workRequest.javaCompilerOptions.toArray)
         .withScalacOptions(
-          Array.concat(
-            workRequest.plugins.map(p => s"-Xplugin:$p").toArray,
-            workRequest.compilerOptions.toArray,
-          ),
+          (
+            workRequest.plugins.map(p => s"-Xplugin:$p") ++
+              workRequest.compilerOptions ++
+              workRequest.compilerOptionsReferencingPaths
+          ).toArray,
         )
 
     val compilers = {

tests/plugins/semanticdb/BUILD

@@ -1,5 +1,6 @@
 load("@rules_scala_annex//rules:scala.bzl", "configure_zinc_scala", "scala_library")
 load("@rules_scala_annex//rules/scala:workspace.bzl", "scala_2_13_version", "scala_3_version")
+load(":rule.bzl", "read_semanticdb_info")
 
 configure_zinc_scala(
     name = "scala_2_13_with_semanticdb",
@@ -39,9 +40,26 @@ scala_library(
     tags = ["manual"],
 )
 
+read_semanticdb_info(
+    name = "semanticdb-2_13-semanticdb-info",
+    scala_target = ":semanticdb-2_13",
+)
+
 scala_library(
     name = "semanticdb-3",
     srcs = glob(["*.scala"]),
     scala = ":scala_3_with_semanticdb",
     tags = ["manual"],
 )
+
+read_semanticdb_info(
+    name = "semanticdb-3-semanticdb-info",
+    scala_target = ":semanticdb-3",
+)
+
+scala_library(
+    name = "semanticdb-empty",
+    srcs = [],
+    scala = ":scala_2_13_with_semanticdb",
+    tags = ["manual"],
+)

tests/plugins/semanticdb/rule.bzl

@@ -0,0 +1,25 @@
+load("@rules_scala_annex//rules:providers.bzl", "SemanticDbInfo")
+
+def _read_semanticdb_info_impl(ctx):
+    semanticdb_info = ctx.attr.scala_target[SemanticDbInfo]
+    output = ctx.actions.declare_file("{}.txt".format(ctx.label.name))
+
+    ctx.actions.write(
+        output,
+        json.encode({
+            "targetRoot": semanticdb_info.target_root,
+            "semanticDbFiles": sorted([file.path for file in semanticdb_info.semanticdb_files]),
+        }),
+    )
+
+    return DefaultInfo(files = depset([output]))
+
+read_semanticdb_info = rule(
+    attrs = {
+        "scala_target": attr.label(
+            mandatory = True,
+            providers = [SemanticDbInfo],
+        ),
+    },
+    implementation = _read_semanticdb_info_impl,
+)

tests/plugins/semanticdb/test

@@ -1,9 +1,11 @@
 #!/bin/bash -e
 . "$(dirname "$0")"/../../common.sh
 
+bazel_bin="$(bazel info bazel-bin)"
+
 check_for_semanticdb_files() {
 	for filename in "A.scala.semanticdb" "B.scala.semanticdb"; do
-		path="../../bazel-bin/plugins/semanticdb/_semanticdb/semanticdb-$1/META-INF/semanticdb/plugins/semanticdb/$filename"
+		path="../../bazel-bin/plugins/semanticdb/semanticdb-$1/semanticdb/META-INF/semanticdb/plugins/semanticdb/$filename"
 
 		if [ ! -f "$path" ]; then
 			echo "Error: $path doesn't exist"
@@ -12,7 +14,22 @@ check_for_semanticdb_files() {
 	done
 }
 
+check_semanticdb_info() {
+	bazel build ":semanticdb-$1-semanticdb-info"
+
+	output_path="$bazel_bin/plugins/semanticdb/semanticdb-$1-semanticdb-info.txt"
+	semanticdb_file_directory="bazel-out/k8-fastbuild/bin/plugins/semanticdb/semanticdb-$1/semanticdb/META-INF/semanticdb/plugins/semanticdb"
+
+	[ "$(jq ".targetRoot" "$output_path")" = "\"plugins/semanticdb/semanticdb-$1/semanticdb\"" ]
+	[ "$(jq -c ".semanticDbFiles" "$output_path")" = "[\"$semanticdb_file_directory/A.scala.semanticdb\",\"$semanticdb_file_directory/B.scala.semanticdb\"]" ]
+}
+
 bazel build :semanticdb-2_13
 check_for_semanticdb_files 2_13
+check_semanticdb_info 2_13
+
 bazel build :semanticdb-3
 check_for_semanticdb_files '3'
+check_semanticdb_info '3'
+
+bazel build :semanticdb-empty