Enforce aspect configuration fragment specification when present.

If an aspect has specified its configuration fragment dependencies,
use these in place of the rule's.

Note that the dynamic configuration support for this is yet to come.

Also in this CL:
* RuleContext is constructed with a ruleClassNameForLogging, which allows
  error messages involving aspects to be clearer.

RELNOTES[NEW]: Skylark aspects can now specify configuration fragment
dependencies with fragments and host_fragments like rules can.

--
MOS_MIGRATED_REVID=112614357

3 files changed, 83 insertions, 30 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 862aa0f878..166879d62c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -960,8 +960,9 @@ public class BuildView {
           throws EvalException, InterruptedException {
     BuildConfiguration targetConfig = target.getConfiguration();
     return new RuleContext.Builder(
-        env, (Rule) target.getTarget(), targetConfig, configurations.getHostConfiguration(),
-        ruleClassProvider.getPrerequisiteValidator())
+        env, (Rule) target.getTarget(), null, targetConfig, configurations.getHostConfiguration(),
+        ruleClassProvider.getPrerequisiteValidator(),
+        ((Rule) target.getTarget()).getRuleClassObject().getConfigurationFragmentPolicy())
             .setVisibility(NestedSetBuilder.<PackageSpecification>create(
                 Order.STABLE_ORDER, PackageSpecification.EVERYTHING))
             .setPrerequisites(getPrerequisiteMapForTesting(eventHandler, target, configurations))
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 3565324489..71a1626100 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
@@ -221,8 +221,10 @@ public final class ConfiguredTargetFactory {
       ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
       Set<ConfigMatchingProvider> configConditions) throws InterruptedException {
     // Visibility computation and checking is done for every rule.
-    RuleContext ruleContext = new RuleContext.Builder(env, rule, configuration, hostConfiguration,
-        ruleClassProvider.getPrerequisiteValidator())
+    RuleContext ruleContext = new RuleContext.Builder(env, rule, null,
+        configuration, hostConfiguration,
+        ruleClassProvider.getPrerequisiteValidator(),
+        rule.getRuleClassObject().getConfigurationFragmentPolicy())
         .setVisibility(convertVisibility(prerequisiteMap, env.getEventHandler(), rule, null))
         .setPrerequisites(prerequisiteMap)
         .setConfigConditions(configConditions)
@@ -296,11 +298,19 @@ public final class ConfiguredTargetFactory {
       Set<ConfigMatchingProvider> configConditions,
       BuildConfiguration hostConfiguration)
       throws InterruptedException {
+    ConfigurationFragmentPolicy aspectPolicy =
+        aspect.getDefinition().getConfigurationFragmentPolicy();
+    ConfigurationFragmentPolicy rulePolicy =
+        ((Rule) associatedTarget.getTarget()).getRuleClassObject().getConfigurationFragmentPolicy();
     RuleContext.Builder builder = new RuleContext.Builder(env,
         associatedTarget.getTarget(),
+        aspect.getAspectClass().getName(),
         associatedTarget.getConfiguration(),
         hostConfiguration,
-        ruleClassProvider.getPrerequisiteValidator());
+        ruleClassProvider.getPrerequisiteValidator(),
+        // TODO(mstaib): When AspectDefinition can no longer have null ConfigurationFragmentPolicy,
+        // remove this conditional.
+        aspectPolicy != null ? aspectPolicy : rulePolicy);
     RuleContext ruleContext =
         builder
             .setVisibility(
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 6e06ef1f91..b54e9e0ba2 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,6 +136,7 @@ public final class RuleContext extends TargetContext
   private final Set<ConfigMatchingProvider> configConditions;
   private final AttributeMap attributes;
   private final ImmutableSet<String> features;
+  private final String ruleClassNameForLogging;
   private final ImmutableMap<String, Attribute> aspectAttributes;
   private final BuildConfiguration hostConfiguration;
   private final ConfigurationFragmentPolicy configurationFragmentPolicy;
@@ -153,6 +154,7 @@ public final class RuleContext extends TargetContext
       ListMultimap<String, ConfiguredFilesetEntry> filesetEntryMap,
       Set<ConfigMatchingProvider> configConditions,
       Class<? extends BuildConfiguration.Fragment> universalFragment,
+      String ruleClassNameForLogging,
       ImmutableMap<String, Attribute> aspectAttributes) {
     super(builder.env, builder.rule, builder.configuration, builder.prerequisiteMap.get(null),
         builder.visibility);
@@ -165,6 +167,7 @@ public final class RuleContext extends TargetContext
     this.attributes =
         ConfiguredAttributeMapper.of(builder.rule, configConditions);
     this.features = getEnabledFeatures();
+    this.ruleClassNameForLogging = ruleClassNameForLogging;
     this.aspectAttributes = aspectAttributes;
     this.hostConfiguration = builder.hostConfiguration;
     reporter = builder.reporter;
@@ -215,6 +218,13 @@ public final class RuleContext extends TargetContext
   }
 
   /**
+   * Returns a rule class name suitable for log messages, including an aspect name if applicable.
+   */
+  public String getRuleClassNameForLogging() {
+    return ruleClassNameForLogging;
+  }
+
+  /**
    * Returns the workspace name for the rule.
    */
   public String getWorkspaceName() {
@@ -295,7 +305,7 @@ public final class RuleContext extends TargetContext
     Preconditions.checkArgument(isLegalFragment(fragment, config),
         "%s has to declare '%s' as a required fragment "
         + "in %s configuration in order to access it.%s",
-        rule.getRuleClass(), name, FragmentCollection.getConfigurationName(config),
+        getRuleClassNameForLogging(), name, FragmentCollection.getConfigurationName(config),
         additionalErrorMessage);
     return getConfiguration(config).getFragment(fragment);
   }
@@ -688,8 +698,8 @@ public final class RuleContext extends TargetContext
     checkAttribute(attributeName, mode);
     List<? extends TransitiveInfoCollection> elements = targetMap.get(attributeName);
     if (elements.size() > 1) {
-      throw new IllegalStateException(rule.getRuleClass() + " attribute " + attributeName
-          + " produces more then one prerequisites");
+      throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName
+          + " produces more than one prerequisite");
     }
     return elements.isEmpty() ? null : elements.get(0);
   }
@@ -731,11 +741,11 @@ public final class RuleContext extends TargetContext
     Attribute ruleDefinition = getAttribute(attributeName);
 
     if (ruleDefinition == null) {
-      throw new IllegalStateException(getRule().getRuleClass() + " attribute " + attributeName
+      throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName
           + " is not defined");
     }
     if (!ruleDefinition.isExecutable()) {
-      throw new IllegalStateException(getRule().getRuleClass() + " attribute " + attributeName
+      throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName
           + " is not configured to be executable");
     }
 
@@ -887,36 +897,36 @@ public final class RuleContext extends TargetContext
   private void checkAttribute(String attributeName, Mode mode) {
     Attribute attributeDefinition = getAttribute(attributeName);
     if (attributeDefinition == null) {
-      throw new IllegalStateException(getRule().getLocation() + ": " + getRule().getRuleClass()
+      throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging()
         + " attribute " + attributeName + " is not defined");
     }
     if (!(attributeDefinition.getType() == BuildType.LABEL
         || attributeDefinition.getType() == BuildType.LABEL_LIST)) {
-      throw new IllegalStateException(rule.getRuleClass() + " attribute " + attributeName
+      throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName
         + " is not a label type attribute");
     }
     if (mode == Mode.HOST) {
       if (attributeDefinition.getConfigurationTransition() != ConfigurationTransition.HOST) {
         throw new IllegalStateException(getRule().getLocation() + ": "
-            + getRule().getRuleClass() + " attribute " + attributeName
+            + getRuleClassNameForLogging() + " attribute " + attributeName
             + " is not configured for the host configuration");
       }
     } else if (mode == Mode.TARGET) {
       if (attributeDefinition.getConfigurationTransition() != ConfigurationTransition.NONE) {
         throw new IllegalStateException(getRule().getLocation() + ": "
-            + getRule().getRuleClass() + " attribute " + attributeName
+            + getRuleClassNameForLogging() + " attribute " + attributeName
             + " is not configured for the target configuration");
       }
     } else if (mode == Mode.DATA) {
       if (attributeDefinition.getConfigurationTransition() != ConfigurationTransition.DATA) {
         throw new IllegalStateException(getRule().getLocation() + ": "
-            + getRule().getRuleClass() + " attribute " + attributeName
+            + getRuleClassNameForLogging() + " attribute " + attributeName
             + " is not configured for the data configuration");
       }
     } else if (mode == Mode.SPLIT) {
       if (!(attributeDefinition.getConfigurationTransition() instanceof SplitTransition)) {
         throw new IllegalStateException(getRule().getLocation() + ": "
-            + getRule().getRuleClass() + " attribute " + attributeName
+            + getRuleClassNameForLogging() + " attribute " + attributeName
             + " is not configured for a split transition");
       }
     }
@@ -929,12 +939,12 @@ public final class RuleContext extends TargetContext
   public Mode getAttributeMode(String attributeName) {
     Attribute attributeDefinition = getAttribute(attributeName);
     if (attributeDefinition == null) {
-      throw new IllegalStateException(getRule().getLocation() + ": " + getRule().getRuleClass()
+      throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging()
         + " attribute " + attributeName + " is not defined");
     }
     if (!(attributeDefinition.getType() == BuildType.LABEL
         || attributeDefinition.getType() == BuildType.LABEL_LIST)) {
-      throw new IllegalStateException(rule.getRuleClass() + " attribute " + attributeName
+      throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName
         + " is not a label type attribute");
     }
     if (attributeDefinition.getConfigurationTransition() == ConfigurationTransition.HOST) {
@@ -947,7 +957,7 @@ public final class RuleContext extends TargetContext
       return Mode.SPLIT;
     }
     throw new IllegalStateException(getRule().getLocation() + ": "
-        + getRule().getRuleClass() + " attribute " + attributeName + " is not configured");
+        + getRuleClassNameForLogging() + " attribute " + attributeName + " is not configured");
   }
 
   /**
@@ -1016,7 +1026,7 @@ public final class RuleContext extends TargetContext
   }
 
   public Artifact getSingleSource() {
-    return getSingleSource(getRule().getRuleClass() + " source file");
+    return getSingleSource(getRuleClassNameForLogging() + " source file");
   }
 
   /**
@@ -1246,22 +1256,29 @@ public final class RuleContext extends TargetContext
     private final BuildConfiguration configuration;
     private final BuildConfiguration hostConfiguration;
     private final PrerequisiteValidator prerequisiteValidator;
+    @Nullable private final String aspectName;
     private final ErrorReporter reporter;
     private ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap;
     private Set<ConfigMatchingProvider> configConditions;
     private NestedSet<PackageSpecification> visibility;
     private ImmutableMap<String, Attribute> aspectAttributes;
 
-    Builder(AnalysisEnvironment env, Rule rule, BuildConfiguration configuration,
+    Builder(
+        AnalysisEnvironment env,
+        Rule rule,
+        @Nullable String aspectName,
+        BuildConfiguration configuration,
         BuildConfiguration hostConfiguration,
-        PrerequisiteValidator prerequisiteValidator) {
+        PrerequisiteValidator prerequisiteValidator,
+        ConfigurationFragmentPolicy configurationFragmentPolicy) {
       this.env = Preconditions.checkNotNull(env);
       this.rule = Preconditions.checkNotNull(rule);
-      this.configurationFragmentPolicy = rule.getRuleClassObject().getConfigurationFragmentPolicy();
+      this.aspectName = aspectName;
+      this.configurationFragmentPolicy = Preconditions.checkNotNull(configurationFragmentPolicy);
       this.configuration = Preconditions.checkNotNull(configuration);
       this.hostConfiguration = Preconditions.checkNotNull(hostConfiguration);
       this.prerequisiteValidator = prerequisiteValidator;
-      reporter = new ErrorReporter(env, rule);
+      reporter = new ErrorReporter(env, rule, getRuleClassNameForLogging());
     }
 
     RuleContext build() {
@@ -1271,7 +1288,13 @@ public final class RuleContext extends TargetContext
       ListMultimap<String, ConfiguredTarget> targetMap = createTargetMap();
       ListMultimap<String, ConfiguredFilesetEntry> filesetEntryMap =
           createFilesetEntryMap(rule, configConditions);
-      return new RuleContext(this, targetMap, filesetEntryMap, configConditions, universalFragment,
+      return new RuleContext(
+          this,
+          targetMap,
+          filesetEntryMap,
+          configConditions,
+          universalFragment,
+          getRuleClassNameForLogging(),
           aspectAttributes != null ? aspectAttributes : ImmutableMap.<String, Attribute>of());
     }
 
@@ -1519,6 +1542,15 @@ public final class RuleContext extends TargetContext
       return rule;
     }
 
+    /**
+     * Returns a rule class name suitable for log messages, including an aspect name if applicable.
+     */
+    public String getRuleClassNameForLogging() {
+      return aspectName != null
+          ? aspectName + " aspect on " + rule.getRuleClass()
+          : rule.getRuleClass();
+    }
+
     public BuildConfiguration getConfiguration() {
       return configuration;
     }
@@ -1569,7 +1601,7 @@ public final class RuleContext extends TargetContext
           }
         }
         attributeError(attribute.getName(), "'" + prerequisite.getLabel()
-            + "' does not produce any " + rule.getRuleClass() + " " + attribute.getName()
+            + "' does not produce any " + getRuleClassNameForLogging() + " " + attribute.getName()
             + " files (expected " + allowedFileTypes + ")");
       }
     }
@@ -1599,10 +1631,12 @@ public final class RuleContext extends TargetContext
   public static final class ErrorReporter implements RuleErrorConsumer {
     private final AnalysisEnvironment env;
     private final Rule rule;
+    private final String ruleClassNameForLogging;
 
-    ErrorReporter(AnalysisEnvironment env, Rule rule) {
+    ErrorReporter(AnalysisEnvironment env, Rule rule, String ruleClassNameForLogging) {
       this.env = env;
       this.rule = rule;
+      this.ruleClassNameForLogging = ruleClassNameForLogging;
     }
 
     public void reportError(Location location, String message) {
@@ -1635,7 +1669,8 @@ public final class RuleContext extends TargetContext
     }
 
     private String prefixRuleMessage(String message) {
-      return String.format("in %s rule %s: %s", rule.getRuleClass(), rule.getLabel(), message);
+      return String.format(
+          "in %s rule %s: %s", getRuleClassNameForLogging(), rule.getLabel(), message);
     }
 
     private String maskInternalAttributeNames(String name) {
@@ -1657,8 +1692,15 @@ public final class RuleContext extends TargetContext
               : "";
 
       return String.format("in %s attribute of %s rule %s: %s%s",
-          maskInternalAttributeNames(attrName), rule.getRuleClass(), rule.getLabel(), message,
-          macroMessageAppendix);
+          maskInternalAttributeNames(attrName), getRuleClassNameForLogging(), rule.getLabel(),
+          message, macroMessageAppendix);
+    }
+
+    /**
+     * Returns a rule class name suitable for log messages, including an aspect name if applicable.
+     */
+    private String getRuleClassNameForLogging() {
+      return ruleClassNameForLogging;
     }
 
     private String getGeneratorFunction() {