Rollback of commit 3514756.

*** Reason for rollback ***

--
MOS_MIGRATED_REVID=119625653

src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java

@@ -158,7 +158,6 @@ public List<TransitiveInfoCollection> getFiles() {
 
   private RuleContext(
       Builder builder,
-      AttributeMap attributes,
       ListMultimap<String, ConfiguredTarget> targetMap,
       ListMultimap<String, ConfiguredFilesetEntry> filesetEntryMap,
       Set<ConfigMatchingProvider> configConditions,
@@ -173,7 +172,8 @@ private RuleContext(
     this.targetMap = targetMap;
     this.filesetEntryMap = filesetEntryMap;
     this.configConditions = configConditions;
-    this.attributes = attributes;
+    this.attributes =
+        ConfiguredAttributeMapper.of(builder.rule, configConditions);
     this.features = getEnabledFeatures();
     this.ruleClassNameForLogging = ruleClassNameForLogging;
     this.aspectAttributes = aspectAttributes;
@@ -1330,14 +1330,11 @@ RuleContext build() {
       Preconditions.checkNotNull(prerequisiteMap);
       Preconditions.checkNotNull(configConditions);
       Preconditions.checkNotNull(visibility);
-      AttributeMap attributes = ConfiguredAttributeMapper.of(rule, configConditions);
-      validateAttributes(attributes);
       ListMultimap<String, ConfiguredTarget> targetMap = createTargetMap();
       ListMultimap<String, ConfiguredFilesetEntry> filesetEntryMap =
           createFilesetEntryMap(rule, configConditions);
       return new RuleContext(
           this,
-          attributes,
           targetMap,
           filesetEntryMap,
           configConditions,
@@ -1346,10 +1343,6 @@ RuleContext build() {
           aspectAttributes != null ? aspectAttributes : ImmutableMap.<String, Attribute>of());
     }
 
-    private void validateAttributes(AttributeMap attributes) {
-      rule.getRuleClassObject().checkAttributesNonEmpty(rule, reporter, attributes);
-    }
-
     Builder setVisibility(NestedSet<PackageSpecification> visibility) {
       this.visibility = visibility;
       return this;

src/main/java/com/google/devtools/build/lib/packages/RuleClass.java

@@ -1401,6 +1401,7 @@ private BitSet populateDefinedRuleAttributeValues(
       boolean explicit = attributeValues.isAttributeExplicitlySpecified(attributeName);
       setRuleAttributeValue(rule, eventHandler, attr, nativeAttributeValue, explicit);
       definedAttrIndices.set(attrIndex);
+      checkAttrValNonEmpty(rule, eventHandler, attributeValue, attr);
     }
     return definedAttrIndices;
   }
@@ -1456,6 +1457,7 @@ private void populateDefaultRuleAttributeValues(
         attrsWithComputedDefaults.add(attr);
       } else {
         Object defaultValue = getAttributeNoncomputedDefaultValue(attr, pkgBuilder);
+        checkAttrValNonEmpty(rule, eventHandler, defaultValue, attr);
         rule.setAttributeValue(attr, defaultValue, /*explicit=*/ false);
         checkAllowedValues(rule, attr, eventHandler);
       }
@@ -1498,27 +1500,26 @@ private static void populateConfigDependenciesAttribute(Rule rule) {
         /*explicit=*/false);
   }
 
-  public void checkAttributesNonEmpty(
-      Rule rule, RuleErrorConsumer ruleErrorConsumer, AttributeMap attributes) {
-    for (String attributeName : attributes.getAttributeNames()) {
-      Attribute attr = attributes.getAttributeDefinition(attributeName);
-      if (!attr.isNonEmpty()) {
-        continue;
-      }
-      Object attributeValue = attributes.get(attributeName, attr.getType());
-
-      boolean isEmpty = false;
-      if (attributeValue instanceof SkylarkList) {
-        isEmpty = ((SkylarkList) attributeValue).isEmpty();
-      } else if (attributeValue instanceof List<?>) {
-        isEmpty = ((List<?>) attributeValue).isEmpty();
-      } else if (attributeValue instanceof Map<?, ?>) {
-        isEmpty = ((Map<?, ?>) attributeValue).isEmpty();
-      }
+  private void checkAttrValNonEmpty(
+      Rule rule, EventHandler eventHandler, Object attributeValue, Attribute attr) {
+    if (!attr.isNonEmpty()) {
+      return;
+    }
 
-      if (isEmpty) {
-        ruleErrorConsumer.attributeError(attr.getName(), "attribute must be non empty");
-      }
+    boolean isEmpty = false;
+
+    if (attributeValue instanceof SkylarkList) {
+      isEmpty = ((SkylarkList) attributeValue).isEmpty();
+    } else if (attributeValue instanceof List<?>) {
+      isEmpty = ((List<?>) attributeValue).isEmpty();
+    } else if (attributeValue instanceof Map<?, ?>) {
+      isEmpty = ((Map<?, ?>) attributeValue).isEmpty();
+    }
+
+    if (isEmpty) {
+      rule.reportError(rule.getLabel() + ": non empty attribute '" + attr.getName()
+          + "' in '" + name + "' rule '" + rule.getLabel() + "' has to have at least one value",
+          eventHandler);
     }
   }
 

src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java

@@ -1449,8 +1449,8 @@ protected String getErrorMsgMisplacedRules(String attrName, String ruleType, Str
   }
 
   protected String getErrorMsgNonEmptyList(String attrName, String ruleType, String ruleName) {
-    return "in " + attrName + " attribute of " + ruleType + " rule " + ruleName + ": attribute "
-        + "must be non empty";
+    return "non empty attribute '" + attrName + "' in '" + ruleType
+        + "' rule '" + ruleName + "' has to have at least one value";
   }
 
   protected String getErrorMsgMandatoryMissing(String attrName, String ruleType) {

src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java

@@ -613,6 +613,68 @@ public void testOrderIndependentAttribute() throws Exception {
                  attributes.get("my-sorted-stringlist-attr", Type.STRING_LIST));
   }
 
+  @Test
+  public void testNonEmptyGood() throws Exception {
+    RuleClass mneRuleClass = setupNonEmpty(
+        attr("list1", LABEL_LIST).mandatory().legacyAllowAnyFileType().build(),
+        attr("list2", LABEL_LIST).nonEmpty().legacyAllowAnyFileType().build(),
+        attr("list3", STRING_LIST).nonEmpty().build());
+
+    Map<String, Object> attributeValues = new LinkedHashMap<>();
+    attributeValues.put("list1", Lists.newArrayList());
+    attributeValues.put("list2", Lists.newArrayList(":nodup1", ":nodup2"));
+    attributeValues.put("list3", Lists.newArrayList("val1", "val2"));
+
+    createRule(mneRuleClass, "ruleTestMNE", attributeValues, testRuleLocation);
+  }
+
+  @Test
+  public void testNonEmptyFail() throws Exception {
+    RuleClass mandNonEmptyRuleClass = setupNonEmpty(
+        attr("list", LABEL_LIST).nonEmpty().legacyAllowAnyFileType().build());
+
+    Map<String, Object> attributeValues = new LinkedHashMap<>();
+    attributeValues.put("list", Lists.newArrayList());
+
+    reporter.removeHandler(failFastHandler);
+    createRule(mandNonEmptyRuleClass, "ruleTestMNE", attributeValues, testRuleLocation);
+
+    assertSame(1, eventCollector.count());
+    assertContainsEvent(getErrorMsgNonEmptyList(
+        "list", "ruleMNE", "//testpackage:ruleTestMNE"));
+  }
+
+  private RuleClass setupNonEmpty(Attribute... attributes) {
+    RuleClass mandNonEmptyRuleClass = new RuleClass(
+        "ruleMNE", false, false, false, false, false, false,
+        ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
+        PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
+        ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
+        MissingFragmentPolicy.FAIL_ANALYSIS, true, attributes);
+    return mandNonEmptyRuleClass;
+  }
+
+  @Test
+  public void testNonEmptyWrongDefVal() throws Exception {
+    List<Label> emptyList = ImmutableList.of();
+    RuleClass mandNonEmptyRuleClass = new RuleClass(
+        "ruleMNE", false, false, false, false, false, false,
+        ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
+        PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
+        ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
+        MissingFragmentPolicy.FAIL_ANALYSIS, true, attr("list", LABEL_LIST)
+        .nonEmpty().legacyAllowAnyFileType().value(emptyList).build());
+
+    Map<String, Object> attributeValues = new LinkedHashMap<>();
+    reporter.removeHandler(failFastHandler);
+    createRule(mandNonEmptyRuleClass, "ruleTestMNE", attributeValues, testRuleLocation);
+
+    assertSame(1, eventCollector.count());
+
+    assertContainsEvent(getErrorMsgNonEmptyList(
+        "list", "ruleMNE", "//testpackage:ruleTestMNE"));
+  }
+
   private Rule createRule(RuleClass ruleClass, String name, Map<String, Object> attributeValues,
       Location location) throws LabelSyntaxException, InterruptedException {
     Package.Builder pkgBuilder = createDummyPackageBuilder();

src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java

@@ -276,4 +276,9 @@ protected void invalidatePackages() throws InterruptedException {
     skyframeExecutor.invalidateFilesUnderPathForTesting(
         reporter, ModifiedFileSet.EVERYTHING_MODIFIED, rootDirectory);
   }
+
+  protected String getErrorMsgNonEmptyList(String attrName, String ruleType, String ruleName) {
+    return "non empty attribute '" + attrName + "' in '" + ruleType
+        + "' rule '" + ruleName + "' has to have at least one value";
+  }
 }