Skylark rules can no longer overwrite built-in attributes.

--
MOS_MIGRATED_REVID=103931317

2 files changed, 19 insertions, 8 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 84296179cb..9e84f0b515 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -770,13 +770,16 @@ public final class RuleClass {
 
     /**
      * Adds or overrides the attribute in the rule class. Meant for Skylark usage.
+     *
+     * @throws IllegalArgumentException if the attribute overrides an existing attribute (will be
+     * legal in the future).
      */
     public void addOrOverrideAttribute(Attribute attribute) {
-      if (attributes.containsKey(attribute.getName())) {
-        overrideAttribute(attribute);
-      } else {
-        addAttribute(attribute);
-      }
+      String name = attribute.getName();
+      // Attributes may be overridden in the future.
+      Preconditions.checkArgument(!attributes.containsKey(name),
+          "There is already a built-in attribute '%s' which cannot be overridden", name);
+      addAttribute(attribute);
     }
 
     /** True if the rule class contains an attribute named {@code name}. */
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
index f2a3733b16..88f61ec942 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
@@ -264,11 +264,12 @@ public class SkylarkRuleClassFunctions {
           Attribute.Builder<?> attrBuilder = (Attribute.Builder<?>) attr.getValue();
           String attrName =
               attributeToNative(attr.getKey(), ast.getLocation(), attrBuilder.hasLateBoundValue());
-          builder.addOrOverrideAttribute(attrBuilder.build(attrName));
+          addAttribute(builder, attrBuilder.build(attrName));
         }
       }
       if (executable || test) {
-        builder.addOrOverrideAttribute(
+        addAttribute(
+            builder,
             attr("$is_executable", BOOLEAN)
                 .value(true)
                 .nonconfigurable("Called from RunCommand.isExecutable, which takes a Target")
@@ -297,12 +298,19 @@ public class SkylarkRuleClassFunctions {
       }
 
       registerRequiredFragments(fragments, hostFragments, builder);
-
       builder.setConfiguredTargetFunction(implementation);
       builder.setRuleDefinitionEnvironment(funcallEnv);
       return new RuleFunction(builder, type);
     }
 
+    private void addAttribute(RuleClass.Builder builder, Attribute attribute) throws EvalException {
+      try {
+        builder.addOrOverrideAttribute(attribute);
+      } catch (IllegalArgumentException ex) {
+        throw new EvalException(location, ex);
+      }
+    }
+
     private void registerRequiredFragments(
         SkylarkList fragments, SkylarkList hostFragments, RuleClass.Builder builder) {
       Map<ConfigurationTransition, ImmutableSet<String>> map = new HashMap<>();