Refactoring: Types report what class of labels they contain.

Currently label-type attributes are detected in many places across the
codebase by simply reference-comparing against each of the label types.
This CL aims to generalize most of these cases, moving the encoding of
this logic into a single place (Type/BuildType itself). Not all of these
cases can be made general without further refactoring, and some perhaps
shouldn't be - serialization and Skylark rule context, for example, need
to do exotic things based on the type. But most sites can avoid having to
enumerate all the types they work with explicitly.

This causes LABEL_DICT_UNARY to start being treated like the other label
types, which means that CcToolchainSuiteRule and JavaRuntimeSuiteRule
need to include a set of allowed file types (none, in their case).

--
PiperOrigin-RevId: 147175424
MOS_MIGRATED_REVID=147175424

1 files changed, 25 insertions, 20 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
index c7e11c0b60..47f4662774 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
@@ -25,11 +25,12 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
 import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.syntax.Type.LabelClass;
+import com.google.devtools.build.lib.syntax.Type.LabelVisitor;
 import com.google.devtools.build.lib.util.Preconditions;
 import java.util.Collection;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
-import java.util.List;
 import java.util.Map;
 import javax.annotation.Nullable;
 
@@ -190,32 +191,36 @@ public final class AspectDefinition {
    * Collects all attribute labels from the specified aspectDefinition.
    */
   public static void addAllAttributesOfAspect(
-      Rule from,
-      Multimap<Attribute, Label> labelBuilder,
+      final Rule from,
+      final Multimap<Attribute, Label> labelBuilder,
       Aspect aspect,
       DependencyFilter dependencyFilter) {
     ImmutableMap<String, Attribute> attributes = aspect.getDefinition().getAttributes();
-    for (Attribute aspectAttribute : attributes.values()) {
+    for (final Attribute aspectAttribute : attributes.values()) {
       if (!dependencyFilter.apply(aspect, aspectAttribute)) {
         continue;
       }
-      if (aspectAttribute.getType() == BuildType.LABEL) {
-        Label label = maybeGetRepositoryRelativeLabel(
-            from, BuildType.LABEL.cast(aspectAttribute.getDefaultValue(from)));
-        if (label != null) {
-          labelBuilder.put(aspectAttribute, label);
-        }
-      } else if (aspectAttribute.getType() == BuildType.LABEL_LIST) {
-        List<Label> defaultLabels = BuildType.LABEL_LIST.cast(
+      Type type = aspectAttribute.getType();
+      if (type.getLabelClass() != LabelClass.DEPENDENCY) {
+        continue;
+      }
+      try {
+        type.visitLabels(
+            new LabelVisitor() {
+              @Override
+              public void visit(Label label) {
+                Label repositoryRelative = maybeGetRepositoryRelativeLabel(from, label);
+                if (repositoryRelative == null) {
+                  return;
+                }
+                labelBuilder.put(aspectAttribute, repositoryRelative);
+              }
+            },
             aspectAttribute.getDefaultValue(from));
-        if (defaultLabels != null) {
-          for (Label defaultLabel : defaultLabels) {
-            Label label = maybeGetRepositoryRelativeLabel(from, defaultLabel);
-            if (label != null) {
-              labelBuilder.put(aspectAttribute, label);
-            }
-          }
-        }
+      } catch (InterruptedException ex) {
+        // Because the LabelVisitor does not throw InterruptedException, it should not be thrown
+        // by visitLabels here.
+        throw new AssertionError(ex);
       }
     }
   }