8040892: Incorrect message in Exception thrown by Collectors.toMap(Function,Function)

Use Map.putIfAbsent instead of Map.merge when collecting into map using unique keys

Reviewed-by: psandoz, chegar

jdk/src/share/classes/java/util/stream/Collectors.java

@@ -120,17 +120,63 @@ public final class Collectors {
     private Collectors() { }
 
     /**
-     * Returns a merge function, suitable for use in
-     * {@link Map#merge(Object, Object, BiFunction) Map.merge()} or
-     * {@link #toMap(Function, Function, BinaryOperator) toMap()}, which always
-     * throws {@code IllegalStateException}.  This can be used to enforce the
-     * assumption that the elements being collected are distinct.
+     * Construct an {@code IllegalStateException} with appropriate message.
      *
-     * @param <T> the type of input arguments to the merge function
-     * @return a merge function which always throw {@code IllegalStateException}
+     * @param k the duplicate key
+     * @param u 1st value to be accumulated/merged
+     * @param v 2nd value to be accumulated/merged
      */
-    private static <T> BinaryOperator<T> throwingMerger() {
-        return (u,v) -> { throw new IllegalStateException(String.format("Duplicate key %s", u)); };
+    private static IllegalStateException duplicateKeyException(
+            Object k, Object u, Object v) {
+        return new IllegalStateException(String.format(
+            "Duplicate key %s (attempted merging values %s and %s)",
+            k, u, v));
+    }
+
+    /**
+     * {@code BinaryOperator<Map>} that merges the contents of its right
+     * argument into its left argument, throwing {@code IllegalStateException}
+     * if duplicate keys are encountered.
+     *
+     * @param <K> type of the map keys
+     * @param <V> type of the map values
+     * @param <M> type of the map
+     * @return a merge function for two maps
+     */
+    private static <K, V, M extends Map<K,V>>
+    BinaryOperator<M> uniqKeysMapMerger() {
+        return (m1, m2) -> {
+            for (Map.Entry<K,V> e : m2.entrySet()) {
+                K k = e.getKey();
+                V v = Objects.requireNonNull(e.getValue());
+                V u = m1.putIfAbsent(k, v);
+                if (u != null) throw duplicateKeyException(k, u, v);
+            }
+            return m1;
+        };
+    }
+
+    /**
+     * {@code BiConsumer<Map, T>} that accumulates (key, value) pairs
+     * extracted from elements into the map, throwing {@code IllegalStateException}
+     * if duplicate keys are encountered.
+     *
+     * @param keyMapper a function that maps an element into a key
+     * @param valueMapper a function that maps an element into a value
+     * @param <T> type of elements
+     * @param <K> type of map keys
+     * @param <V> type of map values
+     * @return an accumulating consumer
+     */
+    private static <T, K, V>
+    BiConsumer<Map<K, V>, T> uniqKeysMapAccumulator(Function<? super T, ? extends K> keyMapper,
+                                                    Function<? super T, ? extends V> valueMapper) {
+        return (map, element) -> {
+            K k = keyMapper.apply(element);
+            V v = Objects.requireNonNull(valueMapper.apply(element));
+            V u = map.putIfAbsent(k, v);
+            if (u != null) throw duplicateKeyException(k, u, v);
+        };
     }
 
     @SuppressWarnings("unchecked")
@@ -1209,7 +1255,10 @@ public final class Collectors {
     public static <T, K, U>
     Collector<T, ?, Map<K,U>> toMap(Function<? super T, ? extends K> keyMapper,
                                     Function<? super T, ? extends U> valueMapper) {
-        return toMap(keyMapper, valueMapper, throwingMerger(), HashMap::new);
+        return new CollectorImpl<>(HashMap::new,
+                                   uniqKeysMapAccumulator(keyMapper, valueMapper),
+                                   uniqKeysMapMerger(),
+                                   CH_ID);
     }
 
     /**
@@ -1372,7 +1421,10 @@ public final class Collectors {
     public static <T, K, U>
     Collector<T, ?, ConcurrentMap<K,U>> toConcurrentMap(Function<? super T, ? extends K> keyMapper,
                                                         Function<? super T, ? extends U> valueMapper) {
-        return toConcurrentMap(keyMapper, valueMapper, throwingMerger(), ConcurrentHashMap::new);
+        return new CollectorImpl<>(ConcurrentHashMap::new,
+                                   uniqKeysMapAccumulator(keyMapper, valueMapper),
+                                   uniqKeysMapMerger(),
+                                   CH_CONCURRENT_ID);
     }
 
     /**