diff --git a/guava-tests/test/com/google/common/collect/CollectSpliteratorsTest.java b/guava-tests/test/com/google/common/collect/CollectSpliteratorsTest.java index 7e49123e11bc..a2cbb2c4290e 100644 --- a/guava-tests/test/com/google/common/collect/CollectSpliteratorsTest.java +++ b/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 custom = (a, b) -> a - b; + assertThat(CollectSpliterators.isNaturalOrder(custom)).isFalse(); + } + + public void testIndexedSortedSpliterator_naturalOrderReturnsNull() { + Spliterator spliterator = + CollectSpliterators.indexed( + 3, Spliterator.SORTED, i -> i + 1, Ordering.natural()); + assertThat(spliterator.getComparator()).isNull(); + assertThat(spliterator.hasCharacteristics(Spliterator.SORTED)).isTrue(); + } + + public void testIndexedSortedSpliterator_comparatorNaturalOrderReturnsNull() { + Spliterator spliterator = + CollectSpliterators.indexed(3, Spliterator.SORTED, i -> i + 1, Comparator.naturalOrder()); + assertThat(spliterator.getComparator()).isNull(); + } + + public void testIndexedSortedSpliterator_customComparatorIsPreserved() { + Comparator reverse = Comparator.reverseOrder(); + Spliterator spliterator = + CollectSpliterators.indexed(3, Spliterator.SORTED, i -> 3 - i, reverse); + assertThat(spliterator.getComparator()).isEqualTo(reverse); + } } diff --git a/guava-tests/test/com/google/common/collect/ImmutableSortedSetTest.java b/guava-tests/test/com/google/common/collect/ImmutableSortedSetTest.java index b43a970391f0..5ad89e221b91 100644 --- a/guava-tests/test/com/google/common/collect/ImmutableSortedSetTest.java +++ b/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 set = ImmutableSortedSet.of(1, 2, 3, 4); + assertThat(set.spliterator().getComparator()).isNull(); + } + + public void testSpliteratorGetComparator_comparatorNaturalOrder_returnsNull() { + ImmutableSortedSet set = + ImmutableSortedSet.copyOf(Comparator.naturalOrder(), asList(1, 2, 3, 4)); + assertThat(set.spliterator().getComparator()).isNull(); + } + + public void testSpliteratorGetComparator_customOrdering_returnsComparator() { + Comparator reverse = Comparator.reverseOrder(); + ImmutableSortedSet set = ImmutableSortedSet.copyOf(reverse, asList(1, 2, 3, 4)); + assertThat(set.spliterator().getComparator()).isEqualTo(reverse); + } + + public void testSpliteratorHasSortedCharacteristic() { + ImmutableSortedSet set = ImmutableSortedSet.of(1, 2, 3, 4); + assertThat(set.spliterator().hasCharacteristics(Spliterator.SORTED)).isTrue(); + } + + public void testAsListSpliteratorGetComparator_naturalOrdering_returnsNull() { + ImmutableSortedSet set = ImmutableSortedSet.of(1, 2, 3, 4); + assertThat(set.asList().spliterator().getComparator()).isNull(); + } + + public void testAsListSpliteratorGetComparator_customOrdering_returnsComparator() { + Comparator reverse = Comparator.reverseOrder(); + ImmutableSortedSet set = ImmutableSortedSet.copyOf(reverse, asList(1, 2, 3, 4)); + assertThat(set.asList().spliterator().getComparator()).isEqualTo(reverse); + } + private static SortedSet newTreeSetWithComparator( Comparator comparator, E... elements) { SortedSet set = new TreeSet<>(comparator); diff --git a/guava/src/com/google/common/collect/CollectSpliterators.java b/guava/src/com/google/common/collect/CollectSpliterators.java index 612f19bcffeb..895c90b20ec5 100644 --- a/guava/src/com/google/common/collect/CollectSpliterators.java +++ b/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. + * + *

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 Spliterator indexed( int size, int extraCharacteristics, IntFunction function) { return indexed(size, extraCharacteristics, function, null); @@ -92,7 +106,10 @@ public int characteristics() { @Override public @Nullable Comparator 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(); } diff --git a/guava/src/com/google/common/collect/ImmutableSortedSet.java b/guava/src/com/google/common/collect/ImmutableSortedSet.java index d072644543eb..824a14823517 100644 --- a/guava/src/com/google/common/collect/ImmutableSortedSet.java +++ b/guava/src/com/google/common/collect/ImmutableSortedSet.java @@ -832,8 +832,11 @@ public boolean tryAdvance(Consumer action) { } @Override - public Comparator getComparator() { - return comparator; + public @Nullable Comparator 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; } }; }