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).
Skylark will continue treating it as a dictionary from String to Label
in its rule context, however, to avoid visible behavior changes.

--
PiperOrigin-RevId: 147471542
MOS_MIGRATED_REVID=147471542

2 files changed, 30 insertions, 14 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index 078d6af669..578db1b98f 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -1383,4 +1383,25 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
         "ruleContext.coverage_instrumented(ruleContext.attr.deps[0])");
     assertThat((Boolean) result).isTrue();
   }
+
+  @Test
+  public void testStringKeyedLabelDictAttributeInSkylarkRuleContext() throws Exception {
+    scratch.file("jvm/BUILD",
+        "java_runtime(name='runtime', srcs=[], java_home='')",
+        "java_runtime_suite(",
+        "  name = 'suite',",
+        "  runtimes = {'x86': ':runtime'},",
+        "  default = ':runtime',",
+        ")");
+
+    invalidatePackages();
+    SkylarkRuleContext ruleContext = createRuleContext("//jvm:suite");
+    assertNoEvents();
+    String keyString =
+        (String) evalRuleContextCode(ruleContext, "ruleContext.attr.runtimes.keys()[0]");
+    assertThat(keyString).isEqualTo("x86");
+    Label valueLabel =
+        (Label) evalRuleContextCode(ruleContext, "ruleContext.attr.runtimes.values()[0]");
+    assertThat(valueLabel).isEqualTo(Label.parseAbsolute("//jvm:runtime"));
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/BuildRuleWithDefaultsBuilder.java b/src/test/java/com/google/devtools/build/lib/testutil/BuildRuleWithDefaultsBuilder.java
index 7db103eba3..e0e6b77407 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/BuildRuleWithDefaultsBuilder.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/BuildRuleWithDefaultsBuilder.java
@@ -20,6 +20,8 @@ import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.RuleClass;
 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.ListType;
 import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.build.lib.util.Preconditions;
 
@@ -57,8 +59,8 @@ public class BuildRuleWithDefaultsBuilder extends BuildRuleBuilder {
    */
   private String getDummyFileLabel(String rulePkg, String filePkg, String extension,
       Type<?> attrType) {
-    boolean isInput = (attrType == BuildType.LABEL || attrType == BuildType.LABEL_LIST);
-    String fileName = (isInput ? "dummy_input" : "dummy_output") + extension;
+    boolean isOutput = attrType.getLabelClass() == LabelClass.OUTPUT;
+    String fileName = (isOutput ? "dummy_output" : "dummy_input") + extension;
     generateFiles.add(filePkg + "/" + fileName);
     if (rulePkg.equals(filePkg)) {
       return ":" + fileName;
@@ -122,7 +124,7 @@ public class BuildRuleWithDefaultsBuilder extends BuildRuleBuilder {
       }
     }
     if (label != null) {
-      if (attrType == BuildType.LABEL_LIST || attrType == BuildType.OUTPUT_LIST) {
+      if (attrType instanceof ListType<?>) {
         addMultiValueAttributes(attribute.getName(), label);
       } else {
         setSingleValueAttribute(attribute.getName(), label);
@@ -175,17 +177,10 @@ public class BuildRuleWithDefaultsBuilder extends BuildRuleBuilder {
   public BuildRuleWithDefaultsBuilder populateAttributes(String rulePkg, boolean heuristics) {
     for (Attribute attribute : ruleClass.getAttributes()) {
       if (attribute.isMandatory()) {
-        if (attribute.getType() == BuildType.LABEL_LIST
-            || attribute.getType() == BuildType.OUTPUT_LIST) {
-          if (attribute.isNonEmpty()) {
-            populateLabelAttribute(rulePkg, attribute);
-          } else {
-            // TODO(bazel-team): actually here an empty list would be fine, but BuildRuleBuilder
-            // doesn't support that, and it makes little sense anyway
-            populateLabelAttribute(rulePkg, attribute);
-          }
-        } else if (attribute.getType() == BuildType.LABEL
-            || attribute.getType() == BuildType.OUTPUT) {
+        if (BuildType.isLabelType(attribute.getType())) {
+          // TODO(bazel-team): actually an empty list would be fine in the case where
+          // attribute instanceof ListType && !attribute.isNonEmpty(), but BuildRuleBuilder
+          // doesn't support that, and it makes little sense anyway
           populateLabelAttribute(rulePkg, attribute);
         } else {
           // Non label type attributes