From 275be6196911bf07cdb38647a5157a4a47b0d8af Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 15 Aug 2023 09:51:39 -0400 Subject: [PATCH] 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. --- .../Collections/Frozen/FrozenDictionary.cs | 4 +- .../tests/Frozen/FrozenDictionaryTests.cs | 40 ++++++++++++------- .../tests/Frozen/FrozenSetTests.cs | 29 +++++++------- 3 files changed, 43 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..02ea95e4a87378 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 ToFrozenDictionary(th public static FrozenDictionary ToFrozenDictionary( this IEnumerable source, Func keySelector, IEqualityComparer? comparer = null) where TKey : notnull => - CreateFromDictionary(source.ToDictionary(keySelector, comparer)); + source.ToDictionary(keySelector, comparer).ToFrozenDictionary(comparer); /// Creates a from an according to specified key selector and element selector functions. /// The type of the elements of . @@ -55,7 +55,7 @@ public static FrozenDictionary ToFrozenDictionary( public static FrozenDictionary ToFrozenDictionary( this IEnumerable source, Func keySelector, Func elementSelector, IEqualityComparer? comparer = null) where TKey : notnull => - CreateFromDictionary(source.ToDictionary(keySelector, elementSelector, comparer)); + source.ToDictionary(keySelector, elementSelector, comparer).ToFrozenDictionary(comparer); /// /// Extracts from the source either an existing instance or a 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.Empty, new Dictionary().ToFrozenDictionary()); - Assert.Same(FrozenDictionary.Empty, Enumerable.Empty>().ToFrozenDictionary()); - Assert.Same(FrozenDictionary.Empty, Array.Empty>().ToFrozenDictionary()); - Assert.Same(FrozenDictionary.Empty, new List>().ToFrozenDictionary()); + IEnumerable>[] sources = new[] + { + new Dictionary(), + Enumerable.Empty>(), + Array.Empty>(), + new List>() + }; - foreach (IEqualityComparer comparer in new IEqualityComparer[] { null, EqualityComparer.Default }) + foreach (IEnumerable> source in sources) { - Assert.Same(FrozenDictionary.Empty, new Dictionary().ToFrozenDictionary(comparer)); - Assert.Same(FrozenDictionary.Empty, Enumerable.Empty>().ToFrozenDictionary(comparer)); - Assert.Same(FrozenDictionary.Empty, Array.Empty>().ToFrozenDictionary(comparer)); - Assert.Same(FrozenDictionary.Empty, new List>().ToFrozenDictionary(comparer)); - } + Assert.Same(FrozenDictionary.Empty, source.ToFrozenDictionary()); + Assert.Same(FrozenDictionary>.Empty, source.ToFrozenDictionary(s => s.Key)); + Assert.Same(FrozenDictionary.Empty, source.ToFrozenDictionary(s => s.Key, s => s.Value)); - Assert.NotSame(FrozenDictionary.Empty, new Dictionary().ToFrozenDictionary(NonDefaultEqualityComparer.Instance)); - Assert.NotSame(FrozenDictionary.Empty, Enumerable.Empty>().ToFrozenDictionary(NonDefaultEqualityComparer.Instance)); - Assert.NotSame(FrozenDictionary.Empty, Array.Empty>().ToFrozenDictionary(NonDefaultEqualityComparer.Instance)); - Assert.NotSame(FrozenDictionary.Empty, new List>().ToFrozenDictionary(NonDefaultEqualityComparer.Instance)); + foreach (IEqualityComparer comparer in new IEqualityComparer[] { null, EqualityComparer.Default }) + { + Assert.Same(FrozenDictionary.Empty, source.ToFrozenDictionary(comparer)); + Assert.Same(FrozenDictionary>.Empty, source.ToFrozenDictionary(s => s.Key, comparer)); + Assert.Same(FrozenDictionary.Empty, source.ToFrozenDictionary(s => s.Key, s => s.Value, comparer)); + } + + Assert.NotSame(FrozenDictionary.Empty, source.ToFrozenDictionary(NonDefaultEqualityComparer.Instance)); + Assert.NotSame(FrozenDictionary>.Empty, source.ToFrozenDictionary(s => s.Key, NonDefaultEqualityComparer.Instance)); + Assert.NotSame(FrozenDictionary.Empty, source.ToFrozenDictionary(s => s.Key, s => s.Value, NonDefaultEqualityComparer.Instance)); + + Assert.Equal(0, source.ToFrozenDictionary(NonDefaultEqualityComparer.Instance).Count); + Assert.Equal(0, source.ToFrozenDictionary(s => s.Key, NonDefaultEqualityComparer.Instance).Count); + Assert.Equal(0, source.ToFrozenDictionary(s => s.Key, s => s.Value, NonDefaultEqualityComparer.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.Empty, new List().ToFrozenSet()); - Assert.Same(FrozenSet.Empty, Enumerable.Empty().ToFrozenSet()); - Assert.Same(FrozenSet.Empty, Array.Empty().ToFrozenSet()); - Assert.Same(FrozenSet.Empty, new List().ToFrozenSet()); + IEnumerable[] sources = new[] + { + new List(), + Enumerable.Empty(), + Array.Empty(), + }; - foreach (IEqualityComparer comparer in new IEqualityComparer[] { null, EqualityComparer.Default }) + foreach (IEnumerable source in sources) { - Assert.Same(FrozenSet.Empty, new List().ToFrozenSet(comparer)); - Assert.Same(FrozenSet.Empty, Enumerable.Empty().ToFrozenSet(comparer)); - Assert.Same(FrozenSet.Empty, Array.Empty().ToFrozenSet(comparer)); - Assert.Same(FrozenSet.Empty, new List().ToFrozenSet(comparer)); - } + Assert.Same(FrozenSet.Empty, source.ToFrozenSet()); + + foreach (IEqualityComparer comparer in new IEqualityComparer[] { null, EqualityComparer.Default }) + { + Assert.Same(FrozenSet.Empty, source.ToFrozenSet(comparer)); + } - Assert.NotSame(FrozenSet.Empty, new List().ToFrozenSet(NonDefaultEqualityComparer.Instance)); - Assert.NotSame(FrozenSet.Empty, Enumerable.Empty().ToFrozenSet(NonDefaultEqualityComparer.Instance)); - Assert.NotSame(FrozenSet.Empty, Array.Empty().ToFrozenSet(NonDefaultEqualityComparer.Instance)); - Assert.NotSame(FrozenSet.Empty, new List().ToFrozenSet(NonDefaultEqualityComparer.Instance)); + Assert.NotSame(FrozenSet.Empty, source.ToFrozenSet(NonDefaultEqualityComparer.Instance)); + } } [Fact]