Cleanup ImmutableArray tests

src/Common/tests/System/Collections/DelegateEqualityComparer.cs

@@ -0,0 +1,36 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Collections.Generic;
+
+namespace System.Collections.Tests
+{
+    internal sealed class DelegateEqualityComparer<T> : IEqualityComparer<T>, IEqualityComparer
+    {
+        private readonly Func<T, T, bool> _equals;
+        private readonly Func<T, int> _getHashCode;
+        private readonly Func<object, object, bool> _objectEquals;
+        private readonly Func<object, int> _objectGetHashCode;
+
+        public DelegateEqualityComparer(
+            Func<T, T, bool> equals = null,
+            Func<T, int> getHashCode = null,
+            Func<object, object, bool> objectEquals = null,
+            Func<object, int> objectGetHashCode = null)
+        {
+            _equals = equals ?? ((x, y) => { throw new NotImplementedException(); });
+            _getHashCode = getHashCode ?? (obj => { throw new NotImplementedException(); });
+            _objectEquals = objectEquals ?? ((x, y) => { throw new NotImplementedException(); });
+            _objectGetHashCode = objectGetHashCode ?? (obj => { throw new NotImplementedException(); });
+        }
+
+        public bool Equals(T x, T y) => _equals(x, y);
+
+        public int GetHashCode(T obj) => _getHashCode(obj);
+
+        bool IEqualityComparer.Equals(object x, object y) => _objectEquals(x, y);
+
+        int IEqualityComparer.GetHashCode(object obj) => _objectGetHashCode(obj);
+    }
+}

src/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.cs

@@ -170,7 +170,7 @@ public bool IsEmpty
         }
 
         /// <summary>
-        /// Gets the number of array in the collection.
+        /// Gets the number of elements in the array.
         /// </summary>
         [DebuggerBrowsable(DebuggerBrowsableState.Never)]
         public int Length
@@ -728,6 +728,9 @@ public ImmutableArray<T> Replace(T oldValue, T newValue)
         public ImmutableArray<T> Replace(T oldValue, T newValue, IEqualityComparer<T> equalityComparer)
         {
             var self = this;
+            // TODO: This is calling the IImmutableList<T> overload which is silently boxing the ImmutableArray<T>.
+            // It's also causing inconsistencies where calling Replace on an IsDefault instance will throw an
+            // InvalidOperationException rather than a NullReferenceException.
             int index = self.IndexOf(oldValue, equalityComparer);
             if (index < 0)
             {
@@ -764,6 +767,7 @@ public ImmutableArray<T> Remove(T item, IEqualityComparer<T> equalityComparer)
         {
             var self = this;
             self.ThrowNullRefIfNotInitialized();
+            // TODO: This is silently boxing.
             int index = self.IndexOf(item, equalityComparer);
             return index < 0
                 ? self
@@ -790,6 +794,12 @@ public ImmutableArray<T> RemoveAt(int index)
         [Pure]
         public ImmutableArray<T> RemoveRange(int index, int length)
         {
+            // TODO: There is an inconsistency here between what type of exception will be thrown
+            // if we are an IsDefault instance.
+            // If the index is less than 0, the && below will short-circuit, and self.Length will
+            // not be evaluated. Then we will get an ArgumentOutOfRangeException.
+            // Otherwise, we will get a NullReferenceException when trying to evaluate self.Length.
+
             var self = this;
             Requires.Range(index >= 0 && index <= self.Length, nameof(index));
             Requires.Range(length >= 0 && index + length <= self.Length, nameof(length));