Change null handling in compareValuesBy(a, b, functions). Provide nullsFirst() and nullsLast() to extend Comparator<T> to Comparator<T?>.

libraries/stdlib/src/kotlin/util/Ordering.kt

@@ -17,6 +17,7 @@
 package kotlin
 
 import java.util.Comparator
+import kotlin.platform.platformName
 
 /**
  * Compares two values using the specified sequence of functions to calculate the result of the comparison.
@@ -24,6 +25,7 @@ import java.util.Comparator
  * objects. As soon as the [Comparable] instances returned by a function for [a] and [b] values do not
  * compare as equal, the result of that comparison is returned.
  */
+deprecated("Use selector functions accepting nullable T as a receiver.")
 public fun <T : Any> compareValuesBy(a: T?, b: T?, vararg functions: (T) -> Comparable<*>?): Int {
     require(functions.size() > 0)
     if (a === b) return 0
@@ -38,15 +40,31 @@ public fun <T : Any> compareValuesBy(a: T?, b: T?, vararg functions: (T) -> Comp
     return 0
 }
 
+/**
+ * Compares two values using the specified sequence of functions to calculate the result of the comparison.
+ * The functions are called sequentially, receive the given values [a] and [b] and return [Comparable]
+ * objects. As soon as the [Comparable] instances returned by a function for [a] and [b] values do not
+ * compare as equal, the result of that comparison is returned.
+ */
+// TODO: Remove platformName after M13
+platformName("compareValuesByNullable")
+public fun <T> compareValuesBy(a: T, b: T, vararg selectors: (T) -> Comparable<*>?): Int {
+    require(selectors.size() > 0)
+    for (fn in selectors) {
+        val v1 = fn(a)
+        val v2 = fn(b)
+        val diff = compareValues(v1, v2)
+        if (diff != 0) return diff
+    }
+    return 0
+}
+
 /**
  * Compares two values using the specified [selector] function to calculate the result of the comparison.
  * The function is applied to the given values [a] and [b] and return [Comparable] objects.
  * The result of comparison of these [Comparable] instances is returned.
  */
-public inline fun <T: Any> compareValuesBy(a: T?, b: T?, selector: (T) -> Comparable<*>?): Int {
-    if (a === b) return 0
-    if (a == null) return -1
-    if (b == null) return 1
+public inline fun <T> compareValuesBy(a: T, b: T, selector: (T) -> Comparable<*>?): Int {
     return compareValues(selector(a), selector(b))
 }
 
@@ -150,4 +168,35 @@ inline public fun <T> Comparator<T>.thenComparator(inlineOptions(InlineOption.ON
             return if (previousCompare != 0) previousCompare else comparison(a, b)
         }
     }
+}
+
+// Not so useful without type inference for receiver of expression
+/**
+ * Extends the given comparator of non-nullable values to a comparator of nullable values
+ * considering null value less than any other value.
+ */
+public fun <T: Any> Comparator<T>.nullsFirst(): Comparator<T?> {
+    return object: Comparator<T?> {
+        override fun compare(a: T?, b: T?): Int {
+            if (a === b) return 0
+            if (a == null) return -1
+            if (b == null) return 1
+            return this@nullsFirst.compare(a, b)
+        }
+    }
+}
+
+/**
+ * Extends the given comparator of non-nullable values to a comparator of nullable values
+ * considering null value greater than any other value.
+ */
+public fun <T: Any> Comparator<T>.nullsLast(): Comparator<T?> {
+    return object: Comparator<T?> {
+        override fun compare(a: T?, b: T?): Int {
+            if (a === b) return 0
+            if (a == null) return 1
+            if (b == null) return -1
+            return this@nullsLast.compare(a, b)
+        }
+    }
 }

libraries/stdlib/test/OrderingTest.kt

@@ -34,12 +34,21 @@ class OrderingTest {
         assertTrue(diff == 0)
     }
 
+    Test fun compareNullables() {
+        val v1: Item? = this.v1
+        val v2: Item? = null
+        val diff = compareValuesBy(v1, v2) { it?.rating }
+        assertTrue(diff > 0)
+        val diff2 = compareBy<Item> { it.rating }.thenBy { it.name }.nullsLast().compare(v1, v2)
+        assertTrue(diff2 < 0)
+    }
+
     Test fun sortComparatorThenComparator() {
         val comparator = comparator<Item> { a, b -> a.name.compareTo(b.name) } thenComparator { a, b -> a.rating.compareTo(b.rating) }
 
         val diff = comparator.compare(v1, v2)
         assertTrue(diff > 0)
-        val items = arrayListOf(v1, v2).sortBy(comparator)
+        val items = arrayListOf(v1, v2).sortedWith(comparator)
         assertEquals(v2, items[0])
         assertEquals(v1, items[1])
     }
@@ -49,7 +58,7 @@ class OrderingTest {
 
         val diff = comparator.compare(v1, v2)
         assertTrue(diff < 0)
-        val items = arrayListOf(v1, v2).sortBy(comparator)
+        val items = arrayListOf(v1, v2).sortedWith(comparator)
         assertEquals(v1, items[0])
         assertEquals(v2, items[1])
     }
@@ -59,7 +68,7 @@ class OrderingTest {
 
         val diff = comparator.compare(v1, v2)
         assertTrue(diff < 0)
-        val items = arrayListOf(v1, v2).sortBy(comparator)
+        val items = arrayListOf(v1, v2).sortedWith(comparator)
         assertEquals(v1, items[0])
         assertEquals(v2, items[1])
     }
@@ -68,7 +77,7 @@ class OrderingTest {
         val comparator = compareBy<Item>({ it.name }, { it.rating })
         val diff = comparator.compare(v1, v2)
         assertTrue(diff > 0)
-        val items = arrayListOf(v1, v2).sortBy(comparator)
+        val items = arrayListOf(v1, v2).sortedWith(comparator)
         assertEquals(v2, items[0])
         assertEquals(v1, items[1])
     }
@@ -85,7 +94,7 @@ class OrderingTest {
         }
         val diff = comparator.compare(v1, v2)
         assertTrue(diff > 0)
-        val items = arrayListOf(v1, v2).sortBy(comparator)
+        val items = arrayListOf(v1, v2).sortedWith(comparator)
         assertEquals(v2, items[0])
         assertEquals(v1, items[1])
     }

libraries/stdlib/test/collections/ArraysTest.kt

@@ -827,9 +827,9 @@ class ArraysTest {
     test fun sortedNullableBy() {
         fun String.nullIfEmpty() = if (isEmpty()) null else this
         arrayOf(null, "").let {
-            expect(listOf(null, "")) { it.sortedBy { it.orEmpty()}}
-            expect(listOf("", null)) { it.sortedByDescending { it.orEmpty() }}
-            expect(listOf("", null)) { it.sortedByDescending { it?.nullIfEmpty() }}
+            expect(listOf(null, "")) { it.sortedWith(compareBy<String> { it }.nullsFirst()) }
+            expect(listOf("", null)) { it.sortedWith(compareByDescending<String> { it }.nullsLast()) }
+            expect(listOf("", null)) { it.sortedWith(compareByDescending<String> { it.nullIfEmpty() }.nullsLast() ) }
         }
     }
 

libraries/stdlib/test/collections/CollectionTest.kt

@@ -573,9 +573,12 @@ class CollectionTest {
     test fun sortedNullableBy() {
         fun String.nullIfEmpty() = if (isEmpty()) null else this
         listOf(null, "").let {
-            expect(listOf(null, "")) { it.sortedBy { it.orEmpty()}}
-            expect(listOf("", null)) { it.sortedByDescending { it.orEmpty() }}
-            expect(listOf("", null)) { it.sortedByDescending { it?.nullIfEmpty() }}
+//            expect(listOf(null, "")) { it.sortedBy { it.orEmpty()}}
+//            expect(listOf("", null)) { it.sortedByDescending { it.orEmpty() }}
+//            expect(listOf("", null)) { it.sortedByDescending { it?.nullIfEmpty() }}
+            expect(listOf(null, "")) { it.sortedWith(compareBy<String> { it }.nullsFirst()) }
+            expect(listOf("", null)) { it.sortedWith(compareByDescending<String> { it }.nullsLast()) }
+            expect(listOf("", null)) { it.sortedWith(compareByDescending<String> { it.nullIfEmpty() }.nullsLast() ) }
         }
     }