When creating RuleContext, explicitly pass the set of attributes an attached Aspect provides.

--
MOS_MIGRATED_REVID=106882046

2 files changed, 19 insertions, 18 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index 067f370f2c..91c4299c79 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -55,6 +55,7 @@ import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.ArrayList;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import javax.annotation.Nullable;
@@ -290,10 +291,11 @@ public final class ConfiguredTargetFactory {
       RuleConfiguredTarget associatedTarget,
       ConfiguredAspectFactory aspectFactory,
       AspectParameters aspectParameters,
+      Map<String, Attribute> aspectAttributes,
       ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
       Set<ConfigMatchingProvider> configConditions,
       BuildConfiguration hostConfiguration)
-      throws InterruptedException {
+          throws InterruptedException {
     RuleContext.Builder builder = new RuleContext.Builder(env,
         associatedTarget.getTarget(),
         associatedTarget.getConfiguration(),
@@ -303,6 +305,7 @@ public final class ConfiguredTargetFactory {
         .setVisibility(convertVisibility(
             prerequisiteMap, env.getEventHandler(), associatedTarget.getTarget(), null))
         .setPrerequisites(prerequisiteMap)
+        .setAspectAttributes(aspectAttributes)
         .setConfigConditions(configConditions)
         .build();
     if (ruleContext.hasErrors()) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 12916b3253..e1ccc44a9d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -136,7 +136,7 @@ public final class RuleContext extends TargetContext
   private final Set<ConfigMatchingProvider> configConditions;
   private final AttributeMap attributes;
   private final ImmutableSet<String> features;
-  private final Map<String, Attribute> attributeMap;
+  private final Map<String, Attribute> aspectAttributes;
   private final BuildConfiguration hostConfiguration;
   private final ConfigurationFragmentPolicy configurationFragmentPolicy;
   private final ErrorReporter reporter;
@@ -148,7 +148,7 @@ public final class RuleContext extends TargetContext
 
   private RuleContext(Builder builder, ListMultimap<String, ConfiguredTarget> targetMap,
       ListMultimap<String, ConfiguredFilesetEntry> filesetEntryMap,
-      Set<ConfigMatchingProvider> configConditions, Map<String, Attribute> attributeMap) {
+      Set<ConfigMatchingProvider> configConditions, Map<String, Attribute> aspectAttributes) {
     super(builder.env, builder.rule, builder.configuration, builder.prerequisiteMap.get(null),
         builder.visibility);
     this.rule = builder.rule;
@@ -159,7 +159,7 @@ public final class RuleContext extends TargetContext
     this.attributes =
         ConfiguredAttributeMapper.of(builder.rule, configConditions);
     this.features = getEnabledFeatures();
-    this.attributeMap = attributeMap;
+    this.aspectAttributes = aspectAttributes;
     this.hostConfiguration = builder.hostConfiguration;
     reporter = builder.reporter;
   }
@@ -561,16 +561,11 @@ public final class RuleContext extends TargetContext
   }
 
   private Attribute getAttribute(String attributeName) {
-    // TODO(bazel-team): We should check original rule for such attribute first, because aspect
-    // can't contain empty attribute. Consider changing type of prerequisiteMap from
-    // ListMultimap<Attribute, ConfiguredTarget> to Map<Attribute, List<ConfiguredTarget>>. This can
-    // also simplify logic in DependencyResolver#resolveExplicitAttributes.
     Attribute result = getRule().getAttributeDefinition(attributeName);
     if (result != null) {
       return result;
     }
-    // Also this attribute can come from aspects, so we also have to check attributeMap.
-    return attributeMap.get(attributeName);
+    return aspectAttributes.get(attributeName);
   }
 
   /**
@@ -1239,6 +1234,7 @@ public final class RuleContext extends TargetContext
     private ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap;
     private Set<ConfigMatchingProvider> configConditions;
     private NestedSet<PackageSpecification> visibility;
+    private Map<String, Attribute> aspectAttributes;
 
     Builder(AnalysisEnvironment env, Rule rule, BuildConfiguration configuration,
         BuildConfiguration hostConfiguration,
@@ -1259,14 +1255,8 @@ public final class RuleContext extends TargetContext
       ListMultimap<String, ConfiguredTarget> targetMap = createTargetMap();
       ListMultimap<String, ConfiguredFilesetEntry> filesetEntryMap =
           createFilesetEntryMap(rule, configConditions);
-      Map<String, Attribute> attributeMap = new HashMap<>();
-      for (Attribute attribute : prerequisiteMap.keySet()) {
-        if (attribute == null) {
-          continue;
-        }
-        attributeMap.put(attribute.getName(), attribute);
-      }
-      return new RuleContext(this, targetMap, filesetEntryMap, configConditions, attributeMap);
+      return new RuleContext(this, targetMap, filesetEntryMap, configConditions,
+          aspectAttributes != null ? aspectAttributes : ImmutableMap.<String, Attribute>of());
     }
 
     Builder setVisibility(NestedSet<PackageSpecification> visibility) {
@@ -1284,6 +1274,14 @@ public final class RuleContext extends TargetContext
     }
 
     /**
+     * Adds attributes which are defined by an Aspect (and not by RuleClass).
+     */
+    Builder setAspectAttributes(Map<String, Attribute> aspectAttributes) {
+      this.aspectAttributes = aspectAttributes;
+      return this;
+    }
+
+    /**
      * Sets the configuration conditions needed to determine which paths to follow for this
      * rule's configurable attributes.
      */