Allow dict() and dict.update() to take arbitrary maps, not necessaril…

…y dicts. In particular, this allows immutable view objects returned by native.existing_rule/s() with --incompatible_existing_rules_immutable_view to be used in dict() and dict.update(). Partially addresses #13605 PiperOrigin-RevId: 478527510 Change-Id: Ic796df9d556027c5d797b03ca85aebf1c90165cd
bazelbuild · Oct 3, 2022 · dfa9c62 · dfa9c62
1 parent cf99f84
commit dfa9c62
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 3 deletions.
diff --git a/src/main/java/net/starlark/java/eval/Dict.java b/src/main/java/net/starlark/java/eval/Dict.java
@@ -339,10 +339,10 @@ public void update(Object pairs, Dict<String, Object> kwargs, StarlarkThread thr
 
   // Common implementation of dict(pairs, **kwargs) and dict.update(pairs, **kwargs).
   static void update(
-      String funcname, Dict<Object, Object> dict, Object pairs, Dict<String, Object> kwargs)
+      String funcname, Dict<Object, Object> dict, Object pairs, Map<String, Object> kwargs)
       throws EvalException {
-    if (pairs instanceof Dict) { // common case
-      dict.putEntries((Dict<?, ?>) pairs);
+    if (pairs instanceof Map) { // common case
+      dict.putEntries((Map<?, ?>) pairs);
     } else {
       Iterable<?> iterable;
       try {

diff --git a/src/test/java/com/google/devtools/build/lib/packages/NativeExistingRulesTest.java b/src/test/java/com/google/devtools/build/lib/packages/NativeExistingRulesTest.java
@@ -336,6 +336,52 @@ public void existingRule_returnsDictLikeObject() throws Exception {
     assertThat(getSaved("'invalid_attr' in r")).isEqualTo(false);
   }
 
+  @Test
+  public void existingRule_asDictArgument() throws Exception {
+    scratch.file(
+        "test/test.bzl",
+        "def save_as_dict(r):",
+        "  test.save('type(dict(r))', type(dict(r)))",
+        "  test.save('dict(r)[\"name\"]', dict(r)[\"name\"])",
+        "  test.save('dict(r)[\"kind\"]', dict(r)[\"kind\"])");
+    scratch.file(
+        "test/BUILD",
+        "load('//test:test.bzl', 'save_as_dict')", //
+        "cc_library(",
+        "    name ='rulename',",
+        ")",
+        "save_as_dict(existing_rule('rulename'))");
+    getConfiguredTarget("//test:rulename");
+    assertThat(getSaved("type(dict(r))")).isEqualTo("dict");
+    assertThat(getSaved("dict(r)[\"name\"]")).isEqualTo("rulename");
+    assertThat(getSaved("dict(r)[\"kind\"]")).isEqualTo("cc_library");
+  }
+
+  @Test
+  public void existingRule_asDictUpdateArgument() throws Exception {
+    // We do not test `existing_rule(r).update({...})` because `existing_rule(r)` may be immutable
+    // (as verified by other test cases).
+    scratch.file(
+        "test/test.bzl",
+        "def save_as_updated_dict(r):",
+        "  updated_dict = {'name': 'dictname', 'dictkey': 1}",
+        "  updated_dict.update(r)",
+        "  test.save('updated_dict[\"name\"]', updated_dict[\"name\"])",
+        "  test.save('updated_dict[\"kind\"]', updated_dict[\"kind\"])",
+        "  test.save('updated_dict[\"dictkey\"]', updated_dict[\"dictkey\"])");
+    scratch.file(
+        "test/BUILD",
+        "load('//test:test.bzl', 'save_as_updated_dict')", //
+        "cc_library(",
+        "    name ='rulename',",
+        ")",
+        "save_as_updated_dict(existing_rule('rulename'))");
+    getConfiguredTarget("//test:rulename");
+    assertThat(getSaved("updated_dict[\"name\"]")).isEqualTo("rulename");
+    assertThat(getSaved("updated_dict[\"kind\"]")).isEqualTo("cc_library");
+    assertThat(getSaved("updated_dict[\"dictkey\"]")).isEqualTo(StarlarkInt.of(1));
+  }
+
   @Test
   public void existingRule_asKwargs() throws Exception {
     scratch.file(