Fix coverage for crates with mixed sources (bazelbuild#4106)

An expansion of bazelbuild#4079 with
some added testing.

closes bazelbuild#4079

Author: UebelAndre

.bazelci/presubmit.yml

@@ -71,6 +71,18 @@ coverage_validation_post_shell_commands: &coverage_validation_post_shell_command
       ; 1>&2 head -50 bazel-out/_coverage/_coverage_report.dat \
       ; exit 1 \
       ; }
+  # Regression check: mixed in-tree/generated-source crates must produce
+  # real coverage, not just the zero-LF baseline record.
+  - |
+    awk '/^SF:test\/generated_inputs\/lib\.rs$/,/^end_of_record$/' \
+        bazel-out/_coverage/_coverage_report.dat \
+      | grep -qE '^LF:[1-9]' \
+    || { 1>&2 echo "No coverage for mixed-source crate (test/generated_inputs/lib.rs)" \
+      ; 1>&2 echo "Coverage was silently dropped for crates with generated sources." \
+      ; 1>&2 awk '/^SF:test\/generated_inputs\/lib\.rs$/,/^end_of_record$/' \
+          bazel-out/_coverage/_coverage_report.dat \
+      ; exit 1 \
+      ; }
 split_coverage_postprocessing_shell_commands: &split_coverage_postprocessing_shell_commands
   - echo "coverage --experimental_fetch_all_coverage_outputs" >> user.bazelrc
   - echo "coverage --experimental_split_coverage_postprocessing" >> user.bazelrc

rust/private/rustc.bzl

@@ -897,6 +897,14 @@ def _has_location_expansion(flags):
                 return True
     return False
 
+def _args_map_bin_dir(file):
+    """Extract `bazel-out/<config>/bin` from a File whose path lives in the configuration's bin directory.
+
+    Evaluated at action-execution time so that Bazel's path mapping (`--experimental_output_paths=strip`) can
+    rewrite the `<config>` segment to `cfg` before we slice it off.
+    """
+    return "/".join(file.path.split("/", 3)[:3])
+
 def construct_arguments(
         *,
         ctx,
@@ -1326,6 +1334,25 @@ def construct_arguments(
         # https://doc.rust-lang.org/rustc/instrument-coverage.html
         rustc_flags.add("--codegen=instrument-coverage")
 
+        # Crates with generated sources are compiled from the output tree
+        # (see `transform_sources`), so the coverage mapping records their
+        # files with a `bazel-out/<config>/bin/` prefix. Bazel's lcov
+        # merger silently drops all coverage for such crates, so we remap
+        # the prefix away. The prefix is derived from the crate's own
+        # output File (rather than `ctx.bin_dir`) so that Bazel's path
+        # mapping (`--experimental_output_paths=strip`) can rewrite the
+        # `<config>` segment to `cfg` before the value reaches rustc —
+        # `ctx.bin_dir` is a `root`, not a `File`, and is not subject to
+        # path-mapping rewriting. Skipped for rustdoc (which passes
+        # `remap_path_prefix=None`), since rustdoc only supports
+        # `--remap-path-prefix` behind `-Zunstable-options`.
+        if remap_path_prefix != None:
+            rustc_flags.add_all(
+                [crate_info.output],
+                format_each = "--remap-path-prefix=%s/=",
+                map_each = _args_map_bin_dir,
+            )
+
     if toolchain._experimental_link_std_dylib:
         rustc_flags.add("--codegen=prefer-dynamic")
 

test/unit/common.bzl

@@ -149,3 +149,25 @@ def assert_env_value(env, action, key, value):
             real_value = action.env[key],
         ),
     )
+
+def get_bin_dir_from_action(action):
+    """Extract `bazel-out/<config>/bin` from an action.
+
+    Inspects argv first so the path-mapping (`--experimental_output_paths=strip`)
+    form `bazel-out/cfg/bin` is returned when active, otherwise falls back to
+    the first output's `.dirname` (handles `-ST-<hash>` config-transition
+    suffixes).
+
+    Args:
+        action: The action to extract the bin directory from.
+
+    Returns:
+        The bin directory path as a string.
+    """
+    for arg in action.argv:
+        if arg.startswith("bazel-out/cfg/bin/"):
+            return "bazel-out/cfg/bin"
+    bin_dir = action.outputs.to_list()[0].dirname
+    if "/bin/" in bin_dir:
+        bin_dir = bin_dir.split("/bin/")[0] + "/bin"
+    return bin_dir

test/unit/native_deps/native_deps_test.bzl

@@ -11,35 +11,12 @@ load(
     "assert_argv_contains_prefix_not",
     "assert_argv_contains_prefix_suffix",
     "assert_list_contains_adjacent_elements",
+    "get_bin_dir_from_action",
 )
 
 def _get_toolchain(ctx):
     return ctx.attr._toolchain[platform_common.ToolchainInfo]
 
-def _get_bin_dir_from_action(action):
-    """Extract the bin directory from an action's outputs.
-
-    This handles config transitions that add suffixes like -ST-<hash>, as
-    well as Bazel path mapping (`--experimental_output_paths=strip` plus a
-    `supports-path-mapping` requirement on the action), which rewrites
-    argv entries to live under `bazel-out/cfg/bin/...` even though the
-    File's `.dirname` still returns the un-mapped, configuration-specific
-    path.
-
-    Args:
-        action: The action to extract the bin directory from.
-
-    Returns:
-        The bin directory path as a string.
-    """
-    for arg in action.argv:
-        if arg.startswith("bazel-out/cfg/bin/"):
-            return "bazel-out/cfg/bin"
-    bin_dir = action.outputs.to_list()[0].dirname
-    if "/bin/" in bin_dir:
-        bin_dir = bin_dir.split("/bin/")[0] + "/bin"
-    return bin_dir
-
 def _get_darwin_component(arg):
     """Extract darwin component from a path.
 
@@ -213,7 +190,7 @@ def _bin_has_native_dep_and_alwayslink_test_impl(ctx, use_cc_linker):
 
     toolchain = _get_toolchain(ctx)
     link_args = _extract_linker_args(action.argv)
-    bin_dir = _get_bin_dir_from_action(action)
+    bin_dir = get_bin_dir_from_action(action)
 
     # Validate bin_dir structure (ignoring ST-{hash} suffix from config transitions)
     _assert_bin_dir_structure(env, ctx, bin_dir, toolchain)
@@ -296,7 +273,7 @@ def _cdylib_has_native_dep_and_alwayslink_test_impl(ctx, use_cc_linker):
     action = tut.actions[0]
 
     linker_args = _extract_linker_args(action.argv)
-    bin_dir = _get_bin_dir_from_action(action)
+    bin_dir = get_bin_dir_from_action(action)
 
     # Validate bin_dir structure (ignoring ST-{hash} suffix from config transitions)
     _assert_bin_dir_structure(env, ctx, bin_dir, toolchain)

test/unit/remap_path_prefix/remap_path_prefix_test.bzl

@@ -6,7 +6,10 @@ load("//rust:defs.bzl", "rust_binary", "rust_library")
 load(
     "//test/unit:common.bzl",
     "assert_action_mnemonic",
+    "assert_argv_contains",
+    "assert_argv_contains_not",
     "assert_list_contains_adjacent_elements",
+    "get_bin_dir_from_action",
 )
 
 def _remap_path_prefix_test_impl(ctx):
@@ -42,6 +45,68 @@ def _subst_flags_test_impl(ctx):
 
 _subst_flags_test = analysistest.make(_subst_flags_test_impl)
 
+def _coverage_remap_path_prefix_test_impl(ctx):
+    """Verify a single `--remap-path-prefix` flag covers the bin directory.
+
+    The flag is derived from a `File` via `map_each` so Bazel's path
+    mapping (`--experimental_output_paths=strip`) rewrites the
+    `<config>` segment to `cfg` before it reaches rustc — one flag
+    works for both the path-mapped and un-mapped forms.
+    """
+    env = analysistest.begin(ctx)
+    target = analysistest.target_under_test(env)
+
+    action = target.actions[0]
+    assert_action_mnemonic(env, action, "Rustc")
+
+    # If the host toolchain does not support coverage (no `llvm_cov` or
+    # missing `profiler_builtins`) the instrument-coverage flag will be
+    # absent and the remap flag is not expected either.
+    if "--codegen=instrument-coverage" not in action.argv:
+        return analysistest.end(env)
+
+    bin_dir = get_bin_dir_from_action(action)
+    assert_argv_contains(env, action, "--remap-path-prefix={}/=".format(bin_dir))
+
+    return analysistest.end(env)
+
+_coverage_remap_path_prefix_test = analysistest.make(
+    _coverage_remap_path_prefix_test_impl,
+    config_settings = {
+        "//command_line_option:collect_code_coverage": True,
+    },
+)
+
+_coverage_remap_path_prefix_path_mapping_test = analysistest.make(
+    _coverage_remap_path_prefix_test_impl,
+    config_settings = {
+        "//command_line_option:collect_code_coverage": True,
+        "//command_line_option:experimental_output_paths": "strip",
+    },
+)
+
+def _no_coverage_remap_path_prefix_test_impl(ctx):
+    """Verify the coverage-specific remap flag is absent without coverage."""
+    env = analysistest.begin(ctx)
+    target = analysistest.target_under_test(env)
+
+    action = target.actions[0]
+    assert_action_mnemonic(env, action, "Rustc")
+
+    assert_argv_contains_not(env, action, "--codegen=instrument-coverage")
+
+    bin_dir = get_bin_dir_from_action(action)
+    assert_argv_contains_not(env, action, "--remap-path-prefix={}/=".format(bin_dir))
+
+    return analysistest.end(env)
+
+_no_coverage_remap_path_prefix_test = analysistest.make(
+    _no_coverage_remap_path_prefix_test_impl,
+    config_settings = {
+        "//command_line_option:collect_code_coverage": False,
+    },
+)
+
 def remap_path_prefix_test_suite(name):
     """Entry-point macro called from the BUILD file.
 
@@ -78,6 +143,38 @@ def remap_path_prefix_test_suite(name):
         edition = "2021",
     )
 
+    # A library whose source set contains a generated file — this is the
+    # case `transform_sources` is designed for, and the one that
+    # previously lost all coverage data because the records pointed at
+    # `bazel-out/.../bin/...`.
+    write_file(
+        name = "mixed_inline_src",
+        out = "mixed_inline.rs",
+        content = [
+            "pub fn inline() {}",
+            "",
+        ],
+    )
+
+    write_file(
+        name = "mixed_generated_src",
+        out = "mixed_generated.rs",
+        content = [
+            "pub fn generated() {}",
+            "",
+        ],
+    )
+
+    rust_library(
+        name = "remap_mixed_lib",
+        srcs = [
+            ":mixed_inline.rs",
+            ":mixed_generated_src",
+        ],
+        crate_root = ":mixed_inline.rs",
+        edition = "2021",
+    )
+
     _remap_path_prefix_test(
         name = "remap_path_prefix_lib_test",
         target_under_test = ":remap_lib",
@@ -98,11 +195,29 @@ def remap_path_prefix_test_suite(name):
         target_under_test = ":remap_bin",
     )
 
+    _coverage_remap_path_prefix_test(
+        name = "coverage_remap_path_prefix_mixed_lib_test",
+        target_under_test = ":remap_mixed_lib",
+    )
+
+    _coverage_remap_path_prefix_path_mapping_test(
+        name = "coverage_remap_path_prefix_path_mapping_mixed_lib_test",
+        target_under_test = ":remap_mixed_lib",
+    )
+
+    _no_coverage_remap_path_prefix_test(
+        name = "no_coverage_remap_path_prefix_lib_test",
+        target_under_test = ":remap_lib",
+    )
+
     tests = [
         ":remap_path_prefix_lib_test",
         ":remap_path_prefix_bin_test",
         ":subst_flags_lib_test",
         ":subst_flags_bin_test",
+        ":coverage_remap_path_prefix_mixed_lib_test",
+        ":coverage_remap_path_prefix_path_mapping_mixed_lib_test",
+        ":no_coverage_remap_path_prefix_lib_test",
     ]
 
     native.test_suite(