Extend dict update to accept keyword args

src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java

@@ -33,7 +33,6 @@
 import com.google.devtools.build.lib.syntax.EvalUtils.ComparisonException;
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
-import com.google.devtools.build.lib.syntax.Type.ConversionException;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Iterator;
@@ -541,40 +540,9 @@ private String getIntegerPrefix(String value) {
           SkylarkDict<?, ?> argsDict =
               (args instanceof SkylarkDict)
                   ? (SkylarkDict<?, ?>) args
-                  : getDictFromArgs(args, loc, env);
+                  : SkylarkDict.getDictFromArgs(args, loc, env);
           return SkylarkDict.plus(argsDict, kwargs, env);
         }
-
-        private SkylarkDict<Object, Object> getDictFromArgs(
-            Object args, Location loc, Environment env) throws EvalException {
-          SkylarkDict<Object, Object> result = SkylarkDict.of(env);
-          int pos = 0;
-          for (Object element : Type.OBJECT_LIST.convert(args, "parameter args in dict()")) {
-            List<Object> pair = convertToPair(element, pos, loc);
-            result.put(pair.get(0), pair.get(1), loc, env);
-            ++pos;
-          }
-          return result;
-        }
-
-        private List<Object> convertToPair(Object element, int pos, Location loc)
-            throws EvalException {
-          try {
-            List<Object> tuple = Type.OBJECT_LIST.convert(element, "");
-            int numElements = tuple.size();
-            if (numElements != 2) {
-              throw new EvalException(
-                  location,
-                  String.format(
-                      "item #%d has length %d, but exactly two elements are required",
-                      pos, numElements));
-            }
-            return tuple;
-          } catch (ConversionException e) {
-            throw new EvalException(
-                loc, String.format("cannot convert item #%d to a sequence", pos), e);
-          }
-        }
       };
 
   @SkylarkSignature(

src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java

@@ -26,6 +26,8 @@
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
 import com.google.devtools.build.lib.syntax.SkylarkMutable.MutableMap;
+import com.google.devtools.build.lib.syntax.Type.ConversionException;
+import java.util.List;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.LinkedHashMap;
@@ -185,19 +187,39 @@ public Object setdefault(
 
   @SkylarkCallable(
     name = "update",
-    doc = "Update the dictionary with the key/value pairs from other, overwriting existing keys.",
+    doc = "Update the dictionary with an optional positional argument <code>[pairs]</code> "
+              + " and an optional set of keyword arguments <code>[, name=value[, ...]</code>\n"
+              + "If the positional argument <code>pairs</code> is present, it must be "
+              + "<code>None</code>, another <code>dict</code>, or some other iterable. "
+              + "If it is another <code>dict</code>, then its key/value pairs are inserted. "
+              + "If it is an iterable, it must provide a sequence of pairs (or other iterables "
+              + "of length 2), each of which is treated as a key/value pair to be inserted.\n"
+              + "For each <code>name=value</code> argument present, the name is converted to a "
+              + "string and used as the key for an insertion into D, with its corresponding "
+              + "value being <code>value</code>.",
     parameters = {
-        @Param(name = "other", type = SkylarkDict.class, doc = "The values to add."),
+        @Param(
+            name = "args",
+            type = Object.class,
+            defaultValue = "[]",
+            doc =
+                "Either a dictionary or a list of entries. Entries must be tuples or lists with "
+                    + "exactly two elements: key, value."),
     },
+    extraKeywords = @Param(name = "kwargs", doc = "Dictionary of additional entries."),
     useLocation = true,
     useEnvironment = true
   )
   public Runtime.NoneType update(
-      SkylarkDict<K, V> other,
+      Object args,
+      SkylarkDict<?, ?> kwargs,
       Location loc,
       Environment env)
       throws EvalException {
-    putAll(other, loc, env.mutability());
+    SkylarkDict<K, V> dict = (args instanceof SkylarkDict) ?
+        (SkylarkDict<K, V>)args: getDictFromArgs(args,loc,env);
+    dict = SkylarkDict.plus(dict, (SkylarkDict<K, V>) kwargs, env);
+    putAll(dict, loc, env.mutability());
     return Runtime.NONE;
   }
 
@@ -489,4 +511,35 @@ public static <K, V> SkylarkDict<K, V> plus(
     result.putAllUnsafe(right);
     return result;
   }
+
+  public static <K, V> SkylarkDict<K, V> getDictFromArgs(
+      Object args, Location loc, @Nullable Environment env) throws EvalException {
+    SkylarkDict<K, V> result = SkylarkDict.of(env);
+    int pos = 0;
+    for (Object element : Type.OBJECT_LIST.convert(args, "parameter args in dict()")) {
+      List<Object> pair = convertToPair(element, pos, loc);
+      result.put((K) pair.get(0), (V) pair.get(1), loc, env);
+      ++pos;
+    }
+    return result;
+  }
+
+  private static List<Object> convertToPair(Object element, int pos, Location loc)
+      throws EvalException {
+    try {
+      List<Object> tuple = Type.OBJECT_LIST.convert(element, "");
+      int numElements = tuple.size();
+      if (numElements != 2) {
+        throw new EvalException(
+            loc,
+            String.format(
+                "item #%d has length %d, but exactly two elements are required",
+                pos, numElements));
+      }
+      return tuple;
+    } catch (ConversionException e) {
+      throw new EvalException(
+          loc, String.format("cannot convert item #%d to a sequence", pos), e);
+    }
+  }
 }

src/test/starlark/testdata/dict.sky

@@ -0,0 +1,72 @@
+# creation
+
+foo = {'a': 1, 'b': [1, 2]}
+bar = dict(a=1, b=[1, 2])
+baz = dict({'a': 1, 'b': [1, 2]})
+
+assert_eq(foo, bar)
+assert_eq(foo, baz)
+
+# get/setdefault
+
+assert_eq(foo.get('a'), 1)
+assert_eq(bar.get('b'), [1, 2])
+assert_eq(baz.get('c'), None)
+assert_eq(baz.setdefault('c', 15), 15)
+assert_eq(baz.setdefault('c'), 15)
+assert_eq(baz.setdefault('c', 20), 15)
+assert_eq(baz.setdefault('d'), None)
+
+# items
+
+assert_eq(foo.items(), [('a', 1), ('b', [1, 2])])
+
+# keys
+
+assert_eq(bar.keys(), ['a', 'b'])
+
+# values
+
+assert_eq(baz.values(), [1, [1, 2], 15, None])
+
+# pop/popitem
+
+assert_eq(baz.pop('d'), None)
+assert_eq(foo.pop('a'), 1)
+assert_eq(bar.popitem(), ('a', 1))
+assert_eq(foo, bar)
+assert_eq(foo.pop('a', 0), 0)
+assert_eq(foo.popitem(), ('b', [1, 2]))
+
+---
+dict().popitem() ### dictionary is empty
+---
+dict(a=2).pop('z') ### KeyError: "z"
+---
+
+# update
+
+foo = dict()
+baz = dict(a=1, b=[1, 2])
+bar = dict(b=[1, 2])
+
+foo.update(baz)
+bar.update(a=1)
+baz.update({'c': 3})
+foo.update([('c', 3)])
+bar['c'] = 3
+quz = dict()
+quz.update(bar.items())
+
+assert_eq(foo, bar)
+assert_eq(foo, baz)
+assert_eq(foo, quz)
+
+# creation with repeated keys
+
+d1 = dict([('a', 1)], a=2)
+d2 = dict({'a': 1}, a=2)
+d3 = dict([('a', 1), ('a', 2)])
+
+assert_eq(d1, d2)
+assert_eq(d1, d3)