Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Extensions for immutable collection builders (#21055) #31071

Merged
merged 6 commits into from
Oct 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ public static partial class ImmutableArray
public static System.Collections.Immutable.ImmutableArray<T> Create<T>(params T[] items) { throw null; }
public static System.Collections.Immutable.ImmutableArray<T> Create<T>(T[] items, int start, int length) { throw null; }
public static System.Collections.Immutable.ImmutableArray<TSource> ToImmutableArray<TSource>(this System.Collections.Generic.IEnumerable<TSource> items) { throw null; }
public static System.Collections.Immutable.ImmutableArray<TSource> ToImmutableArray<TSource>(this System.Collections.Immutable.ImmutableArray<TSource>.Builder builder) { throw null; }
}
public partial struct ImmutableArray<T> : System.Collections.Generic.ICollection<T>, System.Collections.Generic.IEnumerable<T>, System.Collections.Generic.IList<T>, System.Collections.Generic.IReadOnlyCollection<T>, System.Collections.Generic.IReadOnlyList<T>, System.Collections.ICollection, System.Collections.IEnumerable, System.Collections.IList, System.Collections.Immutable.IImmutableList<T>, System.Collections.IStructuralComparable, System.Collections.IStructuralEquatable, System.IEquatable<System.Collections.Immutable.ImmutableArray<T>>
{
Expand Down Expand Up @@ -270,6 +271,7 @@ public static partial class ImmutableDictionary
public static System.Collections.Immutable.ImmutableDictionary<TKey, TValue> ToImmutableDictionary<TSource, TKey, TValue>(this System.Collections.Generic.IEnumerable<TSource> source, System.Func<TSource, TKey> keySelector, System.Func<TSource, TValue> elementSelector) { throw null; }
public static System.Collections.Immutable.ImmutableDictionary<TKey, TValue> ToImmutableDictionary<TSource, TKey, TValue>(this System.Collections.Generic.IEnumerable<TSource> source, System.Func<TSource, TKey> keySelector, System.Func<TSource, TValue> elementSelector, System.Collections.Generic.IEqualityComparer<TKey> keyComparer) { throw null; }
public static System.Collections.Immutable.ImmutableDictionary<TKey, TValue> ToImmutableDictionary<TSource, TKey, TValue>(this System.Collections.Generic.IEnumerable<TSource> source, System.Func<TSource, TKey> keySelector, System.Func<TSource, TValue> elementSelector, System.Collections.Generic.IEqualityComparer<TKey> keyComparer, System.Collections.Generic.IEqualityComparer<TValue> valueComparer) { throw null; }
public static System.Collections.Immutable.ImmutableDictionary<TKey, TValue> ToImmutableDictionary<TKey, TValue>(this System.Collections.Immutable.ImmutableDictionary<TKey, TValue>.Builder builder) { throw null; }
}
public sealed partial class ImmutableDictionary<TKey, TValue> : System.Collections.Generic.ICollection<System.Collections.Generic.KeyValuePair<TKey, TValue>>, System.Collections.Generic.IDictionary<TKey, TValue>, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<TKey, TValue>>, System.Collections.Generic.IReadOnlyCollection<System.Collections.Generic.KeyValuePair<TKey, TValue>>, System.Collections.Generic.IReadOnlyDictionary<TKey, TValue>, System.Collections.ICollection, System.Collections.IDictionary, System.Collections.IEnumerable, System.Collections.Immutable.IImmutableDictionary<TKey, TValue>
{
Expand Down Expand Up @@ -398,6 +400,7 @@ public static partial class ImmutableHashSet
public static System.Collections.Immutable.ImmutableHashSet<T> Create<T>(params T[] items) { throw null; }
public static System.Collections.Immutable.ImmutableHashSet<TSource> ToImmutableHashSet<TSource>(this System.Collections.Generic.IEnumerable<TSource> source) { throw null; }
public static System.Collections.Immutable.ImmutableHashSet<TSource> ToImmutableHashSet<TSource>(this System.Collections.Generic.IEnumerable<TSource> source, System.Collections.Generic.IEqualityComparer<TSource> equalityComparer) { throw null; }
public static System.Collections.Immutable.ImmutableHashSet<TSource> ToImmutableHashSet<TSource>(this System.Collections.Immutable.ImmutableHashSet<TSource>.Builder builder) { throw null; }
}
public sealed partial class ImmutableHashSet<T> : System.Collections.Generic.ICollection<T>, System.Collections.Generic.IEnumerable<T>, System.Collections.Generic.IReadOnlyCollection<T>, System.Collections.Generic.ISet<T>, System.Collections.ICollection, System.Collections.IEnumerable, System.Collections.Immutable.IImmutableSet<T>
{
Expand Down Expand Up @@ -522,6 +525,7 @@ public static partial class ImmutableList
public static System.Collections.Immutable.IImmutableList<T> Remove<T>(this System.Collections.Immutable.IImmutableList<T> list, T value) { throw null; }
public static System.Collections.Immutable.IImmutableList<T> Replace<T>(this System.Collections.Immutable.IImmutableList<T> list, T oldValue, T newValue) { throw null; }
public static System.Collections.Immutable.ImmutableList<TSource> ToImmutableList<TSource>(this System.Collections.Generic.IEnumerable<TSource> source) { throw null; }
public static System.Collections.Immutable.ImmutableList<TSource> ToImmutableList<TSource>(this System.Collections.Immutable.ImmutableList<TSource>.Builder builder) { throw null; }
}
public sealed partial class ImmutableList<T> : System.Collections.Generic.ICollection<T>, System.Collections.Generic.IEnumerable<T>, System.Collections.Generic.IList<T>, System.Collections.Generic.IReadOnlyCollection<T>, System.Collections.Generic.IReadOnlyList<T>, System.Collections.ICollection, System.Collections.IEnumerable, System.Collections.IList, System.Collections.Immutable.IImmutableList<T>
{
Expand Down Expand Up @@ -746,6 +750,7 @@ public static partial class ImmutableSortedDictionary
public static System.Collections.Immutable.ImmutableSortedDictionary<TKey, TValue> ToImmutableSortedDictionary<TSource, TKey, TValue>(this System.Collections.Generic.IEnumerable<TSource> source, System.Func<TSource, TKey> keySelector, System.Func<TSource, TValue> elementSelector) { throw null; }
public static System.Collections.Immutable.ImmutableSortedDictionary<TKey, TValue> ToImmutableSortedDictionary<TSource, TKey, TValue>(this System.Collections.Generic.IEnumerable<TSource> source, System.Func<TSource, TKey> keySelector, System.Func<TSource, TValue> elementSelector, System.Collections.Generic.IComparer<TKey> keyComparer) { throw null; }
public static System.Collections.Immutable.ImmutableSortedDictionary<TKey, TValue> ToImmutableSortedDictionary<TSource, TKey, TValue>(this System.Collections.Generic.IEnumerable<TSource> source, System.Func<TSource, TKey> keySelector, System.Func<TSource, TValue> elementSelector, System.Collections.Generic.IComparer<TKey> keyComparer, System.Collections.Generic.IEqualityComparer<TValue> valueComparer) { throw null; }
public static System.Collections.Immutable.ImmutableSortedDictionary<TKey, TValue> ToImmutableSortedDictionary<TKey, TValue>(this System.Collections.Immutable.ImmutableSortedDictionary<TKey, TValue>.Builder source) { throw null; }
}
public sealed partial class ImmutableSortedDictionary<TKey, TValue> : System.Collections.Generic.ICollection<System.Collections.Generic.KeyValuePair<TKey, TValue>>, System.Collections.Generic.IDictionary<TKey, TValue>, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<TKey, TValue>>, System.Collections.Generic.IReadOnlyCollection<System.Collections.Generic.KeyValuePair<TKey, TValue>>, System.Collections.Generic.IReadOnlyDictionary<TKey, TValue>, System.Collections.ICollection, System.Collections.IDictionary, System.Collections.IEnumerable, System.Collections.Immutable.IImmutableDictionary<TKey, TValue>
{
Expand Down Expand Up @@ -881,6 +886,7 @@ public static partial class ImmutableSortedSet
public static System.Collections.Immutable.ImmutableSortedSet<T> Create<T>(params T[] items) { throw null; }
public static System.Collections.Immutable.ImmutableSortedSet<TSource> ToImmutableSortedSet<TSource>(this System.Collections.Generic.IEnumerable<TSource> source) { throw null; }
public static System.Collections.Immutable.ImmutableSortedSet<TSource> ToImmutableSortedSet<TSource>(this System.Collections.Generic.IEnumerable<TSource> source, System.Collections.Generic.IComparer<TSource> comparer) { throw null; }
public static System.Collections.Immutable.ImmutableSortedSet<TSource> ToImmutableSortedSet<TSource>(this System.Collections.Immutable.ImmutableSortedSet<TSource>.Builder builder) { throw null; }
}
public sealed partial class ImmutableSortedSet<T> : System.Collections.Generic.ICollection<T>, System.Collections.Generic.IEnumerable<T>, System.Collections.Generic.IList<T>, System.Collections.Generic.IReadOnlyCollection<T>, System.Collections.Generic.IReadOnlyList<T>, System.Collections.Generic.ISet<T>, System.Collections.ICollection, System.Collections.IEnumerable, System.Collections.IList, System.Collections.Immutable.IImmutableSet<T>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,19 @@ public static ImmutableArray<TSource> ToImmutableArray<TSource>(this IEnumerable
return CreateRange(items);
}

/// <summary>
/// Returns an immutable copy of the current contents of the builder's collection.
/// </summary>
/// <param name="builder">The builder to create the immutable array from.</param>
/// <returns>An immutable array.</returns>
[Pure]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that [Pure] has no runtime impact, why isn't the attribute included in the ref assemblies so that analyzers can do their work based on it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at it (and not the author) I'd guess that it's because none of the other definitions in the ref assembly have them and he followed the existing pattern on the assumption that someone in the past had good reason to do this.

Adding them in ref would require a dependency on System.Diagnostics.Contracts which would seem an odd thing to do.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed: this PR itself looks consistent with precedent. So my question was perhaps more directed at the authors of the ref assemblies in general. @danmosemsft @csharpfritz perhaps?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was included incorrectly on this...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Pure] is part of Contracts, right? We don't use or enforce those and generally have been removing contract annotations. I believe we discussed this before in the context of immutable collections in fact and @AArnott did you indicate you didn't believe in them either? In which case I guess these can all be removed unless they are somehow still useful to .NET Framework consumers even though they're not on the ref assembly and probably not consistent either.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me we likely want an analyzer proving it adds value first.

Right. Based on the past experiments with Code Contracts, it is very hard to get a return on investment with this. The annotations do catch a few bugs, but they require a prohibitive amount of manual work to do that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annotations aside, if the compiler included an analyzer with a hard coded list of most commonly misused pure functions (even just those on String) it would provide non zero value with ?no false positives. 😃

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the past experiments with Code Contracts, it is very hard to get a return on investment with this.

Code Contracts preceded roslyn analyzers, and required custom build steps, VS extensions, and/or post-compilation IL-rewriting . Roslyn analyzers are proven to be effective and popular. Let's not let Code Contracts' failure deter us from tapping Analyzers for solving this simple issue.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Pure attribute is checked by Rule CA1806 in Microsoft.CodeQuality.Analyzers.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for redrawing attention to this, @Grauenwolf. I've filed a separate issue to track the issue: https://github.com/dotnet/corefx/issues/34778

public static ImmutableArray<TSource> ToImmutableArray<TSource>(this ImmutableArray<TSource>.Builder builder)
{
Requires.NotNull(builder, nameof(builder));

return builder.ToImmutable();
}

/// <summary>
/// Searches an entire one-dimensional sorted <see cref="ImmutableArray{T}"/> for a specific element,
/// using the <see cref="IComparable{T}"/> generic interface implemented by each element
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,19 @@ public static ImmutableDictionary<TKey, TValue> ToImmutableDictionary<TSource, T
.AddRange(source.Select(element => new KeyValuePair<TKey, TValue>(keySelector(element), elementSelector(element))));
}

/// <summary>
/// Returns an immutable copy of the current contents of the builder's collection.
/// </summary>
/// <param name="builder">The builder to create the immutable dictionary from.</param>
/// <returns>An immutable dictionary.</returns>
[Pure]
public static ImmutableDictionary<TKey, TValue> ToImmutableDictionary<TKey, TValue>(this ImmutableDictionary<TKey, TValue>.Builder builder)
{
Requires.NotNull(builder, nameof(builder));

return builder.ToImmutable();
}

/// <summary>
/// Constructs an immutable dictionary based on some transformation of a sequence.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,20 @@ public static ImmutableHashSet<TSource> ToImmutableHashSet<TSource>(this IEnumer
return ImmutableHashSet<TSource>.Empty.WithComparer(equalityComparer).Union(source);
}

/// <summary>
/// Returns an immutable copy of the current contents of the builder's collection.
/// </summary>
/// <param name="builder">The builder to create the immutable set from.</param>
/// <returns>An immutable set.</returns>
[Pure]
public static ImmutableHashSet<TSource> ToImmutableHashSet<TSource>(this ImmutableHashSet<TSource>.Builder builder)
{
Requires.NotNull(builder, nameof(builder));

return builder.ToImmutable();
}


/// <summary>
/// Enumerates a sequence exactly once and produces an immutable set of its contents.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,19 @@ public static ImmutableList<TSource> ToImmutableList<TSource>(this IEnumerable<T
return ImmutableList<TSource>.Empty.AddRange(source);
}

/// <summary>
/// Returns an immutable copy of the current contents of the builder's collection.
/// </summary>
/// <param name="builder">The builder to create the immutable list from.</param>
/// <returns>An immutable list.</returns>
[Pure]
public static ImmutableList<TSource> ToImmutableList<TSource>(this ImmutableList<TSource>.Builder builder)
{
Requires.NotNull(builder, nameof(builder));

return builder.ToImmutable();
}

/// <summary>
/// Replaces the first equal element in the list with the specified element.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,19 @@ public static ImmutableSortedDictionary<TKey, TValue> ToImmutableSortedDictionar
.AddRange(source.Select(element => new KeyValuePair<TKey, TValue>(keySelector(element), elementSelector(element))));
}

/// <summary>
/// Returns an immutable copy of the current contents of the builder's collection.
/// </summary>
/// <param name="builder">The builder to create the immutable map from.</param>
/// <returns>An immutable map.</returns>
[Pure]
public static ImmutableSortedDictionary<TKey, TValue> ToImmutableSortedDictionary<TKey, TValue>(this ImmutableSortedDictionary<TKey, TValue>.Builder builder)
{
Requires.NotNull(builder, nameof(builder));

return builder.ToImmutable();
}

/// <summary>
/// Constructs an immutable sorted dictionary based on some transformation of a sequence.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,5 +167,18 @@ public static ImmutableSortedSet<TSource> ToImmutableSortedSet<TSource>(this IEn
{
return ToImmutableSortedSet(source, null);
}

/// <summary>
/// Returns an immutable copy of the current contents of the builder's collection.
/// </summary>
/// <param name="builder">The builder to create the immutable set from.</param>
/// <returns>An immutable set.</returns>
[Pure]
public static ImmutableSortedSet<TSource> ToImmutableSortedSet<TSource>(this ImmutableSortedSet<TSource>.Builder builder)
{
Requires.NotNull(builder, nameof(builder));

return builder.ToImmutable();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,29 @@ public void ToImmutable()
Assert.True(builder.ToImmutable().IsEmpty);
}

[Fact]
public void ToImmutableArray()
{
var builder = new ImmutableArray<int>.Builder();
builder.AddRange(0, 1, 2);

var array = builder.ToImmutableArray();
Assert.Equal(0, array[0]);
Assert.Equal(1, array[1]);
Assert.Equal(2, array[2]);

builder[1] = 5;
Assert.Equal(5, builder[1]);
Assert.Equal(1, array[1]);

builder.Clear();
Assert.True(builder.ToImmutableArray().IsEmpty);
Assert.False(array.IsEmpty);

ImmutableArray<int>.Builder nullBuilder = null;
AssertExtensions.Throws<ArgumentNullException>("builder", () => nullBuilder.ToImmutableArray());
}

[Fact]
public void CopyTo()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,31 @@ public static void TestDebuggerAttributes_Null()
Assert.IsType<ArgumentNullException>(tie.InnerException);
}

[Fact]
public void ToImmutableDictionary()
{
ImmutableDictionary<int, int>.Builder builder = ImmutableDictionary.CreateBuilder<int, int>();
builder.Add(0, 0);
builder.Add(1, 1);
builder.Add(2, 2);

var dictionary = builder.ToImmutableDictionary();
Assert.Equal(0, dictionary[0]);
Assert.Equal(1, dictionary[1]);
Assert.Equal(2, dictionary[2]);

builder[1] = 5;
Assert.Equal(5, builder[1]);
Assert.Equal(1, dictionary[1]);

builder.Clear();
Assert.True(builder.ToImmutableDictionary().IsEmpty);
Assert.False(dictionary.IsEmpty);

ImmutableDictionary<int, int>.Builder nullBuilder = null;
AssertExtensions.Throws<ArgumentNullException>("builder", () => nullBuilder.ToImmutableDictionary());
}

protected override IImmutableDictionary<TKey, TValue> GetEmptyImmutableDictionary<TKey, TValue>()
{
return ImmutableDictionary.Create<TKey, TValue>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,5 +308,30 @@ public void DebuggerAttributesValid()
{
DebuggerAttributes.ValidateDebuggerDisplayReferences(ImmutableHashSet.CreateBuilder<int>());
}

[Fact]
public void ToImmutableHashSet()
{
ImmutableHashSet<int>.Builder builder = ImmutableHashSet.CreateBuilder<int>();
builder.Add(1);
builder.Add(2);
builder.Add(3);

var set = builder.ToImmutableSortedSet();
Assert.True(builder.Contains(1));
Assert.True(builder.Contains(2));
Assert.True(builder.Contains(3));

builder.Remove(3);
Assert.False(builder.Contains(3));
Assert.True(set.Contains(3));

builder.Clear();
Assert.True(builder.ToImmutableHashSet().IsEmpty);
Assert.False(set.IsEmpty);

ImmutableHashSet<int>.Builder nullBuilder = null;
AssertExtensions.Throws<ArgumentNullException>("builder", () => nullBuilder.ToImmutableHashSet());
}
}
}
25 changes: 25 additions & 0 deletions src/System.Collections.Immutable/tests/ImmutableListBuilderTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,31 @@ public void ItemRef_OutOfBounds()
Assert.Throws<ArgumentOutOfRangeException>(() => builder.ItemRef(5));
}

[Fact]
public void ToImmutableList()
{
ImmutableList<int>.Builder builder = ImmutableList.CreateBuilder<int>();
builder.Add(0);
builder.Add(1);
builder.Add(2);

var list = builder.ToImmutableList();
Assert.Equal(0, builder[0]);
Assert.Equal(1, builder[1]);
Assert.Equal(2, builder[2]);

builder[1] = 5;
Assert.Equal(5, builder[1]);
Assert.Equal(1, list[1]);

builder.Clear();
Assert.True(builder.ToImmutableList().IsEmpty);
Assert.False(list.IsEmpty);

ImmutableList<int>.Builder nullBuilder = null;
AssertExtensions.Throws<ArgumentNullException>("builder", () => nullBuilder.ToImmutableList());
}

protected override IEnumerable<T> GetEnumerableOf<T>(params T[] contents)
{
return ImmutableList<T>.Empty.AddRange(contents).ToBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,31 @@ public void ValueRef_NonExistentKey()
Assert.Throws<KeyNotFoundException>(() => builder.ValueRef("c"));
}

[Fact]
public void ToImmutableSortedDictionary()
{
ImmutableSortedDictionary<int, int>.Builder builder = ImmutableSortedDictionary.CreateBuilder<int, int>();
builder.Add(1, 1);
builder.Add(2, 2);
builder.Add(3, 3);

var dictionary = builder.ToImmutableSortedDictionary();
Assert.Equal(1, dictionary[1]);
Assert.Equal(2, dictionary[2]);
Assert.Equal(3, dictionary[3]);

builder[2] = 5;
Assert.Equal(5, builder[2]);
Assert.Equal(2, dictionary[2]);

builder.Clear();
Assert.True(builder.ToImmutableSortedDictionary().IsEmpty);
Assert.False(dictionary.IsEmpty);

ImmutableSortedDictionary<int, int>.Builder nullBuilder = null;
AssertExtensions.Throws<ArgumentNullException>("builder", () => nullBuilder.ToImmutableSortedDictionary());
}

protected override IImmutableDictionary<TKey, TValue> GetEmptyImmutableDictionary<TKey, TValue>()
{
return ImmutableSortedDictionary.Create<TKey, TValue>();
Expand Down
Loading