Remove the ability for an aspect to propagate a different aspect.

This functionality is never used, have never been exposed to Skylark
and is a continuous pain to maintain and test.

--
PiperOrigin-RevId: 145079832
MOS_MIGRATED_REVID=145079832

2 files changed, 47 insertions, 75 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
index 58b8fea8ee..55b0616d2e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
@@ -14,12 +14,9 @@
 
 package com.google.devtools.build.lib.packages;
 
-import com.google.common.collect.ImmutableCollection;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.SetMultimap;
@@ -30,6 +27,7 @@ import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.Preconditions;
 import java.util.Collection;
 import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import javax.annotation.Nullable;
@@ -53,31 +51,34 @@ import javax.annotation.Nullable;
  */
 @Immutable
 public final class AspectDefinition {
-
   private final AspectClass aspectClass;
   private final RequiredProviders requiredProviders;
   private final RequiredProviders requiredProvidersForAspects;
   private final ImmutableMap<String, Attribute> attributes;
-  private final PropagationFunction attributeAspects;
-  @Nullable private final ConfigurationFragmentPolicy configurationFragmentPolicy;
 
-  private interface PropagationFunction {
-    ImmutableCollection<AspectClass> propagate(Attribute attribute);
-  }
+  /**
+   * Which attributes aspect should propagate along:
+   * <ul>
+   *  <li>A {@code null} value means propagate along all attributes</li>
+   *  <li>A (possibly empty) set means to propagate only along the attributes in a set</li>
+   * </ul>
+   */
+  @Nullable private final ImmutableSet<String> restrictToAttributes;
+  @Nullable private final ConfigurationFragmentPolicy configurationFragmentPolicy;
 
   private AspectDefinition(
       AspectClass aspectClass,
       RequiredProviders requiredProviders,
       RequiredProviders requiredAspectProviders,
       ImmutableMap<String, Attribute> attributes,
-      PropagationFunction attributeAspects,
+      @Nullable ImmutableSet<String> restrictToAttributes,
       @Nullable ConfigurationFragmentPolicy configurationFragmentPolicy) {
     this.aspectClass = aspectClass;
     this.requiredProviders = requiredProviders;
     this.requiredProvidersForAspects = requiredAspectProviders;
 
     this.attributes = attributes;
-    this.attributeAspects = attributeAspects;
+    this.restrictToAttributes = restrictToAttributes;
     this.configurationFragmentPolicy = configurationFragmentPolicy;
   }
 
@@ -119,10 +120,13 @@ public final class AspectDefinition {
 
 
   /**
-   * Returns the set of required aspects for a given atribute.
+   * Returns the set of required aspects for a given attribute.
    */
-  public ImmutableCollection<AspectClass> getAttributeAspects(Attribute attribute) {
-    return attributeAspects.propagate(attribute);
+  public boolean propagateAlong(Attribute attribute) {
+    if (restrictToAttributes != null) {
+      return restrictToAttributes.contains(attribute.getName());
+    }
+    return true;
   }
 
   /**
@@ -216,8 +220,8 @@ public final class AspectDefinition {
     private RequiredProviders.Builder requiredProviders = RequiredProviders.acceptAnyBuilder();
     private RequiredProviders.Builder requiredAspectProviders =
         RequiredProviders.acceptNoneBuilder();
-    private final Multimap<String, AspectClass> attributeAspects = LinkedHashMultimap.create();
-    private ImmutableCollection<AspectClass> allAttributesAspects = null;
+    @Nullable
+    private LinkedHashSet<String> propagateAlongAttributes = new LinkedHashSet<>();
     private final ConfigurationFragmentPolicy.Builder configurationFragmentPolicy =
         new ConfigurationFragmentPolicy.Builder();
 
@@ -265,42 +269,38 @@ public final class AspectDefinition {
 
 
     /**
-     * Declares that this aspect depends on the given aspects in {@code aspectFactories} provided
-     * by direct dependencies through attribute {@code attribute} on the target associated with this
-     * aspect.
+     * Declares that this aspect propagates along an {@code attribute} on the target
+     * associated with this aspect.
+     *
+     * Specify multiple attributes by calling {@link #propagateAlongAttribute(String)}
+     * repeatedly.
      *
-     * <p>Note that {@code ConfiguredAspectFactory} instances are expected in the second argument,
-     * but we cannot reference that interface here.
+     * Aspect can also declare to propagate along all attributes with
+     * {@link #propagateAlongAttributes}.
      */
-    public final Builder attributeAspect(String attribute, NativeAspectClass... aspectClasses) {
+    public final Builder propagateAlongAttribute(String attribute) {
       Preconditions.checkNotNull(attribute);
-      for (NativeAspectClass aspectClass : aspectClasses) {
-        this.attributeAspect(attribute, Preconditions.checkNotNull(aspectClass));
-      }
+      Preconditions.checkState(this.propagateAlongAttributes != null,
+          "Either propagate along all attributes, or along specific attributes, not both");
+
+      this.propagateAlongAttributes.add(attribute);
+
       return this;
     }
 
     /**
-     * Declares that this aspect depends on the given {@link AspectClass} provided
-     * by direct dependencies through attribute {@code attribute} on the target associated with this
-     * aspect.
+     * Declares that this aspect propagates along all attributes on the target
+     * associated with this aspect.
+     *
+     * Specify either this or {@link #propagateAlongAttribute(String)}, not both.
      */
-    public final Builder attributeAspect(String attribute, AspectClass aspectClass) {
-      Preconditions.checkNotNull(attribute);
-      Preconditions.checkState(this.allAttributesAspects == null,
-          "Specify either aspects for all attributes, or for specific attributes, not both");
-
-      this.attributeAspects.put(attribute, Preconditions.checkNotNull(aspectClass));
-
-      return this;
-    }
+    public final Builder propagateAlongAllAttributes() {
+      Preconditions.checkState(this.propagateAlongAttributes != null,
+          "Aspects for all attributes must only be specified once");
 
-    public final Builder allAttributesAspect(AspectClass... aspectClasses) {
-      Preconditions.checkState(this.attributeAspects.isEmpty(),
+      Preconditions.checkState(this.propagateAlongAttributes.isEmpty(),
           "Specify either aspects for all attributes, or for specific attributes, not both");
-      Preconditions.checkState(this.allAttributesAspects == null,
-          "Aspects for all attributes must only be specified once");
-      this.allAttributesAspects = ImmutableList.copyOf(aspectClasses);
+      this.propagateAlongAttributes = null;
       return this;
     }
 
@@ -394,34 +394,6 @@ public final class AspectDefinition {
       return this;
     }
 
-    @Immutable
-    private static final class AllAttributesPropagationFunction implements PropagationFunction {
-      private final ImmutableCollection<AspectClass> aspects;
-
-      private AllAttributesPropagationFunction(ImmutableCollection<AspectClass> aspects) {
-        this.aspects = aspects;
-      }
-
-      @Override
-      public ImmutableCollection<AspectClass> propagate(Attribute attribute) {
-        return aspects;
-      }
-    }
-
-    @Immutable
-    private static final class PerAttributePropagationFunction implements PropagationFunction {
-      ImmutableSetMultimap<String, AspectClass> aspects;
-
-      public PerAttributePropagationFunction(
-          ImmutableSetMultimap<String, AspectClass> aspects) {
-        this.aspects = aspects;
-      }
-
-      @Override
-      public ImmutableCollection<AspectClass> propagate(Attribute attribute) {
-        return aspects.get(attribute.getName());
-      }
-    }
 
     /**
      * Builds the aspect definition.
@@ -433,9 +405,9 @@ public final class AspectDefinition {
           requiredProviders.build(),
           requiredAspectProviders.build(),
           ImmutableMap.copyOf(attributes),
-          allAttributesAspects != null
-              ? new AllAttributesPropagationFunction(allAttributesAspects)
-              : new PerAttributePropagationFunction(ImmutableSetMultimap.copyOf(attributeAspects)),
+          propagateAlongAttributes == null
+              ? null
+              : ImmutableSet.copyOf(propagateAlongAttributes),
           configurationFragmentPolicy.build());
     }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkAspect.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkAspect.java
index 5d04a5cf22..6c393ae71e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkAspect.java
@@ -121,10 +121,10 @@ public class SkylarkAspect implements SkylarkExportable {
   public AspectDefinition getDefinition(AspectParameters aspectParams) {
     AspectDefinition.Builder builder = new AspectDefinition.Builder(aspectClass);
     if (allAttrAspects.equals(attributeAspects)) {
-      builder.allAttributesAspect(aspectClass);
+      builder.propagateAlongAllAttributes();
     } else {
       for (String attributeAspect : attributeAspects) {
-        builder.attributeAspect(attributeAspect, aspectClass);
+        builder.propagateAlongAttribute(attributeAspect);
       }
     }