From 22e39f9b747a75dd387c61b32f837b9aa0005f98 Mon Sep 17 00:00:00 2001 From: Alexander Radchenko Date: Fri, 10 Jan 2025 21:24:59 +0700 Subject: [PATCH] Allow collection expressions for 'System.Collections.ObjectModel' collection types (#111093) * #110161 Allow collection expressions for 'System.Collections.ObjectModel' collection types https://github.com/dotnet/runtime/issues/110161 * Code review Enhance ReadOnlyCollection with documentation and updates The `EditorBrowsable` attribute is replaced with a `SuppressMessage` attribute to clarify the intended usage of these methods. Additionally, the condition for checking if input values are empty is updated from `values.Length <= 0` to `values.IsEmpty` for improved readability and performance. * Remove unused namespace from ReadOnlyCollection.cs Eliminated the `using System.ComponentModel;` directive, reducing dependencies and potentially simplifying the code structure. * Remove suppression attributes from collection methods The `CreateCollection` method in the `ReadOnlyCollection` class had its suppression attribute removed, addressing a style warning related to collection initialization while maintaining its functionality. Similarly, the `CreateSet` method in the `ReadOnlySet` class also had its suppression attribute removed, preserving its original functionality. * Refactor `(HashSet)[.. values]` to manual loop * Marked CreateCollection and CreateSet methods in ReadOnlyCollection with the `[EditorBrowsable(EditorBrowsableState.Never)]` attribute to hide them from IntelliSense. * Apply suggestions from code review * Refactor ReadOnlyCollection and ReadOnlySet APIs Updated method names in ReadOnlyCollection and ReadOnlySet from CreateCollection and CreateSet to Create for clarity and consistency. Removed the CreateSet method from ReadOnlyCollection to streamline functionality. Adjusted documentation comments for improved clarity. Updated CollectionBuilder attributes to reflect the new method names, ensuring proper linkage for collection creation. * Updated the `CollectionBuilder` attribute to reference `ReadOnlySet` instead of `ReadOnlyCollection`, reflecting a change in the collection type used in the `ReadOnlySet` class. * Code Style * Rename methods in ReadOnlyCollection and ReadOnlySet Updated `ReadOnlyCollection` to rename `Create` to `CreateCollection` and introduced `CreateSet` for creating `ReadOnlySet` instances from spans. Adjusted `CollectionBuilder` attributes accordingly and removed the static `ReadOnlySet` class, integrating its functionality into the main class. Updated the `ReadOnlyCollection` partial class in `System.Runtime.cs` to maintain API consistency. --------- Co-authored-by: Eirik Tsarpalis --- .../Generic/ReadOnlySet/ReadOnlySetTests.cs | 19 +++++++++ .../ObjectModel/ReadOnlyCollection.cs | 42 +++++++++++++++++++ .../Collections/ObjectModel/ReadOnlySet.cs | 2 + .../System.Runtime/ref/System.Runtime.cs | 7 ++++ .../ObjectModel/ReadOnlyCollectionTests.cs | 17 ++++++++ 5 files changed, 87 insertions(+) diff --git a/src/libraries/System.Collections/tests/Generic/ReadOnlySet/ReadOnlySetTests.cs b/src/libraries/System.Collections/tests/Generic/ReadOnlySet/ReadOnlySetTests.cs index 0080ceb603c7e6..171d10ef444334 100644 --- a/src/libraries/System.Collections/tests/Generic/ReadOnlySet/ReadOnlySetTests.cs +++ b/src/libraries/System.Collections/tests/Generic/ReadOnlySet/ReadOnlySetTests.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Linq; using Xunit; namespace System.Collections.ObjectModel.Tests @@ -21,6 +22,24 @@ public void Ctor_SetProperty_Roundtrips() Assert.Same(set, new DerivedReadOnlySet(set).Set); } + [Fact] + public static void Ctor_CollectionExpressions_Empty() + { + ReadOnlySet set = []; + Assert.IsType>(set); + Assert.Empty(set); + Assert.Same(set, ReadOnlySet.Empty); + } + + [Fact] + public static void Ctor_CollectionExpressions() + { + int[] array = [1, 2, 3, 3, 2, 1]; + ReadOnlySet set = [.. array]; + Assert.IsType>(set); + Assert.Equal(set.Order(), array.Distinct().Order()); + } + [Fact] public void Empty_EmptyAndIdempotent() { diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/ReadOnlyCollection.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/ReadOnlyCollection.cs index 62e45314a1c7cf..c4fc971b856512 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/ReadOnlyCollection.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/ReadOnlyCollection.cs @@ -8,6 +8,7 @@ namespace System.Collections.ObjectModel { [Serializable] + [CollectionBuilder(typeof(ReadOnlyCollection), "CreateCollection")] [DebuggerTypeProxy(typeof(ICollectionDebugView<>))] [DebuggerDisplay("Count = {Count}")] [TypeForwardedFrom("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")] @@ -229,4 +230,45 @@ void IList.RemoveAt(int index) ThrowHelper.ThrowNotSupportedException(ExceptionResource.NotSupported_ReadOnlyCollection); } } + + /// + /// Provides static methods for read-only collections. + /// + public static class ReadOnlyCollection + { + /// + /// Creates a new from the specified span of values. + /// This method (simplifies collection initialization)[/dotnet/csharp/language-reference/operators/collection-expressions] + /// to create a new with the specified values. + /// + /// The type of elements in the collection. + /// The span of values to include in the collection. + /// A new containing the specified values. + public static ReadOnlyCollection CreateCollection(params ReadOnlySpan values) => + values.IsEmpty ? ReadOnlyCollection.Empty : new ReadOnlyCollection(values.ToArray()); + + /// + /// Creates a new from the specified span of values. + /// This method (simplifies collection initialization)[/dotnet/csharp/language-reference/operators/collection-expressions] + /// to create a new with the specified values. + /// + /// The type of elements in the collection. + /// The span of values to include in the collection. + /// A new containing the specified values. + public static ReadOnlySet CreateSet(params ReadOnlySpan values) + { + if (values.IsEmpty) + { + return ReadOnlySet.Empty; + } + + HashSet hashSet = []; + foreach (T value in values) + { + hashSet.Add(value); + } + + return new ReadOnlySet(hashSet); + } + } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/ReadOnlySet.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/ReadOnlySet.cs index c9b26b24deeaa0..6eeb927248d104 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/ReadOnlySet.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/ReadOnlySet.cs @@ -3,11 +3,13 @@ using System.Collections.Generic; using System.Diagnostics; +using System.Runtime.CompilerServices; namespace System.Collections.ObjectModel { /// Represents a read-only, generic set of values. /// The type of values in the set. + [CollectionBuilder(typeof(ReadOnlyCollection), "CreateSet")] [DebuggerDisplay("Count = {Count}")] public class ReadOnlySet : IReadOnlySet, ISet, ICollection { diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index da44791d9607cf..533b2bc75bf620 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -8366,6 +8366,12 @@ void System.Collections.ICollection.CopyTo(System.Array array, int index) { } void System.Collections.IList.Insert(int index, object? value) { } void System.Collections.IList.Remove(object? value) { } } + public static partial class ReadOnlyCollection + { + public static System.Collections.ObjectModel.ReadOnlyCollection CreateCollection(params System.ReadOnlySpan values) { throw null; } + public static System.Collections.ObjectModel.ReadOnlySet CreateSet(params System.ReadOnlySpan values) { throw null; } + } + [System.Runtime.CompilerServices.CollectionBuilder(typeof(System.Collections.ObjectModel.ReadOnlyCollection), "CreateCollection")] public partial class ReadOnlyCollection : System.Collections.Generic.ICollection, System.Collections.Generic.IEnumerable, System.Collections.Generic.IList, System.Collections.Generic.IReadOnlyCollection, System.Collections.Generic.IReadOnlyList, System.Collections.ICollection, System.Collections.IEnumerable, System.Collections.IList { public ReadOnlyCollection(System.Collections.Generic.IList list) { } @@ -8471,6 +8477,7 @@ void System.Collections.ICollection.CopyTo(System.Array array, int index) { } System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; } } } + [System.Runtime.CompilerServices.CollectionBuilder(typeof(System.Collections.ObjectModel.ReadOnlyCollection), "CreateSet")] public partial class ReadOnlySet : System.Collections.Generic.ICollection, System.Collections.Generic.IEnumerable, System.Collections.Generic.IReadOnlyCollection, System.Collections.Generic.IReadOnlySet, System.Collections.Generic.ISet, System.Collections.ICollection, System.Collections.IEnumerable { public ReadOnlySet(System.Collections.Generic.ISet @set) { } diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/ReadOnlyCollectionTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/ReadOnlyCollectionTests.cs index d8b1467358a0a3..dd8294a8d16439 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/ReadOnlyCollectionTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/ReadOnlyCollectionTests.cs @@ -30,6 +30,23 @@ public static void Ctor_IList() Assert.Same(s_intArray, collection.GetItems()); } + [Fact] + public static void Ctor_CollectionExpressions_Empty() + { + ReadOnlyCollection c = []; + Assert.IsType>(c); + Assert.Empty(c); + Assert.Same(c, ReadOnlyCollection.Empty); + } + + [Fact] + public static void Ctor_CollectionExpressions() + { + ReadOnlyCollection c = [.. s_intArray]; + Assert.IsType>(c); + Assert.Equal(c, s_intArray); + } + [Fact] public static void Count() {