Encapsulate the required provider logic in RequiredProviders class.

For now, only for aspects, but eventually expand to Attribute's
mandatory providers as well.

--
PiperOrigin-RevId: 144063369
MOS_MIGRATED_REVID=144063369

5 files changed, 273 insertions, 110 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
index f024fb225e..2060e81ec9 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
@@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
@@ -26,7 +25,6 @@ import com.google.devtools.build.lib.analysis.config.InvalidConfigurationExcepti
 import com.google.devtools.build.lib.analysis.config.PatchTransition;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-import com.google.devtools.build.lib.packages.AdvertisedProviderSet;
 import com.google.devtools.build.lib.packages.Aspect;
 import com.google.devtools.build.lib.packages.AspectClass;
 import com.google.devtools.build.lib.packages.AspectDescriptor;
@@ -543,21 +541,9 @@ public abstract class DependencyResolver {
     ImmutableSet.Builder<AspectDescriptor> result = ImmutableSet.builder();
 
     for (Aspect aspectCandidate : aspectCandidates) {
-      AdvertisedProviderSet advertisedProviders = ruleClass.getAdvertisedProviders();
-      boolean requireAspect = advertisedProviders.canHaveAnyProvider();
-
-      if (!requireAspect) {
-        ImmutableList<ImmutableSet<Class<?>>> providersList =
-            aspectCandidate.getDefinition().getRequiredProviders();
-        for (ImmutableSet<Class<?>> providers : providersList) {
-          if (Sets.difference(providers, advertisedProviders.getNativeProviders()).isEmpty()) {
-            requireAspect = true;
-            break;
-          }
-        }
-      }
-
-      if (requireAspect) {
+      if (aspectCandidate.getDefinition()
+          .getRequiredProviders()
+          .isSatisfiedBy(ruleClass.getAdvertisedProviders())) {
         result.add(
             new AspectDescriptor(
                 aspectCandidate.getAspectClass(),
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java b/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java
index 78a963ea8f..59c25830bd 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java
@@ -155,8 +155,9 @@ public final class AdvertisedProviderSet {
           ImmutableSet.copyOf(nativeProviders), ImmutableSet.copyOf(skylarkProviders));
     }
 
-    public void addSkylark(String providerName) {
+    public Builder addSkylark(String providerName) {
       skylarkProviders.add(providerName);
+      return this;
     }
   }
 }
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 18cd2ebba6..92f6d0cb39 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
@@ -56,8 +56,7 @@ import javax.annotation.Nullable;
 public final class AspectDefinition {
 
   private final AspectClass aspectClass;
-  private final ImmutableList<ImmutableSet<Class<?>>> requiredProviderSets;
-  private final ImmutableList<ImmutableSet<String>> requiredProviderNameSets;
+  private final RequiredProviders requiredProviders;
   private final ImmutableMap<String, Attribute> attributes;
   private final PropagationFunction attributeAspects;
   @Nullable private final ConfigurationFragmentPolicy configurationFragmentPolicy;
@@ -68,23 +67,16 @@ public final class AspectDefinition {
 
   private AspectDefinition(
       AspectClass aspectClass,
-      ImmutableList<ImmutableSet<Class<?>>> requiredProviderSets,
+      RequiredProviders requiredProviders,
       ImmutableMap<String, Attribute> attributes,
       PropagationFunction attributeAspects,
       @Nullable ConfigurationFragmentPolicy configurationFragmentPolicy) {
     this.aspectClass = aspectClass;
-    this.requiredProviderSets = requiredProviderSets;
+    this.requiredProviders = requiredProviders;
 
     this.attributes = attributes;
     this.attributeAspects = attributeAspects;
     this.configurationFragmentPolicy = configurationFragmentPolicy;
-
-    ImmutableList.Builder<ImmutableSet<String>> requiredProviderNameSetsBuilder =
-        new ImmutableList.Builder<>();
-    for (ImmutableSet<Class<?>> requiredProviderSet : requiredProviderSets) {
-      requiredProviderNameSetsBuilder.add(toStringSet(requiredProviderSet));
-    }
-    this.requiredProviderNameSets = requiredProviderNameSetsBuilder.build();
   }
 
   public String getName() {
@@ -105,34 +97,14 @@ public final class AspectDefinition {
   }
 
   /**
-   * Returns the list of {@link com.google.devtools.build.lib.analysis.TransitiveInfoProvider}
-   * sets. All required providers from at least one set must be present on a configured target so
-   * that this aspect can be applied to it.
-   *
-   * <p>We cannot refer to that class here due to our dependency structure, so this returns a set
-   * of unconstrained class objects.
-   *
-   * <p>If a configured target does not have a required provider, the aspect is silently not created
-   * for it.
-   */
-  public ImmutableList<ImmutableSet<Class<?>>> getRequiredProviders() {
-    return requiredProviderSets;
-  }
-
-  /**
-   * Returns the list of class name sets of
-   * {@link com.google.devtools.build.lib.analysis.TransitiveInfoProvider}. All required providers
-   * from at least one set must be present on a configured target so that this aspect can be applied
-   * to it.
-   *
-   * <p>This set is a mirror of the set returned by {@link #getRequiredProviders}, but contains the
-   * names of the classes rather than the class objects themselves.
+   * Returns {@link RequiredProviders} that a configured target must have so that
+   * this aspect can be applied to it.
    *
-   * <p>If a configured target does not have a required provider, the aspect is silently not created
-   * for it.
+   * <p>If a configured target does not satisfy required providers, the aspect is
+   * silently not created for it.
    */
-  public ImmutableList<ImmutableSet<String>> getRequiredProviderNames() {
-    return requiredProviderNameSets;
+  public RequiredProviders getRequiredProviders() {
+    return requiredProviders;
   }
 
   /**
@@ -176,31 +148,15 @@ public final class AspectDefinition {
     for (Aspect candidateClass : attribute.getAspects(from)) {
       // Check if target satisfies condition for this aspect (has to provide all required
       // TransitiveInfoProviders)
-      if (!advertisedProviders.canHaveAnyProvider()) {
-        ImmutableList<ImmutableSet<Class<?>>> providerNamesList =
-            candidateClass.getDefinition().getRequiredProviders();
-
-        for (ImmutableSet<Class<?>> providerNames : providerNamesList) {
-          if (advertisedProviders.getNativeProviders().containsAll(providerNames)) {
-            addAllAttributesOfAspect(from, result, candidateClass, dependencyFilter);
-            break;
-          }
-        }
-      } else {
+      RequiredProviders requiredProviders =
+          candidateClass.getDefinition().getRequiredProviders();
+      if (requiredProviders.isSatisfiedBy(advertisedProviders)) {
         addAllAttributesOfAspect(from, result, candidateClass, dependencyFilter);
       }
     }
     return ImmutableMultimap.copyOf(result);
   }
 
-  private static ImmutableSet<String> toStringSet(ImmutableSet<Class<?>> classes) {
-    ImmutableSet.Builder<String> classStrings = new ImmutableSet.Builder<>();
-    for (Class<?> clazz : classes) {
-      classStrings.add(clazz.getName());
-    }
-    return classStrings.build();
-  }
-
   @Nullable
   private static Label maybeGetRepositoryRelativeLabel(Rule from, @Nullable Label label) {
     return label == null ? null : from.getLabel().resolveRepositoryRelative(label);
@@ -246,7 +202,7 @@ public final class AspectDefinition {
   public static final class Builder {
     private final AspectClass aspectClass;
     private final Map<String, Attribute> attributes = new LinkedHashMap<>();
-    private ImmutableList<ImmutableSet<Class<?>>> requiredProviderSets = ImmutableList.of();
+    private RequiredProviders.Builder requiredProviders = RequiredProviders.acceptAnyBuilder();
     private final Multimap<String, AspectClass> attributeAspects = LinkedHashMultimap.create();
     private ImmutableCollection<AspectClass> allAttributesAspects = null;
     private final ConfigurationFragmentPolicy.Builder configurationFragmentPolicy =
@@ -261,12 +217,9 @@ public final class AspectDefinition {
      * from at least one set of required providers.
      */
     public Builder requireProviderSets(Iterable<? extends Set<Class<?>>> providerSets) {
-      ImmutableList.Builder<ImmutableSet<Class<?>>> requiredProviderSetsBuilder =
-          ImmutableList.builder();
-      for (Iterable<Class<?>> providerSet : providerSets) {
-        requiredProviderSetsBuilder.add(ImmutableSet.copyOf(providerSet));
+      for (Set<Class<?>> providerSet : providerSets) {
+        requiredProviders.addNativeSet(ImmutableSet.copyOf(providerSet));
       }
-      requiredProviderSets = requiredProviderSetsBuilder.build();
       return this;
     }
 
@@ -274,8 +227,8 @@ public final class AspectDefinition {
      * Asserts that this aspect can only be evaluated for rules that supply all of the specified
      * providers.
      */
-    public Builder requireProviders(Class<?>... requiredProviders) {
-      requireProviderSets(ImmutableList.of(ImmutableSet.copyOf(requiredProviders)));
+    public Builder requireProviders(Class<?>... providers) {
+      requiredProviders.addNativeSet(ImmutableSet.copyOf(providers));
       return this;
     }
 
@@ -445,15 +398,7 @@ public final class AspectDefinition {
      * <p>The builder object is reusable afterwards.
      */
     public AspectDefinition build() {
-      // If there is no required provider set, we still need to at least provide one empty set of
-      // providers. We consider this case specially because aspects with no required providers
-      // should match all rules, and having an empty set faciliates the matching logic.
-      ImmutableList<ImmutableSet<Class<?>>> requiredProviders =
-          requiredProviderSets.isEmpty()
-          ? ImmutableList.of(ImmutableSet.<Class<?>>of())
-          : requiredProviderSets;
-
-      return new AspectDefinition(aspectClass, ImmutableList.copyOf(requiredProviders),
+      return new AspectDefinition(aspectClass, requiredProviders.build(),
           ImmutableMap.copyOf(attributes),
           allAttributesAspects != null
               ? new AllAttributesPropagationFunction(allAttributesAspects)
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java b/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java
new file mode 100644
index 0000000000..371006b6b3
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java
@@ -0,0 +1,234 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.packages;
+
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.util.Preconditions;
+
+/**
+ * Represents a constraint on a set of providers required by a dependency (of a rule
+ * or an aspect).
+ *
+ * Currently we support three kinds of constraints:
+ * <ul>
+ *   <li>accept any dependency.</li>
+ *   <li>accept no dependency (used for aspects-on-aspects to indicate
+ *   that an aspect never wants to see any other aspect applied to a target.</li>
+ *   <li>accept a dependency that provides all providers from one of several sets of providers.
+ *       It just so happens that in all current usages these sets are either all
+ *       native providers or all Skylark providers, so this is the only use case this
+ *       class currently supports.
+ *   </li>
+ * </ul>
+ */
+@Immutable
+public final class RequiredProviders {
+  /** A constraint: either ANY, NONE, or RESTRICTED */
+  private final Constraint constraint;
+  /**
+   * Sets of native providers.
+   * If non-empty, {@link #constraint} is {@link Constraint#RESTRICTED}
+   */
+  private final ImmutableList<ImmutableSet<Class<?>>> nativeProviders;
+  /**
+   * Sets of native providers.
+   * If non-empty, {@link #constraint} is {@link Constraint#RESTRICTED}
+   */
+  private final ImmutableList<ImmutableSet<SkylarkProviderIdentifier>> skylarkProviders;
+
+  /**
+   * Represents one of the constraints as desctibed in {@link RequiredProviders}
+   */
+  private enum Constraint {
+    /** Accept any dependency */
+    ANY {
+      @Override
+      public boolean satisfies(AdvertisedProviderSet advertisedProviderSet,
+          RequiredProviders requiredProviders) {
+        return true;
+      }
+
+      @Override
+      public boolean satisfies(Predicate<Class<?>> hasNativeProvider,
+          Predicate<SkylarkProviderIdentifier> hasSkylarkProvider,
+          RequiredProviders requiredProviders) {
+        return true;
+      }
+    },
+    /** Accept no dependency */
+    NONE {
+      @Override
+      public boolean satisfies(AdvertisedProviderSet advertisedProviderSet,
+          RequiredProviders requiredProviders) {
+        return false;
+      }
+
+      @Override
+      public boolean satisfies(Predicate<Class<?>> hasNativeProvider,
+          Predicate<SkylarkProviderIdentifier> hasSkylarkProvider,
+          RequiredProviders requiredProviders) {
+        return false;
+      }
+    },
+    /** Accept a dependency that has all providers from one of the sets. */
+    RESTRICTED {
+      @Override
+      public boolean satisfies(Predicate<Class<?>> hasNativeProvider,
+          Predicate<SkylarkProviderIdentifier> hasSkylarkProvider,
+          RequiredProviders requiredProviders) {
+        for (ImmutableSet<Class<?>> nativeProviderSet : requiredProviders.nativeProviders) {
+          if (Iterables.all(nativeProviderSet, hasNativeProvider)) {
+            return true;
+          }
+        }
+
+        for (ImmutableSet<SkylarkProviderIdentifier> skylarkProviderSet
+            : requiredProviders.skylarkProviders) {
+          if (Iterables.all(skylarkProviderSet, hasSkylarkProvider)) {
+            return true;
+          }
+        }
+        return false;
+      }
+    };
+
+    /** Checks if {@code advertisedProviderSet} satisfies these {@code RequiredProviders} */
+    public boolean satisfies(final AdvertisedProviderSet advertisedProviderSet,
+        RequiredProviders requiredProviders) {
+      if (advertisedProviderSet.canHaveAnyProvider()) {
+        return true;
+      }
+      return satisfies(
+          new Predicate<Class<?>>() {
+            @Override
+            public boolean apply(Class<?> aClass) {
+              return advertisedProviderSet.getNativeProviders().contains(aClass);
+            }
+          },
+          new Predicate<SkylarkProviderIdentifier>() {
+            @Override
+            public boolean apply(SkylarkProviderIdentifier skylarkProviderIdentifier) {
+              if (!skylarkProviderIdentifier.isLegacy()) {
+                return false;
+              }
+              return advertisedProviderSet.getSkylarkProviders()
+                  .contains(skylarkProviderIdentifier.getLegacyId());
+            }
+          },
+          requiredProviders
+      );
+    }
+
+    /**
+     * Checks if a set of providers encoded by predicates {@code hasNativeProviders}
+     * and {@code hasSkylarkProvider} satisfies these {@code RequiredProviders}
+     */
+    abstract boolean satisfies(Predicate<Class<?>> hasNativeProvider,
+        Predicate<SkylarkProviderIdentifier> hasSkylarkProvider,
+        RequiredProviders requiredProviders);
+  }
+
+  /** Checks if {@code advertisedProviderSet} satisfies this {@code RequiredProviders} instance. */
+  public boolean isSatisfiedBy(AdvertisedProviderSet advertisedProviderSet) {
+    return constraint.satisfies(advertisedProviderSet, this);
+  }
+
+  /**
+   * Checks if a set of providers encoded by predicates {@code hasNativeProviders}
+   * and {@code hasSkylarkProvider} satisfies this {@code RequiredProviders} instance.
+   */
+  public boolean isSatisfiedBy(
+      Predicate<Class<?>> hasNativeProvider,
+      Predicate<SkylarkProviderIdentifier> hasSkylarkProvider) {
+    return constraint.satisfies(hasNativeProvider, hasSkylarkProvider, this);
+  }
+
+
+  private RequiredProviders(
+      Constraint constraint,
+      ImmutableList<ImmutableSet<Class<?>>> nativeProviders,
+      ImmutableList<ImmutableSet<SkylarkProviderIdentifier>> skylarkProviders) {
+    this.constraint = constraint;
+
+    Preconditions.checkState(constraint.equals(Constraint.RESTRICTED)
+        || (nativeProviders.isEmpty() && skylarkProviders.isEmpty())
+    );
+
+    this.nativeProviders = nativeProviders;
+    this.skylarkProviders = skylarkProviders;
+  }
+
+  /**
+   * A builder for {@link RequiredProviders} that accepts any dependency
+   * unless restriction provider sets are added.
+   */
+  public static Builder acceptAnyBuilder() {
+    return new Builder(false);
+  }
+
+  /**
+   * A builder for {@link RequiredProviders} that accepts no dependency
+   * unless restriction provider sets are added.
+   */
+  public static Builder acceptNoneBuilder() {
+    return new Builder(true);
+  }
+
+  /** A builder for {@link RequiredProviders} */
+  public static class Builder {
+    private final ImmutableList.Builder<ImmutableSet<Class<?>>> nativeProviders;
+    private final ImmutableList.Builder<ImmutableSet<SkylarkProviderIdentifier>> skylarkProviders;
+    private Constraint constraint;
+
+    private Builder(boolean acceptNone) {
+      constraint = acceptNone ? Constraint.NONE : Constraint.ANY;
+      nativeProviders = ImmutableList.builder();
+      skylarkProviders = ImmutableList.builder();
+    }
+
+    /**
+     * Add an alternative set of Skylark providers.
+     *
+     * If all of these providers are present in the dependency, the dependency satisfies
+     * {@link RequiredProviders}.
+     */
+    public Builder addSkylarkSet(ImmutableSet<SkylarkProviderIdentifier> skylarkProviderSet) {
+      constraint = Constraint.RESTRICTED;
+      Preconditions.checkState(!skylarkProviderSet.isEmpty());
+      this.skylarkProviders.add(skylarkProviderSet);
+      return this;
+    }
+
+    /**
+     * Add an alternative set of native providers.
+     *
+     * If all of these providers are present in the dependency, the dependency satisfies
+     * {@link RequiredProviders}.
+     */
+    public Builder addNativeSet(ImmutableSet<Class<?>> nativeProviderSet) {
+      constraint = Constraint.RESTRICTED;
+      Preconditions.checkState(!nativeProviderSet.isEmpty());
+      this.nativeProviders.add(nativeProviderSet);
+      return this;
+    }
+
+    public RequiredProviders build() {
+      return new RequiredProviders(constraint, nativeProviders.build(), skylarkProviders.build());
+    }
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index 134cea24a8..e481dbd1d9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -16,11 +16,11 @@ package com.google.devtools.build.lib.skyframe;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Joiner;
+import com.google.common.base.Predicate;
 import com.google.common.base.Verify;
 import com.google.common.base.VerifyException;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.LinkedListMultimap;
@@ -54,7 +54,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.Aspect;
-import com.google.devtools.build.lib.packages.AspectDefinition;
 import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.BuildType;
@@ -64,6 +63,7 @@ import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.RawAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
+import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.skyframe.AspectFunction.AspectCreationException;
@@ -916,24 +916,21 @@ final class ConfiguredTargetFunction implements SkyFunction {
     return key;
   }
 
-  private static boolean aspectMatchesConfiguredTarget(ConfiguredTarget dep, Aspect aspect) {
-    AspectDefinition aspectDefinition = aspect.getDefinition();
-    ImmutableList<ImmutableSet<Class<?>>> providersList = aspectDefinition.getRequiredProviders();
-
-    for (ImmutableSet<Class<?>> providers : providersList) {
-      boolean matched = true;
-      for (Class<?> provider : providers) {
-        if (dep.getProvider(provider.asSubclass(TransitiveInfoProvider.class)) == null) {
-          matched = false;
-          break;
+  private static boolean aspectMatchesConfiguredTarget(final ConfiguredTarget dep, Aspect aspect) {
+    return aspect.getDefinition().getRequiredProviders().isSatisfiedBy(
+        new Predicate<Class<?>>() {
+          @Override
+          public boolean apply(Class<?> provider) {
+            return dep.getProvider(provider.asSubclass(TransitiveInfoProvider.class)) != null;
+          }
+        },
+        new Predicate<SkylarkProviderIdentifier>() {
+          @Override
+          public boolean apply(SkylarkProviderIdentifier skylarkProviderIdentifier) {
+            return dep.get(skylarkProviderIdentifier) != null;
+          }
         }
-      }
-
-      if (matched) {
-        return true;
-      }
-    }
-    return false;
+    );
   }
 
   /**