From 9607d252b0f325340bb9e28355fa95c28c151d9f Mon Sep 17 00:00:00 2001
From: "github-actions[bot]"
 <41898282+github-actions[bot]@users.noreply.github.com>
Date: Tue, 15 Aug 2023 11:16:33 -0700
Subject: [PATCH] [release/8.0-rc1] Fix ToFrozenDictionary(selectors) on empty
 sources (#90599)

* Fix ToFrozenDictionary(selectors) on empty sources

As part of optimizing construction, the empty check when using ToFrozenDictionary taking selector delegates was skipped, leading to later failures when an empty source is used.

* Address PR feedback

---------

Co-authored-by: Stephen Toub <stoub@microsoft.com>
---
 .../Collections/Frozen/FrozenDictionary.cs    |  6 ++-
 .../System/Collections/Frozen/FrozenSet.cs    |  2 +
 .../tests/Frozen/FrozenDictionaryTests.cs     | 40 ++++++++++++-------
 .../tests/Frozen/FrozenSetTests.cs            | 29 +++++++-------
 4 files changed, 47 insertions(+), 30 deletions(-)

diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.cs
index 0fab5139dc6351..e4fbdcef00b3c6 100644
--- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.cs
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.cs
@@ -41,7 +41,7 @@ public static FrozenDictionary<TKey, TValue> ToFrozenDictionary<TKey, TValue>(th
         public static FrozenDictionary<TKey, TSource> ToFrozenDictionary<TSource, TKey>(
             this IEnumerable<TSource> source, Func<TSource, TKey> keySelector, IEqualityComparer<TKey>? comparer = null)
             where TKey : notnull =>
-            CreateFromDictionary(source.ToDictionary(keySelector, comparer));
+            source.ToDictionary(keySelector, comparer).ToFrozenDictionary(comparer);
 
         /// <summary>Creates a <see cref="FrozenDictionary{TKey, TElement}"/> from an <see cref="IEnumerable{TSource}"/> according to specified key selector and element selector functions.</summary>
         /// <typeparam name="TSource">The type of the elements of <paramref name="source"/>.</typeparam>
@@ -55,7 +55,7 @@ public static FrozenDictionary<TKey, TSource> ToFrozenDictionary<TSource, TKey>(
         public static FrozenDictionary<TKey, TElement> ToFrozenDictionary<TSource, TKey, TElement>(
             this IEnumerable<TSource> source, Func<TSource, TKey> keySelector, Func<TSource, TElement> elementSelector, IEqualityComparer<TKey>? comparer = null)
             where TKey : notnull =>
-            CreateFromDictionary(source.ToDictionary(keySelector, elementSelector, comparer));
+            source.ToDictionary(keySelector, elementSelector, comparer).ToFrozenDictionary(comparer);
 
         /// <summary>
         /// Extracts from the source either an existing <see cref="FrozenDictionary{TKey,TValue}"/> instance or a <see cref="Dictionary{TKey,TValue}"/>
@@ -113,6 +113,8 @@ public static FrozenDictionary<TKey, TElement> ToFrozenDictionary<TSource, TKey,
         private static FrozenDictionary<TKey, TValue> CreateFromDictionary<TKey, TValue>(Dictionary<TKey, TValue> source)
             where TKey : notnull
         {
+            Debug.Assert(source.Count > 0, "Empty sources should have been filtered out by caller");
+
             IEqualityComparer<TKey> comparer = source.Comparer;
 
             // Optimize for value types when the default comparer is being used. In such a case, the implementation
diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.cs
index ed472ff80666ae..8c315f214fe03c 100644
--- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.cs
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.cs
@@ -59,6 +59,8 @@ public static FrozenSet<T> ToFrozenSet<T>(this IEnumerable<T> source, IEqualityC
 
         private static FrozenSet<T> CreateFromSet<T>(HashSet<T> source)
         {
+            Debug.Assert(source.Count > 0, "Empty sources should have been filtered out by caller");
+
             IEqualityComparer<T> comparer = source.Comparer;
 
             // Optimize for value types when the default comparer is being used. In such a case, the implementation
diff --git a/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryTests.cs b/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryTests.cs
index a1f6adc0c782bd..13e695ef0a791a 100644
--- a/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryTests.cs
+++ b/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryTests.cs
@@ -86,23 +86,35 @@ public void NullSource_ThrowsException()
         [Fact]
         public void EmptySource_ProducedFrozenDictionaryEmpty()
         {
-            Assert.Same(FrozenDictionary<TKey, TValue>.Empty, new Dictionary<TKey, TValue>().ToFrozenDictionary());
-            Assert.Same(FrozenDictionary<TKey, TValue>.Empty, Enumerable.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary());
-            Assert.Same(FrozenDictionary<TKey, TValue>.Empty, Array.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary());
-            Assert.Same(FrozenDictionary<TKey, TValue>.Empty, new List<KeyValuePair<TKey, TValue>>().ToFrozenDictionary());
+            IEnumerable<KeyValuePair<TKey, TValue>>[] sources = new[]
+            {
+                new Dictionary<TKey, TValue>(),
+                Enumerable.Empty<KeyValuePair<TKey, TValue>>(),
+                Array.Empty<KeyValuePair<TKey, TValue>>(),
+                new List<KeyValuePair<TKey, TValue>>()
+            };
 
-            foreach (IEqualityComparer<TKey> comparer in new IEqualityComparer<TKey>[] { null, EqualityComparer<TKey>.Default })
+            foreach (IEnumerable<KeyValuePair<TKey, TValue>> source in sources)
             {
-                Assert.Same(FrozenDictionary<TKey, TValue>.Empty, new Dictionary<TKey, TValue>().ToFrozenDictionary(comparer));
-                Assert.Same(FrozenDictionary<TKey, TValue>.Empty, Enumerable.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(comparer));
-                Assert.Same(FrozenDictionary<TKey, TValue>.Empty, Array.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(comparer));
-                Assert.Same(FrozenDictionary<TKey, TValue>.Empty, new List<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(comparer));
-            }
+                Assert.Same(FrozenDictionary<TKey, TValue>.Empty, source.ToFrozenDictionary());
+                Assert.Same(FrozenDictionary<TKey, KeyValuePair<TKey, TValue>>.Empty, source.ToFrozenDictionary(s => s.Key));
+                Assert.Same(FrozenDictionary<TKey, TValue>.Empty, source.ToFrozenDictionary(s => s.Key, s => s.Value));
 
-            Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, new Dictionary<TKey, TValue>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance));
-            Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, Enumerable.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance));
-            Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, Array.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance));
-            Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, new List<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance));
+                foreach (IEqualityComparer<TKey> comparer in new IEqualityComparer<TKey>[] { null, EqualityComparer<TKey>.Default })
+                {
+                    Assert.Same(FrozenDictionary<TKey, TValue>.Empty, source.ToFrozenDictionary(comparer));
+                    Assert.Same(FrozenDictionary<TKey, KeyValuePair<TKey, TValue>>.Empty, source.ToFrozenDictionary(s => s.Key, comparer));
+                    Assert.Same(FrozenDictionary<TKey, TValue>.Empty, source.ToFrozenDictionary(s => s.Key, s => s.Value, comparer));
+                }
+
+                Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, source.ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance));
+                Assert.NotSame(FrozenDictionary<TKey, KeyValuePair<TKey, TValue>>.Empty, source.ToFrozenDictionary(s => s.Key, NonDefaultEqualityComparer<TKey>.Instance));
+                Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, source.ToFrozenDictionary(s => s.Key, s => s.Value, NonDefaultEqualityComparer<TKey>.Instance));
+
+                Assert.Equal(0, source.ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance).Count);
+                Assert.Equal(0, source.ToFrozenDictionary(s => s.Key, NonDefaultEqualityComparer<TKey>.Instance).Count);
+                Assert.Equal(0, source.ToFrozenDictionary(s => s.Key, s => s.Value, NonDefaultEqualityComparer<TKey>.Instance).Count);
+            }
         }
 
         [Fact]
diff --git a/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenSetTests.cs b/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenSetTests.cs
index f9bca34ae07060..8465f66c4c3747 100644
--- a/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenSetTests.cs
+++ b/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenSetTests.cs
@@ -63,23 +63,24 @@ public void NullSource_ThrowsException()
         [Fact]
         public void EmptySource_ProducedFrozenSetEmpty()
         {
-            Assert.Same(FrozenSet<T>.Empty, new List<T>().ToFrozenSet());
-            Assert.Same(FrozenSet<T>.Empty, Enumerable.Empty<T>().ToFrozenSet());
-            Assert.Same(FrozenSet<T>.Empty, Array.Empty<T>().ToFrozenSet());
-            Assert.Same(FrozenSet<T>.Empty, new List<T>().ToFrozenSet());
+            IEnumerable<T>[] sources = new[]
+            {
+                new List<T>(),
+                Enumerable.Empty<T>(),
+                Array.Empty<T>(),
+            };
 
-            foreach (IEqualityComparer<T> comparer in new IEqualityComparer<T>[] { null, EqualityComparer<T>.Default })
+            foreach (IEnumerable<T> source in sources)
             {
-                Assert.Same(FrozenSet<T>.Empty, new List<T>().ToFrozenSet(comparer));
-                Assert.Same(FrozenSet<T>.Empty, Enumerable.Empty<T>().ToFrozenSet(comparer));
-                Assert.Same(FrozenSet<T>.Empty, Array.Empty<T>().ToFrozenSet(comparer));
-                Assert.Same(FrozenSet<T>.Empty, new List<T>().ToFrozenSet(comparer));
-            }
+                Assert.Same(FrozenSet<T>.Empty, source.ToFrozenSet());
+
+                foreach (IEqualityComparer<T> comparer in new IEqualityComparer<T>[] { null, EqualityComparer<T>.Default })
+                {
+                    Assert.Same(FrozenSet<T>.Empty, source.ToFrozenSet(comparer));
+                }
 
-            Assert.NotSame(FrozenSet<T>.Empty, new List<T>().ToFrozenSet(NonDefaultEqualityComparer<T>.Instance));
-            Assert.NotSame(FrozenSet<T>.Empty, Enumerable.Empty<T>().ToFrozenSet(NonDefaultEqualityComparer<T>.Instance));
-            Assert.NotSame(FrozenSet<T>.Empty, Array.Empty<T>().ToFrozenSet(NonDefaultEqualityComparer<T>.Instance));
-            Assert.NotSame(FrozenSet<T>.Empty, new List<T>().ToFrozenSet(NonDefaultEqualityComparer<T>.Instance));
+                Assert.NotSame(FrozenSet<T>.Empty, source.ToFrozenSet(NonDefaultEqualityComparer<T>.Instance));
+            }
         }
 
         [Fact]