Fix aliases for users of label-keyed string dicts.

Aliases mess with the assumption that

    attributeValue.containsKey(target.getLabel())

for every target in the prerequisites of a LABEL_KEYED_STRING_DICT
attribute.

The solution is to use AliasProvider.getDependencyLabel(target) instead.
This fixes it for all current users, including SkylarkRuleContext.
This also adjusts config_setting flag_values and Android feature_flags
to do intelligent things with aliases in their respective attributes.

RELNOTES: None.
PiperOrigin-RevId: 157594095

3 files changed, 167 insertions, 0 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
index 1f3767221e..d520a7066a 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
@@ -2762,6 +2762,36 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
   }
 
   @Test
+  public void testFeatureFlagsAttributeFailsAnalysisIfFlagIsAliased()
+      throws Exception {
+    reporter.removeHandler(failFastHandler);
+    useConfiguration("--experimental_dynamic_configs=on");
+    scratch.file(
+        "java/com/foo/BUILD",
+        "config_feature_flag(",
+        "  name = 'flag1',",
+        "  allowed_values = ['on', 'off'],",
+        "  default_value = 'off',",
+        ")",
+        "alias(",
+        "  name = 'alias',",
+        "  actual = 'flag1',",
+        ")",
+        "android_binary(",
+        "  name = 'foo',",
+        "  manifest = 'AndroidManifest.xml',",
+        "  feature_flags = {",
+        "    'alias': 'on',",
+        "  }",
+        ")");
+    assertThat(getConfiguredTarget("//java/com/foo")).isNull();
+    assertContainsEvent(
+        "in feature_flags attribute of android_binary rule //java/com/foo:foo: "
+            + "Feature flags must be named directly, not through aliases; "
+            + "use '//java/com/foo:flag1', not '//java/com/foo:alias'");
+  }
+
+  @Test
   public void testFeatureFlagsAttributeSetsFeatureFlagProviderValues() throws Exception {
     useConfiguration("--experimental_dynamic_configs=on");
     scratch.file(
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
index 26168f6cd3..b46af1e43d 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
@@ -932,6 +932,90 @@ public class ConfigSettingTest extends BuildViewTestCase {
   }
 
   @Test
+  public void matchesAliasedFlagsInFlagValues() throws Exception {
+    scratch.file(
+        "test/BUILD",
+        "config_setting(",
+        "    name = 'alias_matcher',",
+        "    flag_values = {",
+        "        ':alias': 'right',",
+        "    },",
+        ")",
+        "alias(",
+        "    name = 'alias',",
+        "    actual = 'flag',",
+        ")",
+        "config_feature_flag(",
+        "    name = 'flag',",
+        "    allowed_values = ['right', 'wrong'],",
+        "    default_value = 'right',",
+        ")");
+    assertThat(getConfigMatchingProvider("//test:alias_matcher").matches()).isTrue();
+  }
+
+  @Test
+  public void aliasedFlagsAreCountedInRefining() throws Exception {
+    scratch.file(
+        "test/BUILD",
+        "config_setting(",
+        "    name = 'refined',",
+        "    flag_values = {",
+        "        ':alias': 'right',",
+        "        ':flag2': 'good',",
+        "    },",
+        ")",
+        "config_setting(",
+        "    name = 'other',",
+        "    flag_values = {",
+        "        ':flag': 'right',",
+        "    },",
+        ")",
+        "alias(",
+        "    name = 'alias',",
+        "    actual = 'flag',",
+        ")",
+        "config_feature_flag(",
+        "    name = 'flag',",
+        "    allowed_values = ['right', 'wrong'],",
+        "    default_value = 'right',",
+        ")",
+        "config_feature_flag(",
+        "    name = 'flag2',",
+        "    allowed_values = ['good', 'bad'],",
+        "    default_value = 'good',",
+        ")");
+    assertThat(
+            getConfigMatchingProvider("//test:refined")
+                .refines(getConfigMatchingProvider("//test:other")))
+        .isTrue();
+  }
+
+  @Test
+  public void referencingSameFlagViaMultipleAliasesFails() throws Exception {
+    checkError(
+        "test",
+        "multialias",
+        "in flag_values attribute of config_setting rule //test:multialias: "
+            + "flag '//test:direct' referenced multiple times as ['//test:alias', '//test:direct']",
+        "config_setting(",
+        "    name = 'multialias',",
+        "    flag_values = {",
+        "        ':alias': 'right',",
+        "        ':direct': 'right',",
+        "    },",
+        ")",
+        "alias(",
+        "    name = 'alias',",
+        "    actual = 'direct',",
+        ")",
+        "config_feature_flag(",
+        "    name = 'direct',",
+        "    allowed_values = ['right', 'wrong'],",
+        "    default_value = 'right',",
+        ")");
+  }
+
+  @Test
   public void forbidsNonConfigFeatureFlagRulesForFlagValues() throws Exception {
     checkError("test", "invalid_flag",
         "in flag_values attribute of config_setting rule //test:invalid_flag: "
@@ -965,4 +1049,26 @@ public class ConfigSettingTest extends BuildViewTestCase {
         "    default_value = 'valid',",
         ")");
   }
+
+  @Test
+  public void usesAliasLabelWhenReportingErrorInFlagValues() throws Exception {
+    checkError("test", "invalid_flag",
+        "in flag_values attribute of config_setting rule //test:invalid_flag: "
+        + "error while parsing user-defined configuration values: "
+        + "'invalid' is not a valid value for '//test:alias'",
+        "config_setting(",
+        "    name = 'invalid_flag',",
+        "    flag_values = {",
+        "        ':alias': 'invalid',",
+        "    })",
+        "alias(",
+        "    name = 'alias',",
+        "    actual = ':flag',",
+        ")",
+        "config_feature_flag(",
+        "    name = 'flag',",
+        "    allowed_values = ['right', 'valid'],",
+        "    default_value = 'valid',",
+        ")");
+  }
 }
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 0330ccc863..8eeeb23bb4 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
@@ -827,6 +827,37 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
   }
 
   @Test
+  public void testLabelKeyedStringDictTranslatesAliases() throws Exception {
+    scratch.file(
+        "my_rule.bzl",
+        "def _impl(ctx):",
+        "  return",
+        "my_rule = rule(",
+        "  implementation = _impl,",
+        "  attrs = {",
+        "    'label_dict': attr.label_keyed_string_dict(),",
+        "  }",
+        ")");
+
+    scratch.file(
+        "BUILD",
+        "filegroup(name='dep')",
+        "alias(name='alias', actual='dep')",
+        "load('//:my_rule.bzl', 'my_rule')",
+        "my_rule(name='r',",
+        "        label_dict={':alias': 'value'})");
+
+    invalidatePackages();
+    SkylarkRuleContext context = createRuleContext("//:r");
+    Label keyLabel =
+        (Label) evalRuleContextCode(context, "ruleContext.attr.label_dict.keys()[0].label");
+    assertThat(keyLabel).isEqualTo(Label.parseAbsolute("//:dep"));
+    String valueString =
+        (String) evalRuleContextCode(context, "ruleContext.attr.label_dict.values()[0]");
+    assertThat(valueString).isEqualTo("value");
+  }
+
+  @Test
   public void testLabelKeyedStringDictAcceptsDefaultValues() throws Exception {
     scratch.file(
         "my_rule.bzl",