Skylark rules can no longer overwrite built-in attributes.
--
MOS_MIGRATED_REVID=103931317
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 8429617..9e84f0b 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 @@
/**
* 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 f2a3733..88f61ec 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 @@
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 @@
}
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<>();
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index c7ea692..cce9b97 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -66,6 +66,13 @@
")");
}
+ public void testCannotOverrideBuiltInAttribute() throws Exception {
+ checkEvalError(
+ "There is already a built-in attribute 'tags' which cannot be overridden",
+ "def impl(ctx): return",
+ "r = rule(impl, attrs = {'tags': attr.string_list()})");
+ }
+
public void testImplicitArgsAttribute() throws Exception {
eval(
"def _impl(ctx):",
