Make aspect required fragments logic more robust:

- Any configured target an aspect attaches to is considered a transitive dependency.
- Any aspect an aspect requires is considered a transitive dependency.

PiperOrigin-RevId: 4075932

Author: gregestren

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java

@@ -514,7 +514,7 @@ public ConfiguredAspect createAspect(
                     aspectConfiguration,
                     ruleClassProvider.getFragmentRegistry().getUniversalFragments(),
                     configConditions.asProviders(),
-                    prerequisiteMap.values()))
+                    Iterables.concat(prerequisiteMap.values(), ImmutableList.of(associatedTarget))))
             .build();
 
     // If allowing analysis failures, targets should be created as normal as possible, and errors

src/test/java/com/google/devtools/build/lib/analysis/RequiredConfigFragmentsTest.java

@@ -18,6 +18,7 @@
 import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
 
 import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
+import com.google.devtools.build.lib.analysis.config.CoreOptions.IncludeConfigFragmentsEnum;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.packages.AspectDefinition;
 import com.google.devtools.build.lib.packages.AspectParameters;
@@ -31,12 +32,13 @@
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
 import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
 import com.google.devtools.build.lib.util.FileTypeSet;
+import com.google.testing.junit.testparameterinjector.TestParameter;
+import com.google.testing.junit.testparameterinjector.TestParameterInjector;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
 
 /** Tests for {@link RequiredConfigFragmentsProvider}. */
-@RunWith(JUnit4.class)
+@RunWith(TestParameterInjector.class)
 public final class RequiredConfigFragmentsTest extends BuildViewTestCase {
 
   @Test
@@ -350,4 +352,93 @@ public void starlarkAttrTransition() throws Exception {
     // But not the child's rule transition.
     assertThat(requiredFragments.getOptionsClasses()).doesNotContain(CppOptions.class);
   }
+
+  @Test
+  public void aspectInheritsTransitiveFragmentsFromBaseCT(
+      @TestParameter({"DIRECT", "TRANSITIVE"}) IncludeConfigFragmentsEnum setting)
+      throws Exception {
+    writeStarlarkTransitionsAndAllowList();
+    scratch.file(
+        "a/defs.bzl",
+        "",
+        "A1Info = provider()",
+        "def _a1_impl(target, ctx):",
+        "  return []",
+        "a1 = aspect(implementation = _a1_impl)",
+        "",
+        "def _java_depender_impl(ctx):",
+        "  return []",
+        "java_depender = rule(",
+        "  implementation = _java_depender_impl,",
+        "  fragments = ['java'],",
+        "  attrs = {})",
+        "",
+        "def _r_impl(ctx):",
+        "  return []",
+        "r = rule(",
+        "  implementation = _r_impl,",
+        "  attrs = {'dep': attr.label(aspects = [a1])})");
+    scratch.file(
+        "a/BUILD",
+        "load(':defs.bzl', 'java_depender', 'r')",
+        "java_depender(name = 'lib')",
+        "r(name = 'r', dep = ':lib')");
+
+    useConfiguration("--include_config_fragments_provider=" + setting);
+    getConfiguredTarget("//a:r");
+    RequiredConfigFragmentsProvider requiredFragments =
+        getAspect("//a:defs.bzl%a1").getProvider(RequiredConfigFragmentsProvider.class);
+
+    if (setting == IncludeConfigFragmentsEnum.TRANSITIVE) {
+      assertThat(requiredFragments.getFragmentClasses()).contains(JavaConfiguration.class);
+    } else {
+      assertThat(requiredFragments.getFragmentClasses()).doesNotContain(JavaConfiguration.class);
+    }
+  }
+
+  @Test
+  public void aspectInheritsTransitiveFragmentsFromRequiredAspect(
+      @TestParameter({"DIRECT", "TRANSITIVE"}) IncludeConfigFragmentsEnum setting)
+      throws Exception {
+    scratch.file(
+        "a/defs.bzl",
+        "",
+        "A1Info = provider()",
+        "def _a1_impl(target, ctx):",
+        "  return A1Info(var = ctx.var.get('my_var', '0'))",
+        "a1 = aspect(implementation = _a1_impl, provides = [A1Info])",
+        "",
+        "A2Info = provider()",
+        "def _a2_impl(target, ctx):",
+        "  return A2Info()",
+        "a2 = aspect(implementation = _a2_impl, required_aspect_providers = [A1Info])",
+        "",
+        "def _simple_rule_impl(ctx):",
+        "  return []",
+        "simple_rule = rule(",
+        "  implementation = _simple_rule_impl,",
+        "  attrs = {})",
+        "",
+        "def _r_impl(ctx):",
+        "  return []",
+        "r = rule(",
+        "  implementation = _r_impl,",
+        "  attrs = {'dep': attr.label(aspects = [a1, a2])})");
+    scratch.file(
+        "a/BUILD",
+        "load(':defs.bzl', 'r', 'simple_rule')",
+        "simple_rule(name = 'lib')",
+        "r(name = 'r', dep = ':lib')");
+
+    useConfiguration("--include_config_fragments_provider=" + setting, "--define", "my_var=1");
+    getConfiguredTarget("//a:r");
+    RequiredConfigFragmentsProvider requiredFragments =
+        getAspect("//a:defs.bzl%a2").getProvider(RequiredConfigFragmentsProvider.class);
+
+    if (setting == IncludeConfigFragmentsEnum.TRANSITIVE) {
+      assertThat(requiredFragments.getDefines()).contains("my_var");
+    } else {
+      assertThat(requiredFragments.getDefines()).doesNotContain("my_var");
+    }
+  }
 }

src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java

@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.analysis.util;
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.MoreCollectors.onlyElement;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.getFirstArtifactEndingWith;
@@ -66,8 +67,10 @@
 import com.google.devtools.build.lib.analysis.AnalysisOptions;
 import com.google.devtools.build.lib.analysis.AnalysisResult;
 import com.google.devtools.build.lib.analysis.AnalysisUtils;
+import com.google.devtools.build.lib.analysis.AspectValue;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment;
+import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.DependencyResolver.Failure;
@@ -192,6 +195,7 @@
 import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Optional;
 import java.util.Set;
@@ -1083,6 +1087,26 @@ protected FileConfiguredTarget getHostFileConfiguredTarget(String label)
     return (FileConfiguredTarget) getHostConfiguredTarget(label);
   }
 
+  /**
+   * Returns the {@link ConfiguredAspect} with the given label. For example: {@code
+   * //my:base_target%my_aspect}.
+   *
+   * <p>Assumes only one configured aspect exists for this label. If this isn't true, or you need
+   * finer grained selection for different configurations, you'll need to expand this method.
+   */
+  protected ConfiguredAspect getAspect(String label) throws Exception {
+    AspectValue aspect =
+        (AspectValue)
+            skyframeExecutor.getEvaluatorForTesting().getDoneValues().entrySet().stream()
+                .filter(
+                    entry ->
+                        entry.getKey() instanceof AspectKey
+                            && ((AspectKey) entry.getKey()).getAspectName().equals(label))
+                .map(Map.Entry::getValue)
+                .collect(onlyElement());
+    return aspect.getConfiguredAspect();
+  }
+
   /**
    * Rewrites the WORKSPACE to have the required boilerplate and the given lines of content.
    *