8171826: Comparator.reverseOrder(c) mishandles singleton comparators
Reviewed-by: rriggs
--- 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.Serializable;
+
+import java.io.IOException;
import java.io.ObjectOutputStream;
-import java.io.IOException;
+import java.io.Serializable;
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");
+ }
}