Return null from Spliterator.getComparator() for natural ordering (fixes #6187)

guava-tests/test/com/google/common/collect/CollectSpliteratorsTest.java

@@ -23,6 +23,7 @@
 import com.google.common.collect.testing.SpliteratorTester;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Spliterator;
 import java.util.stream.DoubleStream;
@@ -111,4 +112,49 @@ public void testMultisetsSpliterator() {
     multiset.spliterator().forEachRemaining(actualValues::add);
     assertThat(multiset).containsExactly("a", "a", "a", "b", "c", "c").inOrder();
   }
+
+  // Regression tests for https://github.com/google/guava/issues/6187.
+
+  public void testIsNaturalOrder_null() {
+    assertThat(CollectSpliterators.isNaturalOrder(null)).isTrue();
+  }
+
+  public void testIsNaturalOrder_orderingNatural() {
+    assertThat(CollectSpliterators.isNaturalOrder(Ordering.natural())).isTrue();
+  }
+
+  public void testIsNaturalOrder_comparatorNaturalOrder() {
+    assertThat(CollectSpliterators.isNaturalOrder(Comparator.naturalOrder())).isTrue();
+  }
+
+  public void testIsNaturalOrder_reverseOrder_false() {
+    assertThat(CollectSpliterators.isNaturalOrder(Comparator.reverseOrder())).isFalse();
+    assertThat(CollectSpliterators.isNaturalOrder(Ordering.natural().reverse())).isFalse();
+  }
+
+  public void testIsNaturalOrder_customComparator_false() {
+    Comparator<Integer> custom = (a, b) -> a - b;
+    assertThat(CollectSpliterators.isNaturalOrder(custom)).isFalse();
+  }
+
+  public void testIndexedSortedSpliterator_naturalOrderReturnsNull() {
+    Spliterator<Integer> spliterator =
+        CollectSpliterators.indexed(
+            3, Spliterator.SORTED, i -> i + 1, Ordering.<Integer>natural());
+    assertThat(spliterator.getComparator()).isNull();
+    assertThat(spliterator.hasCharacteristics(Spliterator.SORTED)).isTrue();
+  }
+
+  public void testIndexedSortedSpliterator_comparatorNaturalOrderReturnsNull() {
+    Spliterator<Integer> spliterator =
+        CollectSpliterators.indexed(3, Spliterator.SORTED, i -> i + 1, Comparator.naturalOrder());
+    assertThat(spliterator.getComparator()).isNull();
+  }
+
+  public void testIndexedSortedSpliterator_customComparatorIsPreserved() {
+    Comparator<Integer> reverse = Comparator.reverseOrder();
+    Spliterator<Integer> spliterator =
+        CollectSpliterators.indexed(3, Spliterator.SORTED, i -> 3 - i, reverse);
+    assertThat(spliterator.getComparator()).isEqualTo(reverse);
+  }
 }

guava-tests/test/com/google/common/collect/ImmutableSortedSetTest.java

@@ -57,6 +57,7 @@
 import java.util.NoSuchElementException;
 import java.util.Set;
 import java.util.SortedSet;
+import java.util.Spliterator;
 import java.util.TreeSet;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.BiPredicate;
@@ -1219,6 +1220,43 @@ public void testBuilderAsymptotics() {
     // hopefully something quadratic would have more digits
   }
 
+  // Regression tests for https://github.com/google/guava/issues/6187:
+  // Spliterator.getComparator() must return null for naturally-ordered sources so that
+  // stream optimizations (e.g. Stream.sorted() as a no-op) kick in.
+
+  public void testSpliteratorGetComparator_naturalOrdering_returnsNull() {
+    ImmutableSortedSet<Integer> set = ImmutableSortedSet.of(1, 2, 3, 4);
+    assertThat(set.spliterator().getComparator()).isNull();
+  }
+
+  public void testSpliteratorGetComparator_comparatorNaturalOrder_returnsNull() {
+    ImmutableSortedSet<Integer> set =
+        ImmutableSortedSet.copyOf(Comparator.<Integer>naturalOrder(), asList(1, 2, 3, 4));
+    assertThat(set.spliterator().getComparator()).isNull();
+  }
+
+  public void testSpliteratorGetComparator_customOrdering_returnsComparator() {
+    Comparator<Integer> reverse = Comparator.reverseOrder();
+    ImmutableSortedSet<Integer> set = ImmutableSortedSet.copyOf(reverse, asList(1, 2, 3, 4));
+    assertThat(set.spliterator().getComparator()).isEqualTo(reverse);
+  }
+
+  public void testSpliteratorHasSortedCharacteristic() {
+    ImmutableSortedSet<Integer> set = ImmutableSortedSet.of(1, 2, 3, 4);
+    assertThat(set.spliterator().hasCharacteristics(Spliterator.SORTED)).isTrue();
+  }
+
+  public void testAsListSpliteratorGetComparator_naturalOrdering_returnsNull() {
+    ImmutableSortedSet<Integer> set = ImmutableSortedSet.of(1, 2, 3, 4);
+    assertThat(set.asList().spliterator().getComparator()).isNull();
+  }
+
+  public void testAsListSpliteratorGetComparator_customOrdering_returnsComparator() {
+    Comparator<Integer> reverse = Comparator.reverseOrder();
+    ImmutableSortedSet<Integer> set = ImmutableSortedSet.copyOf(reverse, asList(1, 2, 3, 4));
+    assertThat(set.asList().spliterator().getComparator()).isEqualTo(reverse);
+  }
+
   private static <E extends @Nullable Object> SortedSet<E> newTreeSetWithComparator(
       Comparator<? super E> comparator, E... elements) {
     SortedSet<E> set = new TreeSet<>(comparator);

guava/src/com/google/common/collect/CollectSpliterators.java

@@ -40,6 +40,20 @@
 final class CollectSpliterators {
   private CollectSpliterators() {}
 
+  /**
+   * Returns whether {@code comparator} represents the natural ordering of its elements.
+   *
+   * <p>Recognizes the two well-known natural-ordering singletons: {@link Ordering#natural()} and
+   * {@link Comparator#naturalOrder()}.  Any comparator that compares equal to one of these (via
+   * {@link Object#equals}) is treated as natural order so that {@link Spliterator#getComparator()}
+   * can return {@code null} as required by its contract.
+   */
+  static boolean isNaturalOrder(@Nullable Comparator<?> comparator) {
+    return comparator == null
+        || Ordering.natural().equals(comparator)
+        || Comparator.naturalOrder().equals(comparator);
+  }
+
   static <T extends @Nullable Object> Spliterator<T> indexed(
       int size, int extraCharacteristics, IntFunction<T> function) {
     return indexed(size, extraCharacteristics, function, null);
@@ -92,7 +106,10 @@ public int characteristics() {
       @Override
       public @Nullable Comparator<? super T> getComparator() {
         if (hasCharacteristics(Spliterator.SORTED)) {
-          return comparator;
+          // Per Spliterator.getComparator()'s contract, null signals that the source is
+          // sorted in natural order.  Normalizing natural-ordering singletons to null allows
+          // Stream.sorted() to short-circuit instead of re-sorting already-sorted data.
+          return isNaturalOrder(comparator) ? null : comparator;
         } else {
           throw new IllegalStateException();
         }

guava/src/com/google/common/collect/ImmutableSortedSet.java

@@ -832,8 +832,11 @@ public boolean tryAdvance(Consumer<? super E> action) {
       }
 
       @Override
-      public Comparator<? super E> getComparator() {
-        return comparator;
+      public @Nullable Comparator<? super E> getComparator() {
+        // Per Spliterator.getComparator()'s contract, null signals that the source is sorted in
+        // natural order.  Normalizing natural-ordering singletons to null allows Stream.sorted()
+        // to short-circuit instead of re-sorting already-sorted data.
+        return CollectSpliterators.isNaturalOrder(comparator) ? null : comparator;
       }
     };
   }