Add key and reverse parameters to builtin sorted function

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

@@ -49,6 +49,16 @@ public ComparisonException(String msg) {
     }
   }
 
+  public static class SortPair {
+    public Object key;
+    public Object value;
+
+    public SortPair(Object key, Object value){
+      this.key = key;
+      this.value = value;
+    }
+  }
+
   /**
    * Compare two Skylark objects.
    *
@@ -77,6 +87,10 @@ public int compare(Object o1, Object o2) {
           o1 = SkylarkType.convertToSkylark(o1, (Environment) null);
           o2 = SkylarkType.convertToSkylark(o2, (Environment) null);
 
+          if (o1 instanceof SortPair && o2 instanceof SortPair) {
+            o1 = ((SortPair) o1).key;
+            o2 = ((SortPair) o2).key;
+          }
           if (o1 instanceof SkylarkList
               && o2 instanceof SkylarkList
               && ((SkylarkList) o1).isTuple() == ((SkylarkList) o2).isTuple()) {

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

@@ -42,6 +42,9 @@
 import java.util.NoSuchElementException;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.HashMap;
+import java.util.Arrays;
+import java.util.Collections;
 import javax.annotation.Nullable;
 
 /** A helper class containing built in functions for the Skylark language. */
@@ -102,6 +105,19 @@ private static Object findExtreme(
     }
   }
 
+  private static Object evalKey(Object obj, Object key, Location loc, Environment env) throws EvalException, InterruptedException {
+    if (key == Runtime.NONE) {
+      return obj;
+    } else if (key instanceof BuiltinCallable) {
+      return ((BuiltinCallable) key).call(new ArrayList<>(Arrays.asList(obj)),
+              new HashMap<>(), new FuncallExpression(Identifier.of(""), ImmutableList.of()), env);
+    } else if (key instanceof BaseFunction) {
+      return ((BaseFunction) key).call(new ArrayList<>(Arrays.asList(obj)), null, null, env);
+    }
+
+    throw new EvalException(loc, Printer.format("%r object is not callable", EvalUtils.getDataTypeName(key)));
+  }
+
   @SkylarkCallable(
       name = "all",
       doc =
@@ -170,14 +186,45 @@ private static boolean hasElementWithBooleanValue(
             type = Object.class,
             doc = "This collection.",
             // TODO(cparsons): This parameter should be positional-only.
-            legacyNamed = true)
+            legacyNamed = true),
+        @Param(
+            name = "key",
+            doc = "The key to sort with.",
+            named = true,
+            defaultValue = "None",
+            positional = false,
+            noneable = true),
+        @Param(
+            name = "reverse",
+            type = Boolean.class,
+            doc = "Whether to sort asc or desc.",
+            named = true,
+            defaultValue = "False",
+            positional = false,
+            noneable = true)
       },
       useLocation = true,
       useEnvironment = true)
-  public MutableList<?> sorted(Object self, Location loc, Environment env) throws EvalException {
+  public MutableList<?> sorted(Object self, Object key, Boolean reverse,
+      Location loc, Environment env) throws EvalException, InterruptedException {
+    Iterator<?> iterator = EvalUtils.toCollection(self, loc, env).iterator();
+    List<Object> list = new ArrayList<>();
+
+    while (iterator.hasNext()) {
+      Object val = iterator.next();
+      list.add(new EvalUtils.SortPair(evalKey(val, key, loc, env), val));
+    }
+
     try {
-      return MutableList.copyOf(
-          env, EvalUtils.SKYLARK_COMPARATOR.sortedCopy(EvalUtils.toCollection(self, loc, env)));
+      list = EvalUtils.SKYLARK_COMPARATOR.sortedCopy(list);
+      for (int i = 0; i < list.size(); i++) {
+        if (reverse && i < list.size() / 2) {
+          Collections.swap(list, i, list.size() - i - 1);
+        }
+        list.set(i, ((EvalUtils.SortPair) list.get(i)).value);
+
+      }
+      return MutableList.copyOf(env, list);
     } catch (EvalUtils.ComparisonException e) {
       throw new EvalException(loc, e);
     }

src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java

@@ -249,6 +249,9 @@ public void testListSort() throws Exception {
         .testEval("sorted([True, False, True])", "[False, True, True]")
         .testEval("sorted(['a','x','b','z'])", "[\"a\", \"b\", \"x\", \"z\"]")
         .testEval("sorted({1: True, 5: True, 4: False})", "[1, 4, 5]")
+        .testEval("sorted([3, 2, 1, 0], reverse=True)", "[3, 2, 1, 0]")
+        .testEval("sorted([[1], [], [1, 2]], key=len, reverse=True)", "[[1, 2], [1], []]")
+        .testEval("sorted([[0, 5], [4, 1], [1, 7]], key=max)", "[[4, 1], [0, 5], [1, 7]]")
         .testIfExactError("Cannot compare function with function", "sorted([sorted, sorted])");
   }
 
@@ -712,7 +715,7 @@ public void testLegacyNamed() throws Exception {
         // Parameters which may be specified by keyword but are not explicitly 'named'.
         .testStatement("all(elements=[True, True])", Boolean.TRUE)
         .testStatement("any(elements=[True, False])", Boolean.TRUE)
-        .testEval("sorted(self=[3, 0, 2])", "[0, 2, 3]")
+        .testEval("sorted(self=[3, 0, 2], key=None, reverse=False)", "[0, 2, 3]")
         .testEval("reversed(sequence=[3, 2, 0])", "[0, 2, 3]")
         .testEval("tuple(x=[1, 2])", "(1, 2)")
         .testEval("list(x=(1, 2))", "[1, 2]")

src/test/starlark/testdata/sorted.sky

@@ -0,0 +1,37 @@
+assert_eq(sorted([42, 123, 3]), [3, 42, 123])
+assert_eq(sorted(["wiz", "foo", "bar"]), ["bar", "foo", "wiz"])
+assert_eq(sorted([42, 123, 3], reverse=True), [123, 42, 3])
+assert_eq(sorted(["wiz", "foo", "bar"], reverse=True), ["wiz", "foo", "bar"])
+assert_eq(sorted(list({"a": 1, "b": 2})), ['a', 'b'])
+---
+
+def f(x):
+  return x[0]
+pairs = [(4, 0), (3, 1), (4, 2), (2, 3), (3, 4), (1, 5), (2, 6), (3, 7)]
+
+assert_eq(sorted(pairs, key=f),
+         [(1, 5),
+          (2, 3), (2, 6),
+          (3, 1), (3, 4), (3, 7),
+          (4, 0), (4, 2)])
+
+---
+assert_eq(sorted(["two", "three", "four"], key=len),
+         ["two", "four", "three"])
+assert_eq(sorted(["two", "three", "four"], key=len, reverse=True),
+         ["three", "four", "two"])
+assert_eq(sorted([[1, 5], [0, 10], [4]], key=max),
+         [[4], [1, 5], [0, 10]])
+assert_eq(sorted([[1, 5], [0, 10], [4]], key=min, reverse=True),
+         [[4], [1, 5], [0, 10]])
+assert_eq(sorted([[2, 6, 1], [5, 2, 1], [1, 4, 2]], key=sorted),
+         [[1, 4, 2], [5, 2, 1], [2, 6, 1]])
+
+---
+sorted(1) ### type 'int' is not a collection
+---
+sorted([1, 2, None, 3]) ### Cannot compare NoneType with int
+---
+sorted([1, "one"]) ### Cannot compare string with int
+---
+sorted([1, 2, 3], key=1) ### "int" object is not callable