OpenJDK / portola / portola
changeset 48250:c21740de9431
8171826: Comparator.reverseOrder(c) mishandles singleton comparators
Reviewed-by: rriggs
author | psandoz |
---|---|
date | Tue, 12 Dec 2017 09:33:35 -0800 |
parents | 55b9b1e184c6 |
children | 739aa297c260 |
files | src/java.base/share/classes/java/util/Collections.java test/jdk/java/util/Comparator/BasicTest.java |
diffstat | 2 files changed, 46 insertions(+), 16 deletions(-) [+] |
line wrap: on
line diff
--- a/src/java.base/share/classes/java/util/Collections.java Wed Dec 13 01:29:58 2017 +0800 +++ b/src/java.base/share/classes/java/util/Collections.java Tue Dec 12 09:33:35 2017 -0800 @@ -24,9 +24,10 @@ */ package java.util; + +import java.io.IOException; +import java.io.ObjectOutputStream; import java.io.Serializable; -import java.io.ObjectOutputStream; -import java.io.IOException; import java.lang.reflect.Array; import java.util.function.BiConsumer; import java.util.function.BiFunction; @@ -5164,14 +5165,19 @@ * specified comparator. * @since 1.5 */ + @SuppressWarnings("unchecked") public static <T> Comparator<T> reverseOrder(Comparator<T> cmp) { - if (cmp == null) - return reverseOrder(); - - if (cmp instanceof ReverseComparator2) - return ((ReverseComparator2<T>)cmp).cmp; - - return new ReverseComparator2<>(cmp); + if (cmp == null) { + return (Comparator<T>) ReverseComparator.REVERSE_ORDER; + } else if (cmp == ReverseComparator.REVERSE_ORDER) { + return (Comparator<T>) Comparators.NaturalOrderComparator.INSTANCE; + } else if (cmp == Comparators.NaturalOrderComparator.INSTANCE) { + return (Comparator<T>) ReverseComparator.REVERSE_ORDER; + } else if (cmp instanceof ReverseComparator2) { + return ((ReverseComparator2<T>) cmp).cmp; + } else { + return new ReverseComparator2<>(cmp); + } } /**
--- a/test/jdk/java/util/Comparator/BasicTest.java Wed Dec 13 01:29:58 2017 +0800 +++ b/test/jdk/java/util/Comparator/BasicTest.java Tue Dec 12 09:33:35 2017 -0800 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2013, 2017 Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,22 +23,21 @@ /** * @test + * @bug 8171826 * @summary Comparator default method tests * @run testng BasicTest */ -import java.util.TreeMap; -import java.util.Comparator; import org.testng.annotations.Test; +import java.util.Collections; +import java.util.Comparator; import java.util.function.Function; +import java.util.function.ToDoubleFunction; import java.util.function.ToIntFunction; import java.util.function.ToLongFunction; -import java.util.function.ToDoubleFunction; -import static org.testng.Assert.assertEquals; -import static org.testng.Assert.assertTrue; -import static org.testng.Assert.fail; +import static org.testng.Assert.*; @Test(groups = "unit") public class BasicTest { @@ -366,4 +365,29 @@ fail("comparing(null) should throw NPE"); } catch (NullPointerException npe) {} } + + public void testNaturalAndReverseIdentity() { + var naturalOrder = Comparator.<String>naturalOrder(); + var reverseOrder = Comparator.<String>reverseOrder(); + + assertEquals( + naturalOrder, + Collections.reverseOrder(reverseOrder), + "Comparator.naturalOrder() and Collections.reverseOrder(Comparator.reverseOrder()) not equal"); + + assertEquals( + reverseOrder, + Collections.reverseOrder(naturalOrder), + "Comparator.reverseOrder() and Collections.reverseOrder(Comparator.naturalOrder()) not equal"); + + assertEquals( + naturalOrder.reversed(), + reverseOrder, + "Comparator.naturalOrder().reversed() amd Comparator.reverseOrder() not equal"); + + assertEquals( + reverseOrder.reversed(), + naturalOrder, + "Comparator.reverseOrder().reversed() and Comparator.naturalOrder() not equal"); + } }