Support sorting of maps and boolean comparison fns. (#711)

Hi, 

could you please review fix for `sort` to support maps and boolean
comparator fns. This completes #708.

I've also made some changes to the comments of the affected fns to
indicate that we are now using the `compare` fn as default, but feel
free to correct.

Tests for the same were also added.

Thanks

---------

Co-authored-by: ikappaki <[email protected]>

Author: ikappaki

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  * Fix issue with relative paths dropping their first character on MS-Windows (#703).
  * Fix incompatibility with `(str nil)` returning "nil" (#706).
  * Fix `sort-by` support for maps and boolean comparator fns (#709).
+ * Fix `sort` support for maps and boolean comparator fns (#711).
 
 ## [v0.1.0a2]
 ### Added

src/basilisp/core.lpy

@@ -1158,11 +1158,9 @@
   (basilisp.lang.runtime/compare x y))
 
 (defn sort
-  "Return a sorted sequence of the elements from ``coll``\\.
+  "Return a sorted sequence of the elements from ``coll`` using the ``cmp`` comparator\\.
 
-  Unlike in Clojure, this function does not use :lpy:fn:`compare` directly. Instead,
-  the heuristics that ``compare`` uses to produce three-way comparator return values
-  are used to guide sorting."
+  The :lpy:fn:`compare` fn is used in if ``cmp`` is not provided."
   ([coll]
    (basilisp.lang.runtime/sort coll))
   ([cmp coll]
@@ -3226,7 +3224,9 @@
           (rest args)))
 
 (defn sort-by
-  "Return a sorted sequence of the elements from ``coll``\\."
+  "Return a sorted sequence of the elements from ``coll`` using the ``cmp`` comparator on (``keyfn`` item)\\.
+
+  The :lpy:fn:`compare` fn is used in if ``cmp`` is not provided."
   ([keyfn coll]
    (basilisp.lang.runtime/sort-by keyfn coll))
   ([keyfn cmp coll]

src/basilisp/lang/runtime.py

@@ -1373,14 +1373,6 @@ def _compare_sets(x: IPersistentSet, y) -> int:
     )
 
 
-def sort(coll, f=None) -> Optional[ISeq]:
-    """Return a sorted sequence of the elements in coll. If a comparator
-    function f is provided, compare elements in coll using f."""
-    if isinstance(coll, IPersistentMap):
-        coll = lseq.to_seq(coll)
-    return lseq.sequence(sorted(coll, key=Maybe(f).map(functools.cmp_to_key).value))
-
-
 def _fn_to_comparator(f):
     """Coerce F comparator fn to a 3 way comparator fn."""
 
@@ -1401,6 +1393,43 @@ def cmp(x, y):
     return cmp
 
 
+def sort(coll, f=compare) -> Optional[ISeq]:
+    """Return a sorted sequence of the elements in coll. If a
+    comparator function f is provided, compare elements in coll
+    using f or use the `compare` fn if not.
+
+    The comparator fn can be either a boolean or 3-way comparison fn."""
+    if isinstance(coll, IPersistentMap):
+        coll = lseq.to_seq(coll)
+
+    comparator = _fn_to_comparator(f)
+
+    class key:
+        __slots__ = ("obj",)
+
+        def __init__(self, obj):
+            self.obj = obj
+
+        def __lt__(self, other):
+            return comparator(self.obj, other.obj) < 0
+
+        def __gt__(self, other):
+            return comparator(self.obj, other.obj) > 0
+
+        def __eq__(self, other):
+            return comparator(self.obj, other.obj) == 0
+
+        def __le__(self, other):
+            return comparator(self.obj, other.obj) <= 0
+
+        def __ge__(self, other):
+            return comparator(self.obj, other.obj) >= 0
+
+        __hash__ = None  # type: ignore
+
+    return lseq.sequence(sorted(coll, key=key))
+
+
 def sort_by(keyfn, coll, cmp=compare) -> Optional[ISeq]:
     """Return a sorted sequence of the elements in coll. If a
     comparator function cmp is provided, compare elements in coll

tests/basilisp/test_core_fns.lpy

@@ -970,6 +970,32 @@
   (is (= "dsd" (min-key count "asd" "bsd" "dsd" "long word")))
   (is (= "a" (min-key count "a" "bsd" "dsd" "long word"))))
 
+(deftest sort-test
+  (testing "no cmp function"
+    (is (= '() (sort [])))
+    (is (= '(1 2 3 4 5) (sort [5 3 4 1 2])))
+    (is (= '([:a] [5 5] [1 2 3])
+           (sort [[1 2 3] [:a] [5 5]]))))
+
+  (testing "with cmp function"
+    (let [cmp (fn [v1 v2] (< v2 v1))]
+      (is (= '() (sort cmp [])))
+      (is (= '([1 2 3] [5 5] [:a])
+             (sort cmp [[1 2 3] [:a] [5 5]]))))
+
+    ;; taken from clojuredocs
+    (is (= '(:d :c :b :a) (sort #(compare %2 %1) '(:a :b :c :d)))))
+
+  (testing "sorting vectors"
+    (are [res v] (= res (sort v))
+      '([1] [3] [1 2]) [[1] [1 2] [3]]
+      '([1] [1 2] [1 3]) [[1 3] [1] [1 2]]
+      '([1] [0 1] [0 1]) [[0 1] [1] [0 1] ]))
+
+  (testing "sorting maps"
+    (are [res v] (= res (sort v))
+      '([:3 18] [:5 28] [:9 23]) {:9 23 :3 18 :5 28})))
+
 (deftest sort-by-test
   (testing "no cmp function"
     (is (= '() (sort-by count [])))