Separate coverage (bazelbuild#972)

* Separate coverage from compile

* Remove unused

* Simplify condition

* Simplify struct

* Simplify return statement

* Simplify phase

* Fix lint

* Remove condition

scala/private/phases/phase_compile.bzl

@@ -3,12 +3,7 @@
 #
 # DOCUMENT THIS
 #
-load("@bazel_skylib//lib:dicts.bzl", "dicts")
 load("@bazel_tools//tools/jdk:toolchain_utils.bzl", "find_java_runtime_toolchain", "find_java_toolchain")
-load(
-    "@io_bazel_rules_scala//scala/private:coverage_replacements_provider.bzl",
-    _coverage_replacements_provider = "coverage_replacements_provider",
-)
 load(
     "@io_bazel_rules_scala//scala/private:paths.bzl",
     _get_files_with_extension = "get_files_with_extension",
@@ -23,13 +18,6 @@ load(
 )
 load(":resources.bzl", _resource_paths = "paths")
 
-_empty_coverage_struct = struct(
-    external = struct(
-        replacements = {},
-    ),
-    providers_dict = {},
-)
-
 def phase_compile_binary(ctx, p):
     args = struct(
         buildijar = False,
@@ -163,13 +151,12 @@ def _phase_compile(
 
     # TODO: simplify the return values and use provider
     return struct(
-        coverage = out.coverage.external,
         files = depset(out.full_jars),
         rjars = depset(out.full_jars, transitive = [rjars]),
         merged_provider = out.merged_provider,
-        external_providers = dicts.add(out.coverage.providers_dict, {
+        external_providers = {
             "JavaInfo": out.merged_provider,
-        }),
+        },
     )
 
 def _compile_or_empty(
@@ -193,7 +180,6 @@ def _compile_or_empty(
 
         #  no need to build ijar when empty
         return struct(
-            coverage = _empty_coverage_struct,
             full_jars = [ctx.outputs.jar],
             merged_provider = scala_compilation_provider,
         )
@@ -265,15 +251,12 @@ def _compile_or_empty(
         if java_jar:
             full_jars += [java_jar.jar]
 
-        coverage = _jacoco_offline_instrument(ctx, ctx.outputs.jar)
-
         if java_jar:
             merged_provider = java_common.merge([scala_compilation_provider, java_jar.java_compilation_provider])
         else:
             merged_provider = scala_compilation_provider
 
         return struct(
-            coverage = coverage,
             full_jars = full_jars,
             merged_provider = merged_provider,
         )
@@ -339,54 +322,6 @@ def _pack_source_jar(ctx, scala_srcs, in_srcjars):
         host_javabase = find_java_runtime_toolchain(ctx, ctx.attr._host_javabase),
     )
 
-def _jacoco_offline_instrument(ctx, input_jar):
-    if not ctx.configuration.coverage_enabled or not hasattr(ctx.attr, "_code_coverage_instrumentation_worker"):
-        return _empty_coverage_struct
-
-    output_jar = ctx.actions.declare_file(
-        "{}-offline.jar".format(input_jar.basename.split(".")[0]),
-    )
-    in_out_pairs = [
-        (input_jar, output_jar),
-    ]
-
-    args = ctx.actions.args()
-    args.add_all(in_out_pairs, map_each = _jacoco_offline_instrument_format_each)
-    args.set_param_file_format("multiline")
-    args.use_param_file("@%s", use_always = True)
-
-    ctx.actions.run(
-        mnemonic = "JacocoInstrumenter",
-        inputs = [in_out_pair[0] for in_out_pair in in_out_pairs],
-        outputs = [in_out_pair[1] for in_out_pair in in_out_pairs],
-        executable = ctx.attr._code_coverage_instrumentation_worker.files_to_run,
-        execution_requirements = {"supports-workers": "1"},
-        arguments = [args],
-    )
-
-    replacements = {i: o for (i, o) in in_out_pairs}
-    provider = _coverage_replacements_provider.create(
-        replacements = replacements,
-    )
-    instrumented_files_provider = coverage_common.instrumented_files_info(
-        ctx,
-        source_attributes = ["srcs"],
-        dependency_attributes = _coverage_replacements_provider.dependency_attributes,
-        extensions = ["scala", "java"],
-    )
-    return struct(
-        external = struct(
-            replacements = replacements,
-        ),
-        providers_dict = {
-            "_CoverageReplacements": provider,
-            "InstrumentedFilesInfo": instrumented_files_provider,
-        },
-    )
-
-def _jacoco_offline_instrument_format_each(in_out_pair):
-    return (["%s=%s" % (in_out_pair[0].path, in_out_pair[1].path)])
-
 def _try_to_compile_java_jar(
         ctx,
         scala_output,

scala/private/phases/phase_coverage.bzl

@@ -0,0 +1,76 @@
+#
+# PHASE: coverage
+#
+# DOCUMENT THIS
+#
+
+load(
+    "@io_bazel_rules_scala//scala/private:coverage_replacements_provider.bzl",
+    _coverage_replacements_provider = "coverage_replacements_provider",
+)
+
+def phase_coverage_library(ctx, p):
+    args = struct(
+        srcjars = p.collect_srcjars,
+    )
+    return _phase_coverage_default(ctx, p, args)
+
+def phase_coverage_common(ctx, p):
+    return _phase_coverage_default(ctx, p)
+
+def _phase_coverage_default(ctx, p, _args = struct()):
+    return _phase_coverage(
+        ctx,
+        p,
+        _args.srcjars if hasattr(_args, "srcjars") else depset(),
+    )
+
+def _phase_coverage(ctx, p, srcjars):
+    if len(ctx.files.srcs) + len(srcjars.to_list()) == 0 or not ctx.configuration.coverage_enabled:
+        return struct(
+            replacements = {},
+            external_providers = {},
+        )
+    else:
+        input_jar = ctx.outputs.jar
+        output_jar = ctx.actions.declare_file(
+            "{}-offline.jar".format(input_jar.basename.split(".")[0]),
+        )
+        in_out_pairs = [
+            (input_jar, output_jar),
+        ]
+
+        args = ctx.actions.args()
+        args.add_all(in_out_pairs, map_each = _jacoco_offline_instrument_format_each)
+        args.set_param_file_format("multiline")
+        args.use_param_file("@%s", use_always = True)
+
+        ctx.actions.run(
+            mnemonic = "JacocoInstrumenter",
+            inputs = [in_out_pair[0] for in_out_pair in in_out_pairs],
+            outputs = [in_out_pair[1] for in_out_pair in in_out_pairs],
+            executable = ctx.attr._code_coverage_instrumentation_worker.files_to_run,
+            execution_requirements = {"supports-workers": "1"},
+            arguments = [args],
+        )
+
+        replacements = {i: o for (i, o) in in_out_pairs}
+        provider = _coverage_replacements_provider.create(
+            replacements = replacements,
+        )
+        instrumented_files_provider = coverage_common.instrumented_files_info(
+            ctx,
+            source_attributes = ["srcs"],
+            dependency_attributes = _coverage_replacements_provider.dependency_attributes,
+            extensions = ["scala", "java"],
+        )
+        return struct(
+            replacements = replacements,
+            external_providers = {
+                "_CoverageReplacements": provider,
+                "InstrumentedFilesInfo": instrumented_files_provider,
+            },
+        )
+
+def _jacoco_offline_instrument_format_each(in_out_pair):
+    return (["%s=%s" % (in_out_pair[0].path, in_out_pair[1].path)])

scala/private/phases/phase_coverage_runfiles.bzl

@@ -14,7 +14,7 @@ def phase_coverage_runfiles(ctx, p):
     if ctx.configuration.coverage_enabled and _coverage_replacements_provider.is_enabled(ctx):
         coverage_replacements = _coverage_replacements_provider.from_ctx(
             ctx,
-            base = p.compile.coverage.replacements,
+            base = p.coverage.replacements,
         ).replacements
 
         rjars = depset([

scala/private/phases/phases.bzl

@@ -45,6 +45,11 @@ load(
     _phase_runfiles_library = "phase_runfiles_library",
     _phase_runfiles_scalatest = "phase_runfiles_scalatest",
 )
+load(
+    "@io_bazel_rules_scala//scala/private:phases/phase_coverage.bzl",
+    _phase_coverage_common = "phase_coverage_common",
+    _phase_coverage_library = "phase_coverage_library",
+)
 load("@io_bazel_rules_scala//scala/private:phases/phase_default_info.bzl", _phase_default_info = "phase_default_info")
 load("@io_bazel_rules_scala//scala/private:phases/phase_scalac_provider.bzl", _phase_scalac_provider = "phase_scalac_provider")
 load("@io_bazel_rules_scala//scala/private:phases/phase_write_manifest.bzl", _phase_write_manifest = "phase_write_manifest")
@@ -85,6 +90,10 @@ phase_merge_jars = _phase_merge_jars
 # jvm_flags
 phase_jvm_flags = _phase_jvm_flags
 
+# coverage
+phase_coverage_library = _phase_coverage_library
+phase_coverage_common = _phase_coverage_common
+
 # coverage_runfiles
 phase_coverage_runfiles = _phase_coverage_runfiles
 

scala/private/rule_impls.bzl

@@ -14,10 +14,6 @@
 """Rules for supporting the Scala language."""
 
 load("@bazel_skylib//lib:paths.bzl", "paths")
-load(
-    "@io_bazel_rules_scala//scala/private:coverage_replacements_provider.bzl",
-    _coverage_replacements_provider = "coverage_replacements_provider",
-)
 load(
     ":common.bzl",
     _collect_plugin_paths = "collect_plugin_paths",

scala/private/rules/scala_binary.bzl

@@ -14,6 +14,7 @@ load(
     "extras_phases",
     "phase_collect_jars_common",
     "phase_compile_binary",
+    "phase_coverage_common",
     "phase_declare_executable",
     "phase_default_info",
     "phase_java_wrapper_common",
@@ -39,6 +40,7 @@ def _scala_binary_impl(ctx):
             ("declare_executable", phase_declare_executable),
             # no need to build an ijar for an executable
             ("compile", phase_compile_binary),
+            ("coverage", phase_coverage_common),
             ("merge_jars", phase_merge_jars),
             ("runfiles", phase_runfiles_common),
             ("write_executable", phase_write_executable_common),

scala/private/rules/scala_junit_test.bzl

@@ -13,6 +13,7 @@ load(
     "extras_phases",
     "phase_collect_jars_junit_test",
     "phase_compile_junit_test",
+    "phase_coverage_common",
     "phase_declare_executable",
     "phase_default_info",
     "phase_java_wrapper_common",
@@ -43,6 +44,7 @@ def _scala_junit_test_impl(ctx):
             ("declare_executable", phase_declare_executable),
             # no need to build an ijar for an executable
             ("compile", phase_compile_junit_test),
+            ("coverage", phase_coverage_common),
             ("merge_jars", phase_merge_jars),
             ("runfiles", phase_runfiles_common),
             ("jvm_flags", phase_jvm_flags),

scala/private/rules/scala_library.bzl

@@ -26,6 +26,8 @@ load(
     "phase_compile_library",
     "phase_compile_library_for_plugin_bootstrapping",
     "phase_compile_macro_library",
+    "phase_coverage_common",
+    "phase_coverage_library",
     "phase_default_info",
     "phase_merge_jars",
     "phase_runfiles_library",
@@ -63,6 +65,7 @@ def _scala_library_impl(ctx):
             ("unused_deps_checker", phase_unused_deps_checker),
             ("collect_jars", phase_collect_jars_common),
             ("compile", phase_compile_library),
+            ("coverage", phase_coverage_library),
             ("merge_jars", phase_merge_jars),
             ("runfiles", phase_runfiles_library),
             ("collect_exports_jars", phase_collect_exports_jars),
@@ -194,6 +197,7 @@ def _scala_macro_library_impl(ctx):
             ("unused_deps_checker", phase_unused_deps_checker),
             ("collect_jars", phase_collect_jars_macro_library),
             ("compile", phase_compile_macro_library),
+            ("coverage", phase_coverage_common),
             ("merge_jars", phase_merge_jars),
             ("runfiles", phase_runfiles_library),
             ("collect_exports_jars", phase_collect_exports_jars),

scala/private/rules/scala_repl.bzl

@@ -14,6 +14,7 @@ load(
     "extras_phases",
     "phase_collect_jars_repl",
     "phase_compile_repl",
+    "phase_coverage_common",
     "phase_declare_executable",
     "phase_default_info",
     "phase_java_wrapper_repl",
@@ -40,6 +41,7 @@ def _scala_repl_impl(ctx):
             ("declare_executable", phase_declare_executable),
             # no need to build an ijar for an executable
             ("compile", phase_compile_repl),
+            ("coverage", phase_coverage_common),
             ("merge_jars", phase_merge_jars),
             ("runfiles", phase_runfiles_common),
             ("write_executable", phase_write_executable_repl),

scala/private/rules/scala_test.bzl

@@ -14,6 +14,7 @@ load(
     "extras_phases",
     "phase_collect_jars_scalatest",
     "phase_compile_scalatest",
+    "phase_coverage_common",
     "phase_coverage_runfiles",
     "phase_declare_executable",
     "phase_default_info",
@@ -40,6 +41,7 @@ def _scala_test_impl(ctx):
             ("declare_executable", phase_declare_executable),
             # no need to build an ijar for an executable
             ("compile", phase_compile_scalatest),
+            ("coverage", phase_coverage_common),
             ("merge_jars", phase_merge_jars),
             ("runfiles", phase_runfiles_scalatest),
             ("coverage_runfiles", phase_coverage_runfiles),