Implemented copy constructor for Skylark dictionaries: new_dict = dict(old_dict)
--
MOS_MIGRATED_REVID=103932279
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
index 2afcbc3..cb33724 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -32,8 +32,8 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
-import java.util.HashMap;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
@@ -1186,37 +1186,52 @@
+ "will overwrite values from the positional argument if a key appears multiple times. "
+ "Dictionaries are always sorted by their keys",
optionalPositionals = {
- @Param(name = "args", type = Iterable.class, defaultValue = "[]",
+ @Param(name = "args", type = Object.class, defaultValue = "[]",
doc =
- "List of entries. Entries must be tuples or lists with exactly "
- + "two elements: key, value"),
+ "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)
private static final BuiltinFunction dict = new BuiltinFunction("dict") {
@SuppressWarnings("unused")
- public Map<Object, Object> invoke(Iterable<Object> args, Map<Object, Object> kwargs,
- Location loc) throws EvalException, ConversionException {
+ public Map<Object, Object> invoke(Object args, Map<Object, Object> kwargs, Location loc)
+ throws EvalException {
+ Map<Object, Object> result =
+ (args instanceof Map<?, ?>)
+ ? new LinkedHashMap<>((Map<?, ?>) args) : getMapFromArgs(args, loc);
+ result.putAll(kwargs);
+ return result;
+ }
+
+ private Map<Object, Object> getMapFromArgs(Object args, Location loc) throws EvalException {
+ Map<Object, Object> result = new LinkedHashMap<>();
+ 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));
+ ++pos;
+ }
+ return result;
+ }
+
+ private List<Object> convertToPair(Object element, int pos, Location loc)
+ throws EvalException {
try {
- Map<Object, Object> result = new HashMap<>();
- List<Object> list = Type.OBJECT_LIST.convert(args, "dict(args)");
-
- for (Object tuple : list) {
- List<Object> mapping = Type.OBJECT_LIST.convert(tuple, "dict(args)");
- int numElements = mapping.size();
-
- if (numElements != 2) {
- throw new EvalException(
- location,
- String.format(
- "Tuple has length %d, but exactly two elements are required", numElements));
- }
- result.put(mapping.get(0), mapping.get(1));
+ List<Object> tuple = Type.OBJECT_LIST.convert(element, "");
+ int numElements = tuple.size();
+ if (numElements != 2) {
+ throw new EvalException(
+ location,
+ String.format("Sequence #%d has length %d, but exactly two elements are required",
+ pos, numElements));
}
- result.putAll(kwargs);
- return result;
- } catch (IllegalArgumentException | ClassCastException | NullPointerException ex) {
- throw new EvalException(loc, ex);
+ return tuple;
+ } catch (ConversionException e) {
+ throw new EvalException(
+ loc,
+ String.format(
+ "Cannot convert dictionary update sequence element #%d to a sequence", pos));
}
}
};
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
index 72fd205..de92681 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
@@ -123,8 +123,8 @@
+ "but should be string",
"'test'.startswith(1)")
.testIfErrorContains(
- "Method dict(args: Iterable, **kwargs) is not applicable for arguments "
- + "(string, dict): 'args' is string, but should be Iterable",
+ "expected value of type 'list(object)' for parameter args in dict(), "
+ + "but got \"a\" (string)",
"dict('a')");
}
@@ -647,6 +647,20 @@
}
@Test
+ public void testDictionaryCopy() throws Exception {
+ new SkylarkTest()
+ .setUp("x = {1 : 2}", "y = dict(x)")
+ .testEval("x[1] == 2 and y[1] == 2", "True");
+ }
+
+ @Test
+ public void testDictionaryCopyKeyCollision() throws Exception {
+ new SkylarkTest()
+ .setUp("x = {'test' : 2}", "y = dict(x, test = 3)")
+ .testEval("y['test']", "3");
+ }
+
+ @Test
public void testDictionaryWithMultipleKeys() throws Exception {
new BothModesTest().testStatement("{0: 'a', 1: 'b', 0: 'c'}[0]", "c");
}
@@ -741,19 +755,18 @@
@Test
public void testDictionaryCreationInvalidPositional() throws Exception {
- String unexpectedString =
- "expected value of type 'list(object)' for dict(args), but got \"a\" (string)";
-
new BothModesTest()
.testIfErrorContains(
- "Method dict(args: Iterable, **kwargs) is not applicable for arguments "
- + "(string, dict): 'args' is string, but should be Iterable",
+ "expected value of type 'list(object)' for parameter args in dict(), "
+ + "but got \"a\" (string)",
"dict('a')")
- .testIfErrorContains(unexpectedString, "dict(['a'])")
- .testIfErrorContains(unexpectedString, "dict([('a')])")
+ .testIfErrorContains(
+ "Cannot convert dictionary update sequence element #0 to a sequence", "dict(['a'])")
+ .testIfErrorContains(
+ "Cannot convert dictionary update sequence element #0 to a sequence", "dict([('a')])")
.testIfErrorContains("too many (3) positional arguments", "dict((3,4), (3,2), (1,2))")
.testIfErrorContains(
- "Tuple has length 3, but exactly two elements are required",
+ "Sequence #0 has length 3, but exactly two elements are required",
"dict([('a', 'b', 'c')])");
}
