[API Proposal]: ReadOnly empty collection singletons

[stephentoub]

Background and motivation

Various pieces of code today allocate new read-only collections (e.g. ReadOnlyCollection<T>) around empty collections. Sometimes code implements its own singleton caching, e.g.

• https://github.com/dnSpy/dnSpy/blob/2b6dcfaf602fb8ca6462b8b6237fdfc0c74ad994/dnSpy/dnSpy/Documents/Tabs/DocViewer/DebugInfoDocumentViewerCustomDataProvider.cs#L33
• https://github.com/dnSpy/dnSpy/blob/2b6dcfaf602fb8ca6462b8b6237fdfc0c74ad994/Extensions/dnSpy.Debugger/dnSpy.Debugger.DotNet.Metadata/ReadOnlyCollectionHelpers.cs#L26

• runtime/src/libraries/System.ComponentModel.Composition/src/System/ComponentModel/Composition/Hosting/CompositionContainer.cs

  Line 30 in 58b6828

  private static readonly ReadOnlyCollection<ExportProvider> EmptyProviders = new ReadOnlyCollection<ExportProvider>(Array.Empty<ExportProvider>());

• https://github.com/dotnet/roslyn/blob/121d9b5cdba4c917c99fbcb6cfa85a5cf10e6e59/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/EvaluationContext.cs#L401-L402
• https://github.com/dotnet/maui/blob/dbe44e68802b7585918e4c001f75be9770dd063d/src/Controls/src/Core/Element.cs#L15
• https://github.com/PowerShell/PowerShell/blob/c978d467ab778fe544808227d915408eaba6b22c/src/System.Management.Automation/engine/Utils.cs#L1451-L1452
• https://github.com/dotnet/wpf/blob/a19b3d79d3dac18930f943e146cfce67a48bb0eb/src/Microsoft.DotNet.Wpf/src/System.Xaml/System/Xaml/XamlType.cs#L1823-L1824
• https://github.com/dotnet/SqlClient/blob/7d348eb0c985b34ce5b76a9330c57bc1015c93ff/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs#L75

• runtime/src/libraries/System.ComponentModel.Composition/src/System/ComponentModel/Composition/MetadataServices.cs

  Line 12 in 58b6828

  public static readonly IDictionary<string, object?> EmptyMetadata = new ReadOnlyDictionary<string, object?>(new Dictionary<string, object?>(0));

• https://github.com/UiPath/CoreWF/blob/725ab544ec199e7d3013bab3740f77d47c10d892/src/UiPath.Workflow.Runtime/Runtime/DurableInstancing/InstanceOwnerQueryResult.cs#L12-L13
• https://github.com/elastic/elasticsearch-net/blob/fc5d848f3441279bc907ae2000a6ae1f9a2c606d/src/Elastic.Clients.Elasticsearch/Common/IsADictionaryBase.cs#L151
• https://github.com/Particular/NServiceBus/blob/56ca994085a487bbd9597ec1e37513cdf11f02a7/src/NServiceBus.Core/Transports/QueueAddress.cs#L12
• https://github.com/UiPath/CoreWF/blob/725ab544ec199e7d3013bab3740f77d47c10d892/src/UiPath.Workflow.Runtime/Runtime/DurableInstancing/InstanceKey.cs#L13
• https://github.com/datastax/csharp-driver/blob/27738b0ddd62c8e523fb2867c48d331257738a6c/src/Cassandra/Exceptions/ReadFailureException.cs#L29-L30
• https://github.com/googleapis/gax-dotnet/blob/01c7e7fd221649f98eb08586017f62d6ee2ffc22/Google.Api.Gax/EmptyDictionary.cs#L23-L24

and sometimes it doesn't, e.g.

• runtime/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/Pkcs12Info.cs

  Lines 117 to 120 in 58b6828

  	if (authSafeData.Count == 0)
  	{
  	authSafe = new ReadOnlyCollection<Pkcs12SafeContents>(Array.Empty<Pkcs12SafeContents>());
  	}

• https://github.com/JosefPihrt/Roslynator/blob/8b27c6ac78c17f5abd8151d995bdb4f88f394b0e/src/Tools/Metadata/SourceFile.cs#L17
• https://github.com/dotnet-script/dotnet-script/blob/b5ff49b47134a2aa716d9dc9f08355d3e89d7638/src/Dotnet.Script.Core/ScriptContext.cs#L17
• https://github.com/UiPath/CoreWF/blob/725ab544ec199e7d3013bab3740f77d47c10d892/src/UiPath.Workflow.Runtime/HybridCollection.cs#L116
• https://github.com/dotnet/wcf/blob/41dfb82e834f7f798e091be235925d44469a9cac/src/dotnet-svcutil/lib/src/FrameworkFork/System.Runtime.Serialization/System/Runtime/Serialization/DataContractSerializer.cs#L156

• runtime/src/libraries/System.Speech/src/Synthesis/VoiceInfo.cs

  Line 204 in 58b6828

  public IDictionary<string, string> AdditionalInfo => _attributes ??= new ReadOnlyDictionary<string, string>(new Dictionary<string, string>(0));

• https://github.com/psibr/REstate/blob/0a3ae3b6d3c5dbf368385476ea4eec2aa5defbb2/src/REstate.Remote/GrpcStateMachine.cs#L66

In other cases, empty could be special-cased but isn't as doing so would have required someone to add their own cache for it to have meaningful benefits).

In any case, we can make this easier and more efficient by exposing our own singletons that we and others can use.

(Replaces #41147, which is limited to just ReadOnlyCollection.)
(Related to #38340, which asked for such Empty methods on mutable collections like List<T> returning empty instances of those same collections. There we said we'd consider read-only variants with a proposal; this is that proposal.)

API Proposal

namespace System.Collections.Generic
{
    public interface IReadOnlyCollection<out T>
    {
+        public static IReadOnlyCollection<T> Empty { get; }
    }

    public interface IReadOnlyList<out T> : IReadOnlyCollection<T>
    {
+        public static new IReadOnlyList<T> Empty { get; }
    }

    public interface IReadOnlyDictionary<TKey, TValue> : IReadOnlyCollection<KeyValuePair<TKey, TValue>>
    {
+        public static new IReadOnlyDictionary<TKey, TValue> Empty { get; }
    }

    public interface IReadOnlySet<T> : IReadOnlyCollection<T>
    {
+        public static new IReadOnlySet<T> Empty { get; }
    }
}

namespace System.Collections.ObjectModel
{
    public class ReadOnlyCollection<T>
    {
+        public static ReadOnlyCollection<T> Empty { get; }
    }

    public class ReadOnlyDictionary<TKey, TValue>
    {
+        public static ReadOnlyDictionary<TKey, TValue> Empty { get; }
    }

    public class ReadOnlyObservableCollection<T>
    {
+        public static ReadOnlyObservableCollection<T> Empty { get; }
    }
}

API Usage

public ReadOnlyCollection<T> GetValues() =>
    _count != 0 ? new ReadOnlyCollection<T>(_values) : ReadOnlyCollection<T>.Empty;

Alternative Designs

No response

Risks

• These might be our first non-abstract/virtual static interface methods in the core libraries?

[ghost]

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

Various pieces of code today allocate new read-only collections (e.g. ReadOnlyCollection<T>) around empty collections. Sometimes code implements its own singleton caching, e.g.

• https://github.com/dnSpy/dnSpy/blob/2b6dcfaf602fb8ca6462b8b6237fdfc0c74ad994/dnSpy/dnSpy/Documents/Tabs/DocViewer/DebugInfoDocumentViewerCustomDataProvider.cs#L33
• https://github.com/dnSpy/dnSpy/blob/2b6dcfaf602fb8ca6462b8b6237fdfc0c74ad994/Extensions/dnSpy.Debugger/dnSpy.Debugger.DotNet.Metadata/ReadOnlyCollectionHelpers.cs#L26

• runtime/src/libraries/System.ComponentModel.Composition/src/System/ComponentModel/Composition/Hosting/CompositionContainer.cs

  Line 30 in 58b6828

  private static readonly ReadOnlyCollection<ExportProvider> EmptyProviders = new ReadOnlyCollection<ExportProvider>(Array.Empty<ExportProvider>());

• https://github.com/dotnet/roslyn/blob/121d9b5cdba4c917c99fbcb6cfa85a5cf10e6e59/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/EvaluationContext.cs#L401-L402
• https://github.com/dotnet/maui/blob/dbe44e68802b7585918e4c001f75be9770dd063d/src/Controls/src/Core/Element.cs#L15
• https://github.com/PowerShell/PowerShell/blob/c978d467ab778fe544808227d915408eaba6b22c/src/System.Management.Automation/engine/Utils.cs#L1451-L1452
• https://github.com/dotnet/wpf/blob/a19b3d79d3dac18930f943e146cfce67a48bb0eb/src/Microsoft.DotNet.Wpf/src/System.Xaml/System/Xaml/XamlType.cs#L1823-L1824
• https://github.com/dotnet/SqlClient/blob/7d348eb0c985b34ce5b76a9330c57bc1015c93ff/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs#L75

• runtime/src/libraries/System.ComponentModel.Composition/src/System/ComponentModel/Composition/MetadataServices.cs

  Line 12 in 58b6828

  public static readonly IDictionary<string, object?> EmptyMetadata = new ReadOnlyDictionary<string, object?>(new Dictionary<string, object?>(0));

• https://github.com/UiPath/CoreWF/blob/725ab544ec199e7d3013bab3740f77d47c10d892/src/UiPath.Workflow.Runtime/Runtime/DurableInstancing/InstanceOwnerQueryResult.cs#L12-L13
• https://github.com/elastic/elasticsearch-net/blob/fc5d848f3441279bc907ae2000a6ae1f9a2c606d/src/Elastic.Clients.Elasticsearch/Common/IsADictionaryBase.cs#L151
• https://github.com/Particular/NServiceBus/blob/56ca994085a487bbd9597ec1e37513cdf11f02a7/src/NServiceBus.Core/Transports/QueueAddress.cs#L12
• https://github.com/UiPath/CoreWF/blob/725ab544ec199e7d3013bab3740f77d47c10d892/src/UiPath.Workflow.Runtime/Runtime/DurableInstancing/InstanceKey.cs#L13
• https://github.com/datastax/csharp-driver/blob/27738b0ddd62c8e523fb2867c48d331257738a6c/src/Cassandra/Exceptions/ReadFailureException.cs#L29-L30
• https://github.com/googleapis/gax-dotnet/blob/01c7e7fd221649f98eb08586017f62d6ee2ffc22/Google.Api.Gax/EmptyDictionary.cs#L23-L24

and sometimes it doesn't, e.g.

• runtime/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/Pkcs12Info.cs

  Lines 117 to 120 in 58b6828

  	if (authSafeData.Count == 0)
  	{
  	authSafe = new ReadOnlyCollection<Pkcs12SafeContents>(Array.Empty<Pkcs12SafeContents>());
  	}

• https://github.com/JosefPihrt/Roslynator/blob/8b27c6ac78c17f5abd8151d995bdb4f88f394b0e/src/Tools/Metadata/SourceFile.cs#L17
• https://github.com/dotnet-script/dotnet-script/blob/b5ff49b47134a2aa716d9dc9f08355d3e89d7638/src/Dotnet.Script.Core/ScriptContext.cs#L17
• https://github.com/UiPath/CoreWF/blob/725ab544ec199e7d3013bab3740f77d47c10d892/src/UiPath.Workflow.Runtime/HybridCollection.cs#L116
• https://github.com/dotnet/wcf/blob/41dfb82e834f7f798e091be235925d44469a9cac/src/dotnet-svcutil/lib/src/FrameworkFork/System.Runtime.Serialization/System/Runtime/Serialization/DataContractSerializer.cs#L156

• runtime/src/libraries/System.Speech/src/Synthesis/VoiceInfo.cs

  Line 204 in 58b6828

  public IDictionary<string, string> AdditionalInfo => _attributes ??= new ReadOnlyDictionary<string, string>(new Dictionary<string, string>(0));

• https://github.com/psibr/REstate/blob/0a3ae3b6d3c5dbf368385476ea4eec2aa5defbb2/src/REstate.Remote/GrpcStateMachine.cs#L66

In other cases, empty could be special-cased but isn't as doing so would have required someone to add their own cache for it to have meaningful benefits).

In any case, we can make this easier and more efficient by exposing our own singletons that we and others can use.

(Replaces #41147, which is limited to just ReadOnlyCollection.)
(Related to #38340, which asked for such Empty methods on mutable collections like List<T> returning empty instances of those same collections. There we said we'd consider read-only variants with a proposal; this is that proposal.)

API Proposal

namespace System.Collections.Generic
{
    public interface IReadOnlyCollection<out T>
    {
+        public static IReadOnlyCollection<T> Empty { get; }
    }

    public interface IReadOnlyList<out T> : IReadOnlyCollection<T>
    {
+        public static new IReadOnlyList<T> Empty { get; }
    }

    public interface IReadOnlyDictionary<TKey, TValue> : IReadOnlyCollection<KeyValuePair<TKey, TValue>>
    {
+        public static new IReadOnlyDictionary<TKey, TValue> Empty { get; }
    }

    public interface IReadOnlySet<T> : IReadOnlyCollection<T>
    {
+        public static new IReadOnlySet<T> Empty { get; }
    }
}

namespace System.Collections.ObjectModel
{
    public class ReadOnlyCollection<T>
    {
+        public static ReadOnlyCollection<T> Empty { get; }
    }

    public class ReadOnlyDictionary<TKey, TValue>
    {
+        public static ReadOnlyDictionary<TKey, TValue> Empty { get; }
    }
}

API Usage

public ReadOnlyCollection<T> GetValues() =>
    _count != 0 ? new ReadOnlyCollection<T>(_values) : ReadOnlyCollection<T>.Empty;

Alternative Designs

No response

Risks

• These might be our first non-abstract/virtual static interface methods in the core libraries?

Author:	stephentoub
Assignees:	-
Labels:	api-suggestion, area-System.Collections
Milestone:	8.0.0

[eiriktsarpalis]

These might be our first non-abstract/virtual static interface methods in the core libraries?

Any potential risks? From a discoverability standpoint they seem pretty nice, and possibly a pattern we could be introducing in other interface types.

[stephentoub]

Any potential risks?

Not that I'm aware of. It just hasn't been expressible until relatively recently (C# 8 if memory serves).

[huoyaoyuan]

For IReadOnlyCollection<T> and IReadOnlyList<T>, what's the benefit comparing to Array.Empty?

[stephentoub]

For IReadOnlyCollection and IReadOnlyList, what's the benefit comparing to Array.Empty?

Discoverability and consistency across the interfaces. There's no performance benefit I can think of.

[eiriktsarpalis]

For IReadOnlyCollection and IReadOnlyList, what's the benefit comparing to Array.Empty?

Discoverability and consistency across the interfaces. There's no performance benefit I can think of.

Should we be using private implementations for each these interfaces or does returning Array.Empty suffice? Assuming we do the latter, Hyrum's law dictates that users will take a dependency on the underlying type being an array, but I don't foresee this needing to change.

[stephentoub]

Should we be using private implementations for each these interfaces or does returning Array.Empty suffice?

Unless we can optimize something better than what array does, I think we should just use Array.Empty.

[omariom]

ReadOnlyObservableCollection seems to be a good candidate too

[GSPP]

Unless we can optimize something better than what array does, I think we should just use Array.Empty.

This will invariably lead to people taking a dependency on these values being actual arrays. Of course, nobody should do that but it's going to happen. This could make future changes to these properties difficult from a compatibility standpoint. Maybe it's safer to have a private implementation.

For example, a reflection-based serializer might take a different path based on runtime type.

[eiriktsarpalis]

For example, a reflection-based serializer might take a different path based on runtime type.

That's a good point, actually. Any reflection-based serializer using polymorphism might provide divergent serialization contracts for empty instances.

[danmoseley]

What does using the interface members look like -- var empty = IReadOnlyList<Foo>.Empty; ?

List<Foo>.Empty would be more discoverable, presumably. (or create a new ReadOnlyList<T> and put it on there, similar for ReadOnlyHashSet<T> perhaps). what am I missing?

[stephentoub]

what am I missing?

That List<Foo>.Empty would be expected to return a List<Foo> (which we can't do), not IReadOnlyList<Foo>. So it would discoverably return the wrong thing.

[danmoseley]

Right, I figured the return type is something that could be navigated, once you've found the thing. But I defer to API experts.

[danmoseley]

And is there value in considering adding ReadOnlyList<T> and ReadOnlyHashSet<T>, including Empty following the pattern above?

[stephentoub]

And is there value in considering adding ReadOnlyList<T> and ReadOnlyHashSet<T>, including Empty following the pattern above?

ReadOnlyCollection<T> is an IReadOnlyList<T>. I don't think adding a whole new type that's a duplicate of ReadOnlyCollection<T> adds enough values.

There might be value in a ReadOnlySet<T> ala ReadOnlyDictionary<TKey, TValue>, but I wouldn't add that purely for Empty. Rather, we could choose to add such a thing on its own merit, and if we ever did, it's surface area would include an Empty property.

[tfenise]

Would this cause confusions, as the proposed static properties would be inherited? For example, if IReadOnlyList<T>.Empty were introduced, IImmutableList<T>, which inherits from IReadOnlyList<T>, would also have an inherited static property Empty, which unfortunately would be of type IReadOnlyList<T>, not IImmutableList<T>.

For other languages like VB or F#, would static interface members be difficult to consume?

[stephentoub]

add IReadOnlySet<T>.Empty

That's already there in the list.

[terrajobst]

Video

• We're undecided on the statics on the interface
  • We haven't used non-virtual statics on interface
  • Unclear what this means for language interop
  • Unclear how it impacts IntelliSense on interfaces extending these
  • It's new pattern
• We are in favor of exposing them on the concrete types
• We currently don't have a ReadOnlySet<T>; if we had this, we should also a static Empty there

namespace System.Collections.ObjectModel
{
    public partial class ReadOnlyCollection<T>
    {
        public static ReadOnlyCollection<T> Empty { get; }
    }

    public partial class ReadOnlyDictionary<TKey, TValue>
    {
        public static ReadOnlyDictionary<TKey, TValue> Empty { get; }
    }

    public partial class ReadOnlyObservableCollection<T>
    {
        public static ReadOnlyObservableCollection<T> Empty { get; }
    }
}

[alexrp]

FWIW, I just ran into a situation today where I would have liked to have IReadOnlySet<T>.Empty and had to roll my own. Would be a bit unfortunate if that doesn't get included here in some form.