Allow aspects to propagate to all attributes.

--
MOS_MIGRATED_REVID=134378592

4 files changed, 220 insertions, 8 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java
index 4bf2c55102..667af95af3 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java
@@ -31,7 +31,6 @@ import com.google.devtools.build.lib.packages.NativeAspectClass;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.util.FileTypeSet;
-
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -121,8 +120,30 @@ public class AspectDefinitionTest {
         .attributeAspect("srcs", TEST_ASPECT_CLASS)
         .attributeAspect("deps", TEST_ASPECT_CLASS)
         .build();
-    assertThat(withAspects.getAttributeAspects()).containsEntry("srcs", TEST_ASPECT_CLASS);
-    assertThat(withAspects.getAttributeAspects()).containsEntry("deps", TEST_ASPECT_CLASS);
+
+    assertThat(withAspects.getAttributeAspects(createLabelListAttribute("srcs")))
+        .containsExactly(TEST_ASPECT_CLASS);
+    assertThat(withAspects.getAttributeAspects(createLabelListAttribute("deps")))
+        .containsExactly(TEST_ASPECT_CLASS);
+  }
+
+  @Test
+  public void testAttributeAspect_AllAttributes() throws Exception {
+    AspectDefinition withAspects = new AspectDefinition.Builder("attribute_aspect")
+        .allAttributesAspect(TEST_ASPECT_CLASS)
+        .build();
+
+    assertThat(withAspects.getAttributeAspects(createLabelListAttribute("srcs")))
+        .containsExactly(TEST_ASPECT_CLASS);
+    assertThat(withAspects.getAttributeAspects(createLabelListAttribute("deps")))
+        .containsExactly(TEST_ASPECT_CLASS);
+  }
+
+
+  private static Attribute createLabelListAttribute(String name) {
+    return Attribute.attr(name, BuildType.LABEL_LIST)
+        .allowedFileTypes(FileTypeSet.ANY_FILE)
+        .build();
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
index 9274b759b0..a2e9d23191 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -425,6 +425,80 @@ public class AspectTest extends AnalysisTestCase {
         a.getProvider(ExtraActionArtifactsProvider.class)
             .getTransitiveExtraActionArtifacts();
     assertThat(getOnlyElement(extraActionArtifacts).getLabel()).isEqualTo(Label.create("@//a", "b"));
+  }
+
+  @Test
+  public void aspectPropagatesToAllAttributes() throws Exception {
+    setRulesAvailableInTests(new TestAspects.BaseRule(),
+        new TestAspects.SimpleRule(),
+        new TestAspects.AllAttributesAspectRule());
+    pkg("a",
+        "simple(name='a', foo=[':b'], foo1=':c', txt='some text')",
+        "simple(name='b', foo=[], txt='some text')",
+        "simple(name='c', foo=[], txt='more text')",
+        "all_attributes_aspect(name='x', foo=[':a'])");
+
+    ConfiguredTarget a = getConfiguredTarget("//a:x");
+    assertThat(a.getProvider(RuleInfo.class).getData())
+        .containsExactly("aspect //a:a",  "aspect //a:b", "aspect //a:c", "rule //a:x");
+  }
+
+  @Test
+  public void aspectPropagatesToAllAttributesImplicit() throws Exception {
+    setRulesAvailableInTests(new TestAspects.BaseRule(),
+        new TestAspects.SimpleRule(),
+        new TestAspects.ImplicitDepRule(),
+        new TestAspects.AllAttributesAspectRule());
+
+    scratch.file(
+        "extra/BUILD",
+        "simple(name ='extra')"
+    );
+    pkg("a",
+        "simple(name='a', foo=[':b'], foo1=':c', txt='some text')",
+        "simple(name='b', foo=[], txt='some text')",
+        "implicit_dep(name='c')",
+        "all_attributes_aspect(name='x', foo=[':a'])");
+    update();
 
+    ConfiguredTarget a = getConfiguredTarget("//a:x");
+    assertThat(a.getProvider(RuleInfo.class).getData())
+        .containsExactly(
+            "aspect //a:a",
+            "aspect //a:b",
+            "aspect //a:c",
+            "aspect //extra:extra",
+            "rule //a:x");
   }
+
+
+  @Test
+  public void aspectPropagatesToAllAttributesLateBound() throws Exception {
+    setRulesAvailableInTests(new TestAspects.BaseRule(),
+        new TestAspects.SimpleRule(),
+        new TestAspects.LateBoundDepRule(),
+        new TestAspects.AllAttributesAspectRule());
+
+    scratch.file(
+        "extra/BUILD",
+        "simple(name ='extra')"
+    );
+    pkg("a",
+        "simple(name='a', foo=[':b'], foo1=':c', txt='some text')",
+        "simple(name='b', foo=[], txt='some text')",
+        "late_bound_dep(name='c')",
+        "all_attributes_aspect(name='x', foo=[':a'])");
+    useConfiguration("--plugin=//extra:extra");
+    update();
+
+    ConfiguredTarget a = getConfiguredTarget("//a:x");
+    assertThat(a.getProvider(RuleInfo.class).getData())
+        .containsExactly(
+            "aspect //a:a",
+            "aspect //a:b",
+            "aspect //a:c",
+            "aspect //extra:extra",
+            "rule //a:x");
+  }
+
 }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java
index be4988ecaf..d498709148 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java
@@ -33,7 +33,6 @@ import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.NoSuchThingException;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.util.OrderedSetMultimap;
-
 import java.util.List;
 import java.util.Set;
 import javax.annotation.Nullable;
@@ -162,6 +161,19 @@ public class DependencyResolverTest extends AnalysisTestCase {
   }
 
   @Test
+  public void hasAllAttributesAspect() throws Exception {
+    setRulesAvailableInTests(new TestAspects.BaseRule(), new TestAspects.SimpleRule());
+    pkg("a",
+        "simple(name='a', foo=[':b'])",
+        "simple(name='b', foo=[])");
+    OrderedSetMultimap<Attribute, Dependency> map =
+        dependentNodeMap("//a:a", TestAspects.ALL_ATTRIBUTES_ASPECT);
+    assertDep(
+        map, "foo", "//a:b",
+        new AspectDescriptor(TestAspects.ALL_ATTRIBUTES_ASPECT));
+  }
+
+  @Test
   public void hasAspectDependencies() throws Exception {
     setRulesAvailableInTests(new TestAspects.BaseRule());
     pkg("a", "base(name='a')");
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
index 351ae384ef..263a795dbf 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
@@ -38,6 +38,7 @@ import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -47,12 +48,14 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.AspectDefinition;
 import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel;
+import com.google.devtools.build.lib.packages.Attribute.LateBoundLabelList;
 import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.NativeAspectClass;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
+import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileTypeSet;
 import java.util.List;
 
@@ -116,12 +119,19 @@ public class TestAspects {
   private static NestedSet<String> collectAspectData(String me, RuleContext ruleContext) {
     NestedSetBuilder<String> result = new NestedSetBuilder<>(Order.STABLE_ORDER);
     result.add(me);
-    if (ruleContext.attributes().has("foo", LABEL_LIST)) {
-      for (AspectInfo dep : ruleContext.getPrerequisites("foo", Mode.TARGET, AspectInfo.class)) {
-        result.addTransitive(dep.getData());
+
+    Iterable<String> attributeNames = ruleContext.attributes().getAttributeNames();
+    for (String attributeName : attributeNames) {
+      Type<?> attributeType = ruleContext.attributes().getAttributeType(attributeName);
+      if (!LABEL.equals(attributeType) && !LABEL_LIST.equals(attributeType)) {
+        continue;
+      }
+      Iterable<AspectInfo> prerequisites = ruleContext
+          .getPrerequisites(attributeName, Mode.DONT_CHECK, AspectInfo.class);
+      for (AspectInfo prerequisite : prerequisites) {
+        result.addTransitive(prerequisite.getData());
       }
     }
-
     return result.build();
   }
 
@@ -212,6 +222,24 @@ public class TestAspects {
       .build();
 
   /**
+   * An aspect that propagates along all attributes.
+   */
+  public static class AllAttributesAspect extends BaseAspect {
+
+    @Override
+    public AspectDefinition getDefinition(AspectParameters aspectParameters) {
+      return ALL_ATTRIBUTES_ASPECT_DEFINITION;
+    }
+  }
+  public static final NativeAspectClass ALL_ATTRIBUTES_ASPECT = new AllAttributesAspect();
+  private static final AspectDefinition ALL_ATTRIBUTES_ASPECT_DEFINITION =
+      new AspectDefinition.Builder("all_attributes_aspect")
+          .allAttributesAspect(ALL_ATTRIBUTES_ASPECT)
+          .build();
+
+
+
+  /**
    * An aspect that requires aspects on the attributes of rules it attaches to.
    */
   public static class AttributeAspect extends BaseAspect {
@@ -525,6 +553,29 @@ public class TestAspects {
   }
 
   /**
+   * A rule that defines an {@link AllAttributesAspect} on one of its attributes.
+   */
+  public static class AllAttributesAspectRule implements RuleDefinition {
+
+    @Override
+    public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) {
+      return builder
+          .add(attr("foo", LABEL_LIST).allowedFileTypes(FileTypeSet.ANY_FILE)
+              .aspect(ALL_ATTRIBUTES_ASPECT))
+          .build();
+    }
+
+    @Override
+    public Metadata getMetadata() {
+      return RuleDefinition.Metadata.builder()
+          .name("all_attributes_aspect")
+          .factoryClass(DummyRuleFactory.class)
+          .ancestors(BaseRule.class)
+          .build();
+    }
+  }
+
+  /**
    * A rule that defines a {@link WarningAspect} on one of its attributes.
    */
   public static class WarningAspectRule implements RuleDefinition {
@@ -578,6 +629,8 @@ public class TestAspects {
     public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) {
       return builder
           .add(attr("foo", LABEL_LIST).allowedFileTypes(FileTypeSet.ANY_FILE))
+          .add(attr("foo1", LABEL).allowedFileTypes(FileTypeSet.ANY_FILE))
+          .add(attr("txt", STRING))
           .build();
     }
 
@@ -634,4 +687,56 @@ public class TestAspects {
           .build();
     }
   }
+
+  /**
+   * Rule with an implcit dependency.
+   */
+  public static class ImplicitDepRule implements RuleDefinition {
+    @Override
+    public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) {
+      return builder
+          .add(attr("$dep", LABEL).value(Label.parseAbsoluteUnchecked("//extra:extra")))
+          .build();
+    }
+
+    @Override
+    public Metadata getMetadata() {
+      return RuleDefinition.Metadata.builder()
+          .name("implicit_dep")
+          .factoryClass(DummyRuleFactory.class)
+          .ancestors(BaseRule.class)
+          .build();
+    }
+  }
+
+  /**
+   * Rule with a late-bound dependency.
+   */
+  public static class LateBoundDepRule implements RuleDefinition {
+    private static final LateBoundLabelList<BuildConfiguration> PLUGINS_LABEL_LIST =
+        new LateBoundLabelList<BuildConfiguration>() {
+          @Override
+          public List<Label> resolve(Rule rule, AttributeMap attributes,
+              BuildConfiguration configuration) {
+            return configuration.getPlugins();
+          }
+        };
+
+    @Override
+    public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) {
+      return builder
+          .add(attr(":plugins", LABEL_LIST).value(PLUGINS_LABEL_LIST))
+          .build();
+    }
+
+    @Override
+    public Metadata getMetadata() {
+      return RuleDefinition.Metadata.builder()
+          .name("late_bound_dep")
+          .factoryClass(DummyRuleFactory.class)
+          .ancestors(BaseRule.class)
+          .build();
+    }
+  }
+
 }