Remove special handling of name attribute. Fixes #278

The name attribute gets special treatment in the codebase, in that
it's not simply yet another attribute but stored in it's own field.

Thus, every callside dealing with attributes needs to be aware of
this special case and explicitly handle the name attribute. It's
easy to see that this can lead to bugs. For example, querying for
the name attribute is currently broken due the querying code not
being aware of the special case [1].

Discussions with experienced bazel developers came to the conclusion
that there is no need (anymore) to treat the name attribute specially
and thus we decided it's best to remove the special treatment and
handle the name attribute as any other attribute.

This change removes the handling of name attributes and also adds a test
case to verify that bug [1] is fixed.

[1] https://github.com/bazelbuild/bazel/issues/278

--
PiperOrigin-RevId: 147446345
MOS_MIGRATED_REVID=147446345

1 files changed, 30 insertions, 6 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
index ebffbf8e42..8739918377 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
@@ -151,6 +151,7 @@ public class BaseRuleClasses {
       return RuleDefinition.Metadata.builder()
           .name("$test_base_rule")
           .type(RuleClassType.ABSTRACT)
+          .ancestors(RootRule.class)
           .build();
     }
   }
@@ -160,8 +161,6 @@ public class BaseRuleClasses {
    */
   public static RuleClass.Builder commonCoreAndSkylarkAttributes(RuleClass.Builder builder) {
     return builder
-        .add(attr("name", STRING)
-            .nonconfigurable("Rule name"))
         // The visibility attribute is special: it is a nodep label, and loading the
         // necessary package groups is handled by {@link LabelVisitor#visitTargetVisibility}.
         // Package groups always have the null configuration so that they are not duplicated
@@ -198,15 +197,38 @@ public class BaseRuleClasses {
         );
   }
 
+  public static RuleClass.Builder nameAttribute(RuleClass.Builder builder) {
+    return builder.add(attr("name", STRING).nonconfigurable("Rule name"));
+  }
+
   /**
-   * Common parts of rules.
+   * Ancestor of every rule.
+   *
+   * <p>Adds the name attribute to every rule.
+   */
+  public static final class RootRule implements RuleDefinition {
+
+    @Override
+    public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) {
+        return nameAttribute(builder).build();
+    }
+
+    @Override
+    public Metadata getMetadata() {
+      return RuleDefinition.Metadata.builder()
+          .name("$root_rule")
+          .type(RuleClassType.ABSTRACT)
+          .build();
+    }
+  }
+
+  /**
+   * Common parts of some rules.
    */
   public static final class BaseRule implements RuleDefinition {
     @Override
     public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
       return commonCoreAndSkylarkAttributes(builder)
-          // The name attribute is handled specially, so it does not appear here.
-          //
           // Aggregates the labels of all {@link ConfigRuleClasses} rules this rule uses (e.g.
           // keys for configurable attributes). This is specially populated in
           // {@RuleClass#populateRuleAttributeValues}.
@@ -233,12 +255,13 @@ public class BaseRuleClasses {
       return RuleDefinition.Metadata.builder()
           .name("$base_rule")
           .type(RuleClassType.ABSTRACT)
+          .ancestors(RootRule.class)
           .build();
     }
   }
 
   /**
-   * Common ancestor class for all rules.
+   * Common ancestor class for some rules.
    */
   public static final class RuleBase implements RuleDefinition {
     @Override
@@ -283,6 +306,7 @@ public class BaseRuleClasses {
       return RuleDefinition.Metadata.builder()
           .name("$binary_base_rule")
           .type(RuleClassType.ABSTRACT)
+          .ancestors(RootRule.class)
           .build();
     }
   }