Proposal: Add IsImmutable property to IReadOnlyCollection<T> and IReadOnlyDictionary<TKey,TValue>

[dmitriyse]

When dotnet/csharplang#52 feature will be added to C#/CLR (at least in restricted form), we will have ability to extend base BCL contracts.
This proposal rely on this suggestion and tries to solve ambiguous semantic of ICollection<T>.IsReadOnly property.

We needs a new property.

public interface IReadOnlyCollection<T>
{
     bool IsImmutable { get;}
}

public interface ICollection<T>
{
     bool IsImmutable { get;}
}

public interface IReadOnlyDictionary<T>
{
     bool IsImmutable { get;}
}

public interface IDictionary<T>
{
     bool IsImmutable { get;}
}

Let suppose we have two different collections both inherited from ICollection<T>

First example is when library allows to access some internal collection by the interface.

internal class PrivateCollection: ICollection<T>
{
     // Read methods allowed for library and for external usage.
     bool Contains(T item)
     {
           // Some valid implementation.
     }
     
     // This method is only allowed for some library internal classes.
     internal void Add(T item)
     {
           // Some valid imiplementation.
     }
     
     // Modification is closed for external users, but not for internal.
     void ICollection<T>.Add(T item)
     {
           throw new NotSupportedException();
     }
     
     public bool IsReadOnly
     {
           get
           {
                 // In this case true mean - this collection allowed to only read from ICollection<T>
                 // interface.
                 // But does not guaranties that collection will not be changed by some internal method.
                 return true;
           }
     }
}

this is ReadOnly meaning of IsReadOnly property.

---

Second collection can be well known ImmutableList

public class ImmutableList<T>: ICollection<T>
{
      bool ICollection<T>.IsReadOnly
      {
            get
            {
                  // Here it tells that collection is not only "ReadOnly" but also will never be changed by nobody.
                  return true;
            }
      }
}

Probably "it's too late" and we have tons of code one collections uses ReadOnly semantic
another Collections uses ReadOnly+Immutable semantic.

But now we can do the next improvement in the documentation:
IsReadOnly says that only:
all mutation method will return NotSupportedException or InvalidOperationException. And no any more guaranties.

[dmitriyse]

Second approach I described in this issue https://github.com/dotnet/corefx/issues/16661

[svick]

In many cases we needs to know if provided IReadOnlyCollection (actually "readable" semantic) is also "immutable".

Could you explain what those cases are? The one I can think of is when you're caching some collection and want to know if it's safe to cache it directly, or if you have to create a copy of it.

ICollection contains property IsReadOnly with exactly this logic.

That's not what ICollection<T>.IsReadOnly does. Consider this code:

var list = new List<int>();
ICollection<int> ro = new ReadOnlyCollection<int>(list);
Console.WriteLine(ro.IsReadOnly);
Console.WriteLine(ro.Count);
list.Add(1);
Console.WriteLine(ro.Count);

The output shows that even though ro.IsReadOnly returns true, ro is not immutable:

True
0
1

The documentation could be clearer about this.

[dmitriyse]

This is very bad example because ReadOnlyCollection is a special case. Main purpose of this class is to save some collection from changes but allow to read collection content.
It's protects from guys who will do:

public void ReadMyCollection(IReadOnlyCollection<int> c)
{
    var mutableCollection = (ICollection<T>)c;
    // Make diversion
    c.Add(34);
}

IsReadOnly looks like have the problem with convention. IsReadOnly is ReadOnly or Immutable.
So we exactly trying to move from this dualism hell.

[dmitriyse]

Probably the only solution would be adding new IsImmutable property.
We should analyze all BCL usages of IsReadOnly.

And finally write strict convention into all documentation.

[karelz]

@svick do you have documentation change proposal? We can ask docs folks to update it ...

@dmitriyse I still don't get your differentiation between IsReadOnly and IsImmutable, sorry. Do you have a small sample to demonstrate the difference? (something like @svick used above?)

[dmitriyse]

I updated topic and there an answer.

[karelz]

OK, so IsImmutable means that it is truly immutable and doesn't change any internal state (caching, etc.).
What are the scenarios when you need that information? As interface/collection class user isn't IsReadOnly sufficient for you?
Why do you need to know more about implementation details of the collection?

[karelz]

The motivation was discussed here: https://github.com/dotnet/corefx/issues/16661#issuecomment-284587410
It looks like this is a dupe (part of) the large issue.
We have to choose to either:

• First clarify high-level motivation (as I mentioned maybe not on GitHub), then we can spin off smaller work items like this one, or
• Spin off standalone API proposals (like this one) which can clearly express motivation and justification on their own.

Of course, let's open only API proposals which do NOT depend on not-yet-implemented C# features - in such case we should wait (see https://github.com/dotnet/corefx/issues/16818#issuecomment-284790069).

Closing as dupe for now, please let me know if you disagree.