Properly filter out Aspect attributes when `blaze query` requests --noimplicit_deps and/or --nohost_deps.

RELNOTES: --noimplicit_deps and --nohost_deps work correctly for Aspect attributes.

--
MOS_MIGRATED_REVID=112724917

12 files changed, 53 insertions, 29 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
index c2da5dc7b5..34f3bfff3f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
@@ -22,7 +22,7 @@ import java.util.Objects;
  * created with help of aspect factory instances and parameters are used to configure them, so we
  * have to keep them together.
  */
-public final class Aspect {
+public final class Aspect implements DependencyFilter.AttributeInfoProvider {
   // TODO(bazel-team): class objects are not really hashable or comparable for equality other than
   // by reference. We should identify the aspect here in a way that does not rely on comparison
   // by reference so that keys can be serialized and deserialized properly.
@@ -80,4 +80,10 @@ public final class Aspect {
   public AspectDefinition getDefinition() {
     return aspectClass.getDefinition(parameters);
   }
+
+  @Override
+  public boolean isAttributeValueExplicitlySpecified(Attribute attribute) {
+    // All aspect attributes are implicit.
+    return false;
+  }
 }
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 5c905ba4b2..875d9dcb33 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
@@ -24,7 +24,6 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
 import com.google.devtools.build.lib.packages.NativeAspectClass.NativeAspectFactory;
-import com.google.devtools.build.lib.util.BinaryPredicate;
 import com.google.devtools.build.lib.util.Preconditions;
 
 import java.util.Collection;
@@ -141,21 +140,23 @@ public final class AspectDefinition {
    * Returns the attribute -> set of labels that are provided by aspects of attribute.
    */
   public static ImmutableMultimap<Attribute, Label> visitAspectsIfRequired(
-      Target from, Attribute attribute, Target to) {
+      Target from, Attribute attribute, Target to,
+      DependencyFilter dependencyFilter) {
     // Aspect can be declared only for Rules.
     if (!(from instanceof Rule) || !(to instanceof Rule)) {
       return ImmutableMultimap.of();
     }
     RuleClass ruleClass = ((Rule) to).getRuleClassObject();
     ImmutableSet<Class<?>> providers = ruleClass.getAdvertisedProviders();
-    return visitAspectsIfRequired((Rule) from, attribute, toStringSet(providers));
+    return visitAspectsIfRequired((Rule) from, attribute, toStringSet(providers), dependencyFilter);
   }
 
   /**
    * Returns the attribute -&gt; set of labels that are provided by aspects of attribute.
    */
   public static ImmutableMultimap<Attribute, Label> visitAspectsIfRequired(
-      Rule from, Attribute attribute, Set<String> advertisedProviders) {
+      Rule from, Attribute attribute, Set<String> advertisedProviders,
+      DependencyFilter dependencyFilter) {
     if (advertisedProviders.isEmpty()) {
       return ImmutableMultimap.of();
     }
@@ -168,8 +169,7 @@ public final class AspectDefinition {
           candidateClass.getDefinition().getRequiredProviderNames())) {
         continue;
       }
-      addAllAttributesOfAspect(
-          from, result, candidateClass.getDefinition(), DependencyFilter.ALL_DEPS);
+      addAllAttributesOfAspect(from, result, candidateClass, dependencyFilter);
     }
     return ImmutableMultimap.copyOf(result);
   }
@@ -188,11 +188,11 @@ public final class AspectDefinition {
   public static void addAllAttributesOfAspect(
       Rule from,
       Multimap<Attribute, Label> labelBuilder,
-      AspectDefinition aspectDefinition,
-      BinaryPredicate<? super Rule, Attribute> predicate) {
-    ImmutableMap<String, Attribute> attributes = aspectDefinition.getAttributes();
+      Aspect aspect,
+      DependencyFilter dependencyFilter) {
+    ImmutableMap<String, Attribute> attributes = aspect.getDefinition().getAttributes();
     for (Attribute aspectAttribute : attributes.values()) {
-      if (!predicate.apply(from, aspectAttribute)) {
+      if (!dependencyFilter.apply(aspect, aspectAttribute)) {
         continue;
       }
       if (aspectAttribute.getType() == BuildType.LABEL) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
index d4d4f17b56..e2cf7bf700 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Rule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
@@ -664,8 +664,7 @@ public final class Rule implements Target, DependencyFilter.AttributeInfoProvide
     LinkedHashMultimap<Attribute, Label> labels = LinkedHashMultimap.create();
     for (Attribute attribute : this.getAttributes()) {
       for (Aspect candidateClass : attribute.getAspects(this)) {
-        AspectDefinition.addAllAttributesOfAspect(
-            Rule.this, labels, candidateClass.getDefinition(), predicate);
+        AspectDefinition.addAllAttributesOfAspect(Rule.this, labels, candidateClass, predicate);
       }
     }
     return labels.values();
diff --git a/src/main/java/com/google/devtools/build/lib/query2/LabelVisitor.java b/src/main/java/com/google/devtools/build/lib/query2/LabelVisitor.java
index d2015c988e..f0dd92fbf3 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/LabelVisitor.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/LabelVisitor.java
@@ -326,10 +326,6 @@ final class LabelVisitor {
       // Don't perform the targetProvider lookup if at the maximum depth already.
       if (depth >= maxDepth) {
         return;
-      } else if (attr != null && from instanceof Rule) {
-        if (!edgeFilter.apply((Rule) from, attr)) {
-          return;
-        }
       }
 
       // Avoid thread-related overhead when not crossing packages.
@@ -366,11 +362,15 @@ final class LabelVisitor {
     private void visitTargetVisibility(Target target, int depth, int count) {
       Attribute attribute = null;
       if (target instanceof Rule) {
-        RuleClass ruleClass = ((Rule) target).getRuleClassObject();
+        Rule rule = (Rule) target;
+        RuleClass ruleClass = rule.getRuleClassObject();
         if (!ruleClass.hasAttr("visibility", BuildType.NODEP_LABEL_LIST)) {
           return;
         }
         attribute = ruleClass.getAttributeByName("visibility");
+        if (!edgeFilter.apply(rule, attribute)) {
+          return;
+        }
       }
 
       for (Label label : target.getVisibility().getDependencyLabels()) {
@@ -391,6 +391,9 @@ final class LabelVisitor {
       AggregatingAttributeMapper.of(rule).visitLabels(new AttributeMap.AcceptsLabelAttribute() {
         @Override
         public void acceptLabelAttribute(Label label, Attribute attribute) {
+          if (!edgeFilter.apply(rule, attribute)) {
+            return;
+          }
           enqueueTarget(rule, attribute, label, depth, count);
         }
       });
@@ -427,7 +430,7 @@ final class LabelVisitor {
     private void visitAspectsIfRequired(
         Target from, Attribute attribute, final Target to, int depth, int count) {
       ImmutableMultimap<Attribute, Label> labelsFromAspects =
-          AspectDefinition.visitAspectsIfRequired(from, attribute, to);
+          AspectDefinition.visitAspectsIfRequired(from, attribute, to, edgeFilter);
       // Create an edge from target to the attribute value.
       for (Entry<Attribute, Label> entry : labelsFromAspects.entries()) {
         enqueueTarget(from, entry.getKey(), entry.getValue(), depth, count);
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/AspectResolver.java b/src/main/java/com/google/devtools/build/lib/query2/output/AspectResolver.java
index 52f9875b1a..712010006c 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/AspectResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/AspectResolver.java
@@ -17,6 +17,7 @@ import com.google.common.collect.ImmutableMultimap;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.DependencyFilter;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.pkgcache.PackageProvider;
@@ -86,7 +87,8 @@ public interface AspectResolver {
    * Compute additional dependencies of target from aspects. This method may load the direct deps
    * of target to determine their types. Returns map of attributes and corresponding label values.
    */
-  ImmutableMultimap<Attribute, Label> computeAspectDependencies(Target target)
+  ImmutableMultimap<Attribute, Label> computeAspectDependencies(Target target,
+      DependencyFilter dependencyFilter)
       throws InterruptedException;
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/ConservativeAspectResolver.java b/src/main/java/com/google/devtools/build/lib/query2/output/ConservativeAspectResolver.java
index 47ed795747..1b28bcdbc8 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/ConservativeAspectResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/ConservativeAspectResolver.java
@@ -34,7 +34,8 @@ import java.util.Set;
  */
 public class ConservativeAspectResolver implements AspectResolver {
   @Override
-  public ImmutableMultimap<Attribute, Label> computeAspectDependencies(Target target)
+  public ImmutableMultimap<Attribute, Label> computeAspectDependencies(Target target,
+      DependencyFilter dependencyFilter)
       throws InterruptedException {
     if (!(target instanceof Rule)) {
       return ImmutableMultimap.of();
@@ -45,7 +46,7 @@ public class ConservativeAspectResolver implements AspectResolver {
     for (Attribute attribute : rule.getAttributes()) {
       for (Aspect aspect : attribute.getAspects(rule)) {
         AspectDefinition.addAllAttributesOfAspect(
-            rule, result, aspect.getDefinition(), DependencyFilter.ALL_DEPS);
+            rule, result, aspect, dependencyFilter);
       }
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/NullAspectResolver.java b/src/main/java/com/google/devtools/build/lib/query2/output/NullAspectResolver.java
index c52e4e6c52..1b1f9a3b99 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/NullAspectResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/NullAspectResolver.java
@@ -17,6 +17,7 @@ import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.DependencyFilter;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.Target;
 
@@ -29,7 +30,8 @@ import java.util.Set;
  */
 public class NullAspectResolver implements AspectResolver {
   @Override
-  public ImmutableMultimap<Attribute, Label> computeAspectDependencies(Target target)
+  public ImmutableMultimap<Attribute, Label> computeAspectDependencies(Target target,
+      DependencyFilter dependencyFilter)
       throws InterruptedException {
     return ImmutableMultimap.of();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/PreciseAspectResolver.java b/src/main/java/com/google/devtools/build/lib/query2/output/PreciseAspectResolver.java
index 2abe834737..4035d2201c 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/PreciseAspectResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/PreciseAspectResolver.java
@@ -54,7 +54,8 @@ public class PreciseAspectResolver implements AspectResolver {
   }
 
   @Override
-  public ImmutableMultimap<Attribute, Label> computeAspectDependencies(Target target)
+  public ImmutableMultimap<Attribute, Label> computeAspectDependencies(Target target,
+      DependencyFilter dependencyFilter)
       throws InterruptedException {
     Multimap<Attribute, Label> result = LinkedListMultimap.create();
     if (target instanceof Rule) {
@@ -64,7 +65,12 @@ public class PreciseAspectResolver implements AspectResolver {
         Target toTarget;
         try {
           toTarget = packageProvider.getTarget(eventHandler, entry.getValue());
-          result.putAll(AspectDefinition.visitAspectsIfRequired(target, entry.getKey(), toTarget));
+          result.putAll(
+              AspectDefinition.visitAspectsIfRequired(
+                  target,
+                  entry.getKey(),
+                  toTarget,
+                  dependencyFilter));
         } catch (NoSuchThingException e) {
           // Do nothing. One of target direct deps has an error. The dependency on the BUILD file
           // (or one of the files included in it) will be reported in the query result of :BUILD.
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java
index 84554af961..e8fa723003 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java
@@ -177,7 +177,7 @@ public class ProtoOutputFormatter extends AbstractUnorderedFormatter {
       }
 
       ImmutableMultimap<Attribute, Label> aspectsDependencies =
-          aspectResolver.computeAspectDependencies(target);
+          aspectResolver.computeAspectDependencies(target, dependencyFilter);
       // Add information about additional attributes from aspects.
       for (Entry<Attribute, Collection<Label>> entry : aspectsDependencies.asMap().entrySet()) {
         Attribute attribute = entry.getKey();
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java
index a14facf398..6de67a259e 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java
@@ -155,7 +155,8 @@ class XmlOutputFormatter extends AbstractUnorderedFormatter {
         inputElem.setAttribute("name", label.toString());
         elem.appendChild(inputElem);
       }
-      for (Label label : aspectResolver.computeAspectDependencies(target).values()) {
+      for (Label label :
+          aspectResolver.computeAspectDependencies(target, dependencyFilter).values()) {
         Element inputElem = doc.createElement("rule-input");
         inputElem.setAttribute("name", label.toString());
         elem.appendChild(inputElem);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
index 9b0eb68ee5..4035bfaf6e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
@@ -30,6 +30,7 @@ import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.AspectDefinition;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy;
+import com.google.devtools.build.lib.packages.DependencyFilter;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.NoSuchThingException;
@@ -230,7 +231,8 @@ public class TransitiveTargetFunction
         return ImmutableList.of();
       }
       Target dependedTarget = pkgValue.getPackage().getTarget(toLabel.getName());
-      return AspectDefinition.visitAspectsIfRequired(fromRule, attr, dependedTarget).values();
+      return AspectDefinition.visitAspectsIfRequired(fromRule, attr, dependedTarget,
+          DependencyFilter.ALL_DEPS).values();
     } catch (NoSuchThingException e) {
       // Do nothing. This error was handled when we computed the corresponding
       // TransitiveTargetValue.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java
index 838f73d765..338d9c0e46 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java
@@ -19,6 +19,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.AspectDefinition;
 import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.DependencyFilter;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.NoSuchThingException;
@@ -96,7 +97,8 @@ public class TransitiveTraversalFunction
       // Retrieve the providers of the dep from the TransitiveTraversalValue, so we can avoid
       // issuing a dep on its defining Package.
       Set<String> providers = traversalVal.getProviders();
-      return AspectDefinition.visitAspectsIfRequired(fromRule, attr, providers).values();
+      return AspectDefinition.visitAspectsIfRequired(fromRule, attr, providers,
+          DependencyFilter.ALL_DEPS).values();
     } catch (NoSuchThingException e) {
       // Do nothing. This error was handled when we computed the corresponding
       // TransitiveTargetValue.