Add ctx.coverage_instrumented function to Skylark

Skylark already has ctx.configuration.coverage_enabled to determine if coverage data collection is on for an entire run. But that does not reveal which targets specifically are supposed to be instrumented (based on the values of --instrumentation_filer and --instrument_test_targets). This is inefficient for languages which add coverage instrumentation at compile-time, though correct coverage output can still be produced by instrumenting everything and filtering later.

By default, this function returns whether the rule represented by ctx should be instrumented. If a Skylark Target (e.g. from a label or label_list attribute in ctx.attr) is passed to the function, it instead returns whether that Target is a rule whose sources should be instrumented. Rules that directly incorporate source-files from their dependencies before compilation (e.g. header files) may need to know if those source files need to be instrumented when compiled.

Expanded the documentation of instrumented_files to be a more general section on implementing code coverage instrumentation in Skylark.

Also tweaked the code comment and variable names for the version of shouldIncludeLocalSources that takes a TransitiveInfoCollection.

RELNOTES: Add ctx.coverage_instrumented function to Skylark, to indicate whether a specific targets should be instrumented for code coverage data collection.

--
MOS_MIGRATED_REVID=139460989

5 files changed, 148 insertions, 11 deletions

diff --git a/site/versions/master/docs/skylark/rules.md b/site/versions/master/docs/skylark/rules.md
index cc92341398..a93ece56e0 100644
--- a/site/versions/master/docs/skylark/rules.md
+++ b/site/versions/master/docs/skylark/rules.md
@@ -417,11 +417,10 @@ with an error describing the conflict. To fix, you will need to modify your
 any targets using your rule, as well as targets of any kind that depend on those
 targets.
 
-## Instrumented files
+## Code coverage instrumentation
 
-Instrumented files are a set of files used by the coverage command. A rule can
-use the `instrumented_files` provider to provide information about which files
-should be used for measuring coverage.
+A rule can use the `instrumented_files` provider to provide information about
+which files should be measured when code coverage data collection is enabled:
 
 ```python
 def rule_implementation(ctx):
@@ -439,6 +438,36 @@ def rule_implementation(ctx):
       dependency_attributes=["data", "deps"]))
 ```
 
+`ctx.config.coverage_enabled` notes whether coverage data collection is enabled
+for the current run in general (but says nothing about which files specifically
+should be instrumented). If a rule implementation needs to add coverage
+instrumentation at compile-time, it can determine if its sources should be
+instrumented with:
+
+```python
+# Are this rule's sources instrumented?
+if ctx.coverage_instrumented():
+  # Do something to turn on coverage for this compile action
+```
+
+Note that function will always return false if `ctx.config.coverage_enabled` is
+false, so you don't need to check both.
+
+If the rule directly includes sources from its dependencies before compilation
+(e.g. header files), it may also need to turn on compile-time instrumentation
+if the dependencies' sources should be instrumented. In this case, it may
+also be worth checking `ctx.config.coverage_enabled` so you can avoid looping
+over dependencies unnecessarily:
+
+```python
+# Are this rule's sources or any of the sources for its direct dependencies
+# in deps instrumented?
+if ctx.config.coverage_enabled:
+    if (ctx.coverage_instrumented() or
+        any(ctx.coverage_instrumented(dep) for dep in ctx.attr.deps):
+        # Do something to turn on coverage for this compile action
+```
+
 ## Executable rules
 
 An executable rule is a rule that users can run using `bazel run`.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index b8b670acdb..1abd5fca84 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -2228,7 +2228,10 @@ public final class BuildConfiguration {
   }
 
   @SkylarkCallable(name = "coverage_enabled", structField = true,
-      doc = "A boolean that tells whether code coverage is enabled.")
+      doc = "A boolean that tells whether code coverage is enabled for this run. Note that this "
+          + "does not compute whether a specific rule should be instrumented for code coverage "
+          + "data collection. For that, see the <a href=\"ctx.html#coverage_instrumented\"><code>"
+          + "ctx.coverage_instrumented</code></a> function.")
   public boolean isCodeCoverageEnabled() {
     return options.collectCodeCoverage;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
index ae6a630955..50fe8510cf 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
@@ -31,6 +31,7 @@ import com.google.devtools.build.lib.analysis.LabelExpander.NotUniqueExpansionEx
 import com.google.devtools.build.lib.analysis.MakeVariableExpander.ExpansionException;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.FragmentCollection;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -47,8 +48,11 @@ import com.google.devtools.build.lib.packages.RawAttributeMapper;
 import com.google.devtools.build.lib.packages.SkylarkAspect;
 import com.google.devtools.build.lib.packages.SkylarkClassObject;
 import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor;
+import com.google.devtools.build.lib.rules.test.InstrumentedFilesCollector;
+import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
 import com.google.devtools.build.lib.shell.ShellUtils;
 import com.google.devtools.build.lib.shell.ShellUtils.TokenizationException;
+import com.google.devtools.build.lib.skylarkinterface.Param;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
@@ -514,6 +518,35 @@ public final class SkylarkRuleContext {
     return ruleContext.getHostConfiguration();
   }
 
+  @SkylarkCallable(name = "coverage_instrumented",
+    doc = "Returns whether code coverage instrumentation should be generated when performing "
+        + "compilation actions for this rule or, if <code>target</code> is provided, the rule "
+        + "specified by that Target. (If a non-rule Target is provided, this returns False.) This "
+        + "differs from <code>coverage_enabled</code> in the <a href=\"configuration.html\">"
+        + "configuration</a>, which notes whether coverage data collection is enabled for the "
+        + "entire run, but not whether a specific target should be instrumented.",
+    parameters = {
+      @Param(
+          name = "target",
+          type = TransitiveInfoCollection.class,
+          defaultValue = "None",
+          noneable = true,
+          named = true,
+          doc = "A Target specifying a rule. If not provided, defaults to the current rule.")
+    })
+  public boolean instrumentCoverage(Object targetUnchecked) {
+    BuildConfiguration config = ruleContext.getConfiguration();
+    if (!config.isCodeCoverageEnabled()) {
+      return false;
+    }
+    if (targetUnchecked == Runtime.NONE) {
+      return InstrumentedFilesCollector.shouldIncludeLocalSources(ruleContext);
+    }
+    TransitiveInfoCollection target = (TransitiveInfoCollection) targetUnchecked;
+    return (target.getProvider(InstrumentedFilesProvider.class) != null)
+        && InstrumentedFilesCollector.shouldIncludeLocalSources(config, target);
+  }
+
   @SkylarkCallable(name = "features", structField = true,
       doc = "Returns the set of features that are enabled for this rule."
   )
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesCollector.java b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesCollector.java
index 1fb2fbfeea..c2ba279bf3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesCollector.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesCollector.java
@@ -174,14 +174,14 @@ public final class InstrumentedFilesCollector {
   }
 
   /**
-   * Return whether the sources included from {@code dep} (a {@link TransitiveInfoCollection}
-   * representing a rule's dependency) should be instrumented according the --instrumentation_filter
-   * and --instrument_test_targets settings in {@code config}.
+   * Return whether the sources included by {@code target} (a {@link TransitiveInfoCollection}
+   * representing a rule) should be instrumented according the --instrumentation_filter and
+   * --instrument_test_targets settings in {@code config}.
    */
   public static boolean shouldIncludeLocalSources(BuildConfiguration config,
-      TransitiveInfoCollection dep) {
-    return shouldIncludeLocalSources(config, dep.getLabel(),
-        dep.getProvider(TestProvider.class) != null);
+      TransitiveInfoCollection target) {
+    return shouldIncludeLocalSources(config, target.getLabel(),
+        target.getProvider(TestProvider.class) != null);
   }
 
   private static boolean shouldIncludeLocalSources(BuildConfiguration config, Label label,
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index 8f200c340a..15277ff181 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -1296,4 +1296,76 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
     assertThat(substitutionsUnchecked).isInstanceOf(SkylarkDict.class);
     assertThat(substitutionsUnchecked).isEqualTo(SkylarkDict.of(null, "a", "b"));
   }
+
+  private void setUpCoverageInstrumentedTest() throws Exception {
+    scratch.file("test/BUILD",
+        "cc_library(",
+        "  name = 'foo',",
+        "  srcs = ['foo.cc'],",
+        "  deps = [':bar'],",
+        ")",
+        "cc_library(",
+        "  name = 'bar',",
+        "  srcs = ['bar.cc'],",
+        ")");
+  }
+
+  @Test
+  public void testCoverageInstrumentedCoverageDisabled() throws Exception {
+    setUpCoverageInstrumentedTest();
+    useConfiguration("--nocollect_code_coverage", "--instrumentation_filter=.");
+    SkylarkRuleContext ruleContext = createRuleContext("//test:foo");
+    Object result = evalRuleContextCode(ruleContext, "ruleContext.coverage_instrumented()");
+    assertThat((Boolean) result).isFalse();
+  }
+
+  @Test
+  public void testCoverageInstrumentedFalseForSourceFileLabel() throws Exception {
+    setUpCoverageInstrumentedTest();
+    useConfiguration("--collect_code_coverage", "--instrumentation_filter=.");
+    SkylarkRuleContext ruleContext = createRuleContext("//test:foo");
+    Object result = evalRuleContextCode(ruleContext,
+        "ruleContext.coverage_instrumented(ruleContext.attr.srcs[0])");
+    assertThat((Boolean) result).isFalse();
+  }
+
+  @Test
+  public void testCoverageInstrumentedDoesNotMatchFilter() throws Exception {
+    setUpCoverageInstrumentedTest();
+    useConfiguration("--collect_code_coverage", "--instrumentation_filter=:foo");
+    SkylarkRuleContext ruleContext = createRuleContext("//test:bar");
+    Object result = evalRuleContextCode(ruleContext, "ruleContext.coverage_instrumented()");
+    assertThat((Boolean) result).isFalse();
+  }
+
+  @Test
+  public void testCoverageInstrumentedMatchesFilter() throws Exception {
+    setUpCoverageInstrumentedTest();
+    useConfiguration("--collect_code_coverage", "--instrumentation_filter=:foo");
+    SkylarkRuleContext ruleContext = createRuleContext("//test:foo");
+    Object result = evalRuleContextCode(ruleContext, "ruleContext.coverage_instrumented()");
+    assertThat((Boolean) result).isTrue();
+  }
+
+  @Test
+  public void testCoverageInstrumentedDoesNotMatchFilterNonDefaultLabel() throws Exception {
+    setUpCoverageInstrumentedTest();
+    useConfiguration("--collect_code_coverage", "--instrumentation_filter=:foo");
+    SkylarkRuleContext ruleContext = createRuleContext("//test:foo");
+    // //test:bar does not match :foo, though //test:foo would.
+    Object result = evalRuleContextCode(ruleContext,
+        "ruleContext.coverage_instrumented(ruleContext.attr.deps[0])");
+    assertThat((Boolean) result).isFalse();
+  }
+
+  @Test
+  public void testCoverageInstrumentedMatchesFilterNonDefaultLabel() throws Exception {
+    setUpCoverageInstrumentedTest();
+    useConfiguration("--collect_code_coverage", "--instrumentation_filter=:bar");
+    SkylarkRuleContext ruleContext = createRuleContext("//test:foo");
+    // //test:bar does match :bar, though //test:foo would not.
+    Object result = evalRuleContextCode(ruleContext,
+        "ruleContext.coverage_instrumented(ruleContext.attr.deps[0])");
+    assertThat((Boolean) result).isTrue();
+  }
 }