From 843e72629522336e2078eea732d074b4e15c8de2 Mon Sep 17 00:00:00 2001 From: Nickita Khylkouski <90287684+nickita-khylkouski@users.noreply.github.com> Date: Mon, 26 Jan 2026 11:01:51 -0800 Subject: [PATCH] collect: Return null from Spliterator.getComparator() for natural ordering Per the Spliterator.getComparator() contract, return null to indicate natural ordering. This enables stream optimizations where sorted() becomes a no-op for already-sorted collections using natural ordering. Previously, ImmutableSortedSet and CollectSpliterators returned Ordering.natural() instead of null, which caused the JDK stream implementation to clear the SORTED flag and re-sort unnecessarily. The fix checks for both Ordering.natural() and Comparator.naturalOrder() using reference equality (both are singletons). Fixes #6187 --- .../collect/ImmutableSortedSetTest.java | 30 +++++++++++++++++++ .../common/collect/CollectSpliterators.java | 7 ++++- .../common/collect/ImmutableSortedSet.java | 3 ++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/guava-tests/test/com/google/common/collect/ImmutableSortedSetTest.java b/guava-tests/test/com/google/common/collect/ImmutableSortedSetTest.java index 3b8000644bf7..7d5540d9a6f5 100644 --- a/guava-tests/test/com/google/common/collect/ImmutableSortedSetTest.java +++ b/guava-tests/test/com/google/common/collect/ImmutableSortedSetTest.java @@ -55,6 +55,7 @@ import java.util.NoSuchElementException; import java.util.Set; import java.util.SortedSet; +import java.util.Spliterator; import java.util.TreeSet; import java.util.function.BiPredicate; import java.util.stream.Collector; @@ -1225,4 +1226,33 @@ public void testBuilderAsymptotics() { assertThat(compares[0]).isAtMost(10000); // hopefully something quadratic would have more digits } + + @GwtIncompatible // Spliterator + public void testSpliteratorComparator_naturalOrdering() { + // Per Spliterator.getComparator() contract, null indicates natural ordering + ImmutableSortedSet set = ImmutableSortedSet.of(1, 2, 3); + Spliterator spliterator = set.spliterator(); + assertThat(spliterator.hasCharacteristics(Spliterator.SORTED)).isTrue(); + assertThat(spliterator.getComparator()).isNull(); + } + + @GwtIncompatible // Spliterator + public void testSpliteratorComparator_customComparator() { + // Custom comparator should be returned from getComparator() + Comparator comparator = Comparator.reverseOrder(); + ImmutableSortedSet set = + ImmutableSortedSet.orderedBy(comparator).add(1, 2, 3).build(); + Spliterator spliterator = set.spliterator(); + assertThat(spliterator.hasCharacteristics(Spliterator.SORTED)).isTrue(); + assertThat(spliterator.getComparator()).isEqualTo(comparator); + } + + @GwtIncompatible // Spliterator + public void testAsListSpliteratorComparator_naturalOrdering() { + // asList().spliterator() should also return null for natural ordering + ImmutableSortedSet set = ImmutableSortedSet.of(1, 2, 3); + Spliterator spliterator = set.asList().spliterator(); + assertThat(spliterator.hasCharacteristics(Spliterator.SORTED)).isTrue(); + assertThat(spliterator.getComparator()).isNull(); + } } diff --git a/guava/src/com/google/common/collect/CollectSpliterators.java b/guava/src/com/google/common/collect/CollectSpliterators.java index 612f19bcffeb..0e484a230618 100644 --- a/guava/src/com/google/common/collect/CollectSpliterators.java +++ b/guava/src/com/google/common/collect/CollectSpliterators.java @@ -53,6 +53,11 @@ private CollectSpliterators() {} if (comparator != null) { checkArgument((extraCharacteristics & Spliterator.SORTED) != 0); } + // Per Spliterator.getComparator() contract, return null for natural ordering + @Nullable Comparator spliteratorComparator = + (comparator == Ordering.natural() || comparator == Comparator.naturalOrder()) + ? null + : comparator; final class WithCharacteristics implements Spliterator { private final Spliterator.OfInt delegate; @@ -92,7 +97,7 @@ public int characteristics() { @Override public @Nullable Comparator getComparator() { if (hasCharacteristics(Spliterator.SORTED)) { - return comparator; + return spliteratorComparator; } else { throw new IllegalStateException(); } diff --git a/guava/src/com/google/common/collect/ImmutableSortedSet.java b/guava/src/com/google/common/collect/ImmutableSortedSet.java index d072644543eb..c6d4e14e736c 100644 --- a/guava/src/com/google/common/collect/ImmutableSortedSet.java +++ b/guava/src/com/google/common/collect/ImmutableSortedSet.java @@ -833,6 +833,9 @@ public boolean tryAdvance(Consumer action) { @Override public Comparator getComparator() { + if (comparator == Ordering.natural() || comparator == Comparator.naturalOrder()) { + return null; + } return comparator; } };