Clean up --experimental_ignore_deprecated_instrumentation_spec

RELNOTES: Removed --experimental_ignore_deprecated_instrumentation_spec and cleaned up the old deprecated behavior.
PiperOrigin-RevId: 325824279

src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java

@@ -661,10 +661,6 @@ public boolean experimentalForwardInstrumentedFilesInfoByDefault() {
     return options.experimentalForwardInstrumentedFilesInfoByDefault;
   }
 
-  public boolean experimentalIgnoreDeprecatedInstrumentationSpec() {
-    return options.experimentalIgnoreDeprecatedInstrumentationSpec;
-  }
-
   public RunUnder getRunUnder() {
     return options.runUnder;
   }

src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java

@@ -386,17 +386,6 @@ public String getTypeDescription() {
               + "contents of InstrumentedFilesInfo from transitive dependencies.")
   public boolean experimentalForwardInstrumentedFilesInfoByDefault;
 
-  @Option(
-      name = "experimental_ignore_deprecated_instrumentation_spec",
-      defaultValue = "true",
-      documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
-      effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
-      help =
-          "If specified, use a new configuration of InstrumentationSpec which distinguishes "
-              + "between source and dependency attributes for rules which still used a legacy "
-              + "configuration with a single list of coverage-relevant attributes.")
-  public boolean experimentalIgnoreDeprecatedInstrumentationSpec;
-
   @Option(
       name = "build_runfile_manifests",
       defaultValue = "true",

src/main/java/com/google/devtools/build/lib/analysis/test/InstrumentedFilesCollector.java

@@ -36,7 +36,6 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
-import java.util.Optional;
 import javax.annotation.Nullable;
 
 /**
@@ -181,15 +180,8 @@ public static InstrumentedFilesInfo collect(
             ruleContext, coverageSupportFiles, coverageEnvironment, reportedToActualSources);
 
     // Transitive instrumentation data.
-    boolean useDeprecatedSourceOrDependencyAttributes =
-        spec.deprecatedSourceOrDependencyAttributes.isPresent()
-            && !ruleContext.getConfiguration().experimentalIgnoreDeprecatedInstrumentationSpec();
     for (TransitiveInfoCollection dep :
-        getPrerequisitesForAttributes(
-            ruleContext,
-            useDeprecatedSourceOrDependencyAttributes
-                ? spec.deprecatedSourceOrDependencyAttributes.get()
-                : spec.dependencyAttributes)) {
+        getPrerequisitesForAttributes(ruleContext, spec.dependencyAttributes)) {
       instrumentedFilesInfoBuilder.addFromDependency(dep);
     }
 
@@ -199,15 +191,7 @@ public static InstrumentedFilesInfo collect(
         ruleContext.getConfiguration(), ruleContext.getLabel(), ruleContext.isTestTarget())) {
       NestedSetBuilder<Artifact> localSourcesBuilder = NestedSetBuilder.stableOrder();
       for (TransitiveInfoCollection dep :
-          getPrerequisitesForAttributes(
-              ruleContext,
-              useDeprecatedSourceOrDependencyAttributes
-                  ? spec.deprecatedSourceOrDependencyAttributes.get()
-                  : spec.sourceAttributes)) {
-        if (useDeprecatedSourceOrDependencyAttributes
-            && dep.get(InstrumentedFilesInfo.STARLARK_CONSTRUCTOR) != null) {
-          continue;
-        }
+          getPrerequisitesForAttributes(ruleContext, spec.sourceAttributes)) {
         for (Artifact artifact : dep.getProvider(FileProvider.class).getFilesToBuild().toList()) {
           if (artifact.isSourceArtifact() &&
               spec.instrumentedFileTypes.matches(artifact.getFilename())) {
@@ -264,9 +248,6 @@ public static boolean shouldIncludeLocalSources(
   public static final class InstrumentationSpec {
     private final FileTypeSet instrumentedFileTypes;
 
-    /** Deprecated list of attributes which should be checked for sources or dependencies. */
-    private final Optional<ImmutableList<String>> deprecatedSourceOrDependencyAttributes;
-
     /** The list of attributes which should be checked for sources. */
     private final ImmutableList<String> sourceAttributes;
 
@@ -276,17 +257,14 @@ public static final class InstrumentationSpec {
     private InstrumentationSpec(
         FileTypeSet instrumentedFileTypes,
         ImmutableList<String> instrumentedSourceAttributes,
-        ImmutableList<String> instrumentedDependencyAttributes,
-        Optional<ImmutableList<String>> deprecatedInstrumentedSourceOrDependencyAttributes) {
+        ImmutableList<String> instrumentedDependencyAttributes) {
       this.instrumentedFileTypes = instrumentedFileTypes;
       this.sourceAttributes = instrumentedSourceAttributes;
       this.dependencyAttributes = instrumentedDependencyAttributes;
-      this.deprecatedSourceOrDependencyAttributes =
-          deprecatedInstrumentedSourceOrDependencyAttributes;
     }
 
     public InstrumentationSpec(FileTypeSet instrumentedFileTypes) {
-      this(instrumentedFileTypes, ImmutableList.of(), ImmutableList.of(), Optional.empty());
+      this(instrumentedFileTypes, ImmutableList.of(), ImmutableList.of());
     }
 
     /**
@@ -295,10 +273,7 @@ public InstrumentationSpec(FileTypeSet instrumentedFileTypes) {
      */
     public InstrumentationSpec withSourceAttributes(Collection<String> attributes) {
       return new InstrumentationSpec(
-          instrumentedFileTypes,
-          ImmutableList.copyOf(attributes),
-          dependencyAttributes,
-          deprecatedSourceOrDependencyAttributes);
+          instrumentedFileTypes, ImmutableList.copyOf(attributes), dependencyAttributes);
     }
 
     /**
@@ -315,10 +290,7 @@ public InstrumentationSpec withSourceAttributes(String... attributes) {
      */
     public InstrumentationSpec withDependencyAttributes(Collection<String> attributes) {
       return new InstrumentationSpec(
-          instrumentedFileTypes,
-          sourceAttributes,
-          ImmutableList.copyOf(attributes),
-          deprecatedSourceOrDependencyAttributes);
+          instrumentedFileTypes, sourceAttributes, ImmutableList.copyOf(attributes));
     }
 
     /**
@@ -328,27 +300,6 @@ public InstrumentationSpec withDependencyAttributes(Collection<String> attribute
     public InstrumentationSpec withDependencyAttributes(String... attributes) {
       return withDependencyAttributes(ImmutableList.copyOf(attributes));
     }
-
-    /**
-     * Returns a new instrumentation spec with the given attribute names to look for sources _or_
-     * dependencies in the legacy behavior replacing the ones stored in this object.
-     */
-    public InstrumentationSpec withDeprecatedSourceOrDependencyAttributes(
-        Collection<String> attributes) {
-      return new InstrumentationSpec(
-          instrumentedFileTypes,
-          sourceAttributes,
-          dependencyAttributes,
-          Optional.of(ImmutableList.copyOf(attributes)));
-    }
-
-    /**
-     * Returns a new instrumentation spec with the given attribute names to look for sources _or_
-     * dependencies in the legacy behavior replacing the ones stored in this object.
-     */
-    public InstrumentationSpec withDeprecatedSourceOrDependencyAttributes(String... attributes) {
-      return withDeprecatedSourceOrDependencyAttributes(ImmutableList.copyOf(attributes));
-    }
   }
 
   /**

src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java

@@ -70,7 +70,6 @@ public class BazelPythonSemantics implements PythonSemantics {
       Template.forResource(BazelPythonSemantics.class, "python_stub_template.txt");
   public static final InstrumentationSpec PYTHON_COLLECTION_SPEC =
       new InstrumentationSpec(FileTypeSet.of(BazelPyRuleClasses.PYTHON_SOURCE))
-          .withDeprecatedSourceOrDependencyAttributes("srcs", "deps", "data")
           .withSourceAttributes("srcs")
           .withDependencyAttributes("deps", "data");
 

src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShCoverage.java

@@ -23,7 +23,6 @@ private ShCoverage() {}
 
   public static final InstrumentationSpec INSTRUMENTATION_SPEC =
       new InstrumentationSpec(FileTypeSet.ANY_FILE)
-          .withDeprecatedSourceOrDependencyAttributes("srcs", "deps", "data")
           .withSourceAttributes("srcs")
           .withDependencyAttributes("deps", "data");
 }

src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java

@@ -73,23 +73,20 @@ public ConfiguredTarget create(RuleContext ruleContext)
         InstrumentedFilesCollector.collectTransitive(
             ruleContext,
             // Seems strange to have "srcs" in "dependency attributes" instead of "source
-            // attributes", but that's correct behavior here because:
-            // 1. This rule is essentially forwarding, it has no idea how the stuff in srcs is used.
-            //    Thus, it needs to look at any dependencies transitively via
-            //    InstrumentedFilesProvider.
-            // 2. This rule doesn't _process_ any source files. The rule which does process the
-            //    source files in filegroup.srcs will include those files in its inputs and in its
-            //    InstrumentedFileProvider the same way, via FileProvider. This ensures that when
-            //    --instrumentation_filter says a rule's sources should be instrumented for coverage
-            //    data collection, it also says all of those sources should be included in the
-            //    coverage manifest.
-            // Previously, this would have needed to include "srcs" in "source attributes" anyways,
-            // since it might have been _consumed_ by a rule using the legacy InstrumentationSpec.
-            // In that case, since filegroup provided InstrumentedFilesProvider, the legacy
-            // consumer would never try to gather filegroup's instrumented sources via FileProvider.
-            new InstrumentationSpec(FileTypeSet.ANY_FILE)
-                .withDeprecatedSourceOrDependencyAttributes("srcs", "deps", "data")
-                .withDependencyAttributes("srcs", "data"),
+            // attributes", but that's correct behavior here because this rule just forwards
+            // files, it doesn't process them. It doesn't know if the dependencies of the stuff
+            // in srcs is a runtime dependency of its consumers or not. Consumers decide which
+            // of the following is the case about a filegroup it depends on based on whether the
+            // attribute the dependency is via is in the consumer's source attributes or
+            // dependency attributes:
+            // * If the filegroup contains coverage-relevant source files, it should be depended
+            //   on via something in source attributes. The dependencies for actions which generate
+            //   source files are generally not runtime dependencies.
+            // * If the dependencies of the filegroup might be coverage-relevant source files (e.g.
+            //   a binary target is included in filegroup's srcs and the filegroup target is
+            //   included in some other target's data), it should be depended on via something in
+            //   dependency attributes.
+            new InstrumentationSpec(FileTypeSet.ANY_FILE).withDependencyAttributes("srcs", "data"),
             /* reportedToActualSources= */ NestedSetBuilder.create(Order.STABLE_ORDER));
 
     RunfilesProvider runfilesProvider =