8171826: Comparator.reverseOrder(c) mishandles singleton comparators
authorpsandoz
Tue, 12 Dec 2017 09:33:35 -0800
changeset 48249 c21740de9431
parent 48248 55b9b1e184c6
child 48250 739aa297c260
8171826: Comparator.reverseOrder(c) mishandles singleton comparators Reviewed-by: rriggs
src/java.base/share/classes/java/util/Collections.java
test/jdk/java/util/Comparator/BasicTest.java
--- 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");
+    }
 }