Convert values to Starlark values when concating string_list_dict type

Convert values to Starlark values when concating string_list_dict type.

This is because Dict checks that all the values put in it are Starlark valid values. But string_list_dict value is of type List<String>, when Java's List isn't a Starlark value.

Fixes bazelbuild#23065

My first PR to bazel 🤩 Would love you to review the PR.
 Checked that the test didn't pass before the change and did after it.

Closes bazelbuild#23424.

PiperOrigin-RevId: 673463108
Change-Id: Ib2cdce7d94243f4e3bda23203a196fee4e766189

src/main/java/com/google/devtools/build/lib/packages/Type.java

@@ -621,11 +621,11 @@ public Object copyAndLiftStarlarkValue(
 
     @Override
     public Map<KeyT, ValueT> concat(Iterable<Map<KeyT, ValueT>> iterable) {
-      Dict.Builder<KeyT, ValueT> output = new Dict.Builder<>();
+      ImmutableMap.Builder<KeyT, ValueT> builder = ImmutableMap.builder();
       for (Map<KeyT, ValueT> map : iterable) {
-        output.putAll(map);
+        builder.putAll(map);
       }
-      return output.buildImmutable();
+      return builder.buildKeepingLast();
     }
 
     @Override

src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java

@@ -31,10 +31,12 @@
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.RuleClass.ToolchainResolutionMode;
 import com.google.devtools.build.lib.packages.Type;
+import com.google.devtools.build.lib.packages.Types;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
 import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
 import com.google.devtools.build.lib.util.FileTypeSet;
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Set;
 import org.junit.Before;
@@ -166,6 +168,12 @@ public Object getDefault(AttributeMap rule) {
                       .add(attr("deps", LABEL_LIST).allowedFileTypes())
                       .toolchainResolutionMode(ToolchainResolutionMode.DISABLED));
 
+  private static final MockRule RULE_WITH_STRING_LIST_DICT_ATTR =
+      () ->
+          MockRule.define(
+              "rule_with_string_list_dict_attr",
+              attr("string_list_dict_attr", Types.STRING_LIST_DICT));
+
   @Override
   protected ConfiguredRuleClassProvider createRuleClassProvider() {
     ConfiguredRuleClassProvider.Builder builder =
@@ -175,7 +183,8 @@ protected ConfiguredRuleClassProvider createRuleClassProvider() {
             .addRuleDefinition(RULE_WITH_BOOLEAN_ATTR)
             .addRuleDefinition(RULE_WITH_ALLOWED_VALUES)
             .addRuleDefinition(RULE_WITH_LABEL_DEFAULT)
-            .addRuleDefinition(RULE_WITH_NO_PLATFORM);
+            .addRuleDefinition(RULE_WITH_NO_PLATFORM)
+            .addRuleDefinition(RULE_WITH_STRING_LIST_DICT_ATTR);
     TestRuleClassProvider.addStandardRules(builder);
     // Allow use of --foo as a dummy flag
     builder.addConfigurationFragment(DummyTestFragment.class);
@@ -2115,4 +2124,24 @@ def my_java_binary(name, deps = [], **kwargs):
         /*expected:*/ ImmutableList.of("bin java/foo/libb.jar", "bin java/foo/libb2.jar"),
         /*not expected:*/ ImmutableList.of("bin java/foo/liba.jar", "bin java/foo/liba2.jar"));
   }
+
+  @Test
+  public void stringListDictTypeConcatConfigurable() throws Exception {
+    writeConfigRules();
+    scratch.file(
+        "foo/BUILD",
+        """
+        rule_with_string_list_dict_attr(
+            name = 'rule',
+            string_list_dict_attr =  {'a': ['a.out']} | select({
+                '//conditions:b': {'b': ['b.out']},
+            }))
+        """);
+
+    useConfiguration("--foo=b");
+    ConfiguredTargetAndData ctad = getConfiguredTargetAndData("//foo:rule");
+    AttributeMap attributes = getMapperFromConfiguredTargetAndTarget(ctad);
+    assertThat(attributes.get("string_list_dict_attr", Types.STRING_LIST_DICT))
+        .containsExactly("a", Arrays.asList("a.out"), "b", Arrays.asList("b.out"));
+  }
 }

src/test/java/com/google/devtools/build/lib/packages/TypeTest.java

@@ -374,6 +374,35 @@ public void testStringListDict() throws Exception {
     assertThat(collectLabels(Types.STRING_LIST_DICT, converted)).isEmpty();
   }
 
+  @Test
+  public void testStringListDict_concat() throws Exception {
+    assertThat(Types.STRING_LIST_DICT.concat(ImmutableList.of())).isEmpty();
+
+    ImmutableMap<String, List<String>> expected =
+        ImmutableMap.of(
+            "foo", Arrays.asList("foo", "bar"),
+            "wiz", Arrays.asList("bang"));
+    assertThat(Types.STRING_LIST_DICT.concat(ImmutableList.of(expected))).isEqualTo(expected);
+
+    ImmutableMap<String, List<String>> map1 =
+        ImmutableMap.of(
+            "foo", Arrays.asList("a", "b"),
+            "bar", Arrays.asList("c", "d"));
+    ImmutableMap<String, List<String>> map2 =
+        ImmutableMap.of(
+            "bar", Arrays.asList("x", "y"),
+            "baz", Arrays.asList("z"));
+
+    ImmutableMap<String, List<String>> expectedAfterConcat =
+        ImmutableMap.of(
+            "foo", Arrays.asList("a", "b"),
+            "bar", Arrays.asList("x", "y"),
+            "baz", Arrays.asList("z"));
+
+    assertThat(Types.STRING_LIST_DICT.concat(ImmutableList.of(map1, map2)))
+        .isEqualTo(expectedAfterConcat);
+  }
+
   @Test
   public void testStringListDictBadFirstElement() throws Exception {
     Object input =