Do not sort dict keys when printing
Dict keys can in theory belong to different types, it's not allowed anymore to
compare such objects by < in Skylark, so the Printer class shouldn't do it
either.
--
PiperOrigin-RevId: 147827109
MOS_MIGRATED_REVID=147827109
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
index 1580fb4..75b9a6b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
@@ -98,34 +98,6 @@
};
/**
- * Legacy Skylark comparator.
- *
- * <p>Falls back to comparing by class if objects are not comparable otherwise.
- */
- public static final Ordering<Object> SAFE_SKYLARK_COMPARATOR =
- new Ordering<Object>() {
- @Override
- @SuppressWarnings("unchecked")
- public int compare(Object o1, Object o2) {
- try {
- return SKYLARK_COMPARATOR.compare(o1, o2);
- } catch (ComparisonException e) {
- return compareByClass(o1, o2);
- }
- }
- };
-
- public static final int compareByClass(Object o1, Object o2) {
- try {
- // Different types -> let the class names decide
- return o1.getClass().getName().compareTo(o2.getClass().getName());
- } catch (NullPointerException ex) {
- throw new ComparisonException(
- "Cannot compare " + getDataTypeName(o1) + " with " + EvalUtils.getDataTypeName(o2));
- }
- }
-
- /**
* Checks that an Object is a valid key for a Skylark dict.
* @param o an Object to validate
* @throws EvalException if o is not a valid key
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Printer.java b/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
index 730a830..c0e6f22 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
@@ -26,9 +26,6 @@
import java.util.List;
import java.util.Map;
import java.util.MissingFormatWidthException;
-import java.util.Set;
-import java.util.SortedMap;
-import java.util.TreeMap;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -145,7 +142,7 @@
} else if (o instanceof Map<?, ?>) {
Map<?, ?> dict = (Map<?, ?>) o;
- printList(buffer, getSortedEntrySet(dict), "{", ", ", "}", null, quotationMark);
+ printList(buffer, dict.entrySet(), "{", ", ", "}", null, quotationMark);
} else if (o instanceof Map.Entry<?, ?>) {
Map.Entry<?, ?> entry = (Map.Entry<?, ?>) o;
@@ -166,21 +163,6 @@
return buffer;
}
- /**
- * Returns the sorted entry set of the given map
- */
- private static <K, V> Set<Map.Entry<K, V>> getSortedEntrySet(Map<K, V> dict) {
- if (!(dict instanceof SortedMap<?, ?>)) {
- // TODO(bazel-team): Dict keys should not be sorted, because comparison of objects of
- // potentially different types is not supported anymore in Skylark.
- Map<K, V> tmp = new TreeMap<>(EvalUtils.SAFE_SKYLARK_COMPARATOR);
- tmp.putAll(dict);
- dict = tmp;
- }
-
- return dict.entrySet();
- }
-
public static Appendable write(Appendable buffer, Object o) {
return write(buffer, o, SKYLARK_QUOTATION_MARK);
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java b/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
index 2807b1b..d2a427c 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
@@ -25,12 +25,12 @@
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
+import java.util.LinkedHashMap;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import java.util.Arrays;
-import java.util.HashMap;
import java.util.IllegalFormatException;
import java.util.LinkedList;
import java.util.List;
@@ -94,14 +94,13 @@
}
@Test
- public void testSortedOutputOfUnsortedMap() throws Exception {
- Map<Integer, Integer> map = new HashMap<>();
- int[] data = {5, 7, 3};
-
- for (int current : data) {
- map.put(current, current);
- }
- assertThat(Printer.str(map)).isEqualTo("{3: 3, 5: 5, 7: 7}");
+ public void testOutputOrderOfMap() throws Exception {
+ Map<Object, Object> map = new LinkedHashMap<>();
+ map.put(5, 5);
+ map.put(3, 3);
+ map.put("foo", 42);
+ map.put(7, "bar");
+ assertThat(Printer.str(map)).isEqualTo("{5: 5, 3: 3, \"foo\": 42, 7: \"bar\"}");
}
@Test
