Convert values to Starlark values when concating string_list_dict type

[lior10r]

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, when Java's List isn't a Starlark value.

Fixes #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.

[tetromino]

Thank you for your contribution! I had reviewed #15075 two years ago - and had failed to notice the problem.

My main concern with the cast() call is that (1) it introduces a completely unnecessary temporary Dict object per each concatenated map, and (2) the semantics of Type.concat - as far as I can tell - don't require the returned value to be a subclass of StarlarkValue (compare Type.ListType.concat, which returns an ImmutableList, not a StarlarkList).

So instead, can we use a plain ordinary ImmutableMap.Builder?

      ImmutableMap.Builder<KeyT, ValueT> builder = ImmutableMap.builder();
      for (Map<KeyT, ValueT> map : iterable) {
        builder.putAll(map);
      }
      return builder.buildKeepingLast();

[lior10r]

Thanks for the review, this is really the better solution for the problem.
I thought maybe adding another test that recreates the scenario that I posted on the issue. What do you think about it? if yes, where should I add it to?

[tetromino]

Thanks for the review, this is really the better solution for the problem. I thought maybe adding another test that recreates the scenario that I posted on the issue. What do you think about it? if yes, where should I add it to?

@lior10r - that's an excellent idea. I would suggest adding such a test as a new method to src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java

[tetromino]

Thanks for the test, looks good! I apologize for missing notification about the last commit.

[tetromino]

@bazel-io fork 7.4.0