issue 56835 make ordered dictionary kv collection implement i list (d…

…otnet#60060)

* issue-56835 Initial commit: make OrderedDictionaryKeyValueCollection implements IList.

* issue-56835 Use GetReadOnlyMessage() to give detailed (Keys/Values) readonly exception message.

* Fix comment: reuse existing wording to describe not supported error message.

* Add tests for IList implementation of KeyValueCollection.

* issue-56835: rename isKeysProperty to testKeysProperty.

* issue-56835: Fix comment: reduce code size; use Assert.IsAssignableFrom().

* Fix comment: remove small OrderedDictionaryKeyValueCollection ctor; Ensure initializing Hashtable field before construct keyed OrderedDictionaryKeyValueCollection.

* Fix comment: remove field '_isKeys'.

src/libraries/System.Collections.Specialized/src/Resources/Strings.resx

@@ -96,4 +96,10 @@
   <data name="Serialization_InvalidOnDeser" xml:space="preserve">
     <value>OnDeserialization method was called while the object was not being deserialized.</value>
   </data>
+  <data name="NotSupported_KeyCollectionSet" xml:space="preserve">
+    <value>Mutating a key collection derived from a dictionary is not allowed.</value>
+  </data>
+  <data name="NotSupported_ValueCollectionSet" xml:space="preserve">
+    <value>Mutating a value collection derived from a dictionary is not allowed.</value>
+  </data>
 </root>

src/libraries/System.Collections.Specialized/src/System/Collections/Specialized/OrderedDictionary.cs

@@ -128,7 +128,8 @@ public ICollection Keys
             get
             {
                 ArrayList objectsArray = EnsureObjectsArray();
-                return new OrderedDictionaryKeyValueCollection(objectsArray, true);
+                Hashtable objectsTable = EnsureObjectsTable();
+                return new OrderedDictionaryKeyValueCollection(objectsArray, objectsTable, _comparer);
             }
         }
 
@@ -210,7 +211,7 @@ public ICollection Values
             get
             {
                 ArrayList objectsArray = EnsureObjectsArray();
-                return new OrderedDictionaryKeyValueCollection(objectsArray, false);
+                return new OrderedDictionaryKeyValueCollection(objectsArray);
             }
         }
 
@@ -541,21 +542,30 @@ public void Reset()
         }
 
         /// <devdoc>
-        /// OrderedDictionaryKeyValueCollection implements a collection for the Values and Keys properties
+        /// OrderedDictionaryKeyValueCollection implements IList for the Values and Keys properties
         /// that is "live"- it will reflect changes to the OrderedDictionary on the collection made after the getter
         /// was called.
         /// </devdoc>
-        private sealed class OrderedDictionaryKeyValueCollection : ICollection
+        private sealed class OrderedDictionaryKeyValueCollection : IList
         {
             private readonly ArrayList _objects;
-            private readonly bool _isKeys;
+            private readonly Hashtable? _objectsTable;
+            private readonly IEqualityComparer? _comparer;
+
+            public OrderedDictionaryKeyValueCollection(ArrayList array, Hashtable objectsTable, IEqualityComparer? comparer)
+            {
+                _objects = array;
+                _objectsTable = objectsTable;
+                _comparer = comparer;
+            }
 
-            public OrderedDictionaryKeyValueCollection(ArrayList array, bool isKeys)
+            public OrderedDictionaryKeyValueCollection(ArrayList array)
             {
                 _objects = array;
-                _isKeys = isKeys;
             }
 
+            private bool IsKeys => _objectsTable is not null;
+
             void ICollection.CopyTo(Array array, int index)
             {
                 if (array == null)
@@ -565,38 +575,111 @@ void ICollection.CopyTo(Array array, int index)
                 foreach (object? o in _objects)
                 {
                     Debug.Assert(o != null);
-                    array.SetValue(_isKeys ? ((DictionaryEntry)o).Key : ((DictionaryEntry)o).Value, index);
+                    array.SetValue(IsKeys ? ((DictionaryEntry)o).Key : ((DictionaryEntry)o).Value, index);
                     index++;
                 }
             }
 
-            int ICollection.Count
+            int ICollection.Count => _objects.Count;
+
+            bool ICollection.IsSynchronized => false;
+
+            object ICollection.SyncRoot => _objects.SyncRoot;
+
+            IEnumerator IEnumerable.GetEnumerator()
             {
-                get
+                return new OrderedDictionaryEnumerator(_objects, IsKeys ? OrderedDictionaryEnumerator.Keys : OrderedDictionaryEnumerator.Values);
+            }
+
+            bool IList.Contains(object? value)
+            {
+                if (IsKeys)
+                {
+                    Debug.Assert(_objectsTable is not null);
+                    return value != null && _objectsTable.ContainsKey(value);
+                }
+
+                foreach (object? o in _objects)
                 {
-                    return _objects.Count;
+                    Debug.Assert(o != null);
+                    if (object.Equals(((DictionaryEntry)o).Value, value))
+                    {
+                        return true;
+                    }
                 }
+
+                return false;
             }
 
-            bool ICollection.IsSynchronized
+            int IList.IndexOf(object? value)
             {
-                get
+                for (int i = 0; i < _objects.Count; i++)
                 {
-                    return false;
+                    if (IsKeys)
+                    {
+                        object entryKey = ((DictionaryEntry)_objects[i]!).Key;
+                        if (_comparer != null)
+                        {
+                            if (_comparer.Equals(entryKey, value))
+                            {
+                                return i;
+                            }
+                        }
+                        else if (entryKey.Equals(value))
+                        {
+                            return i;
+                        }
+                    }
+                    else if (object.Equals(((DictionaryEntry)_objects[i]!).Value, value))
+                    {
+                        return i;
+                    }
                 }
+
+                return -1;
             }
 
-            object ICollection.SyncRoot
+            bool IList.IsFixedSize => true;
+            bool IList.IsReadOnly => true;
+
+            object? IList.this[int index]
             {
                 get
                 {
-                    return _objects.SyncRoot;
+                    DictionaryEntry entry = (DictionaryEntry)_objects[index]!;
+                    return IsKeys ? entry.Key : entry.Value;
                 }
+                set => throw new NotSupportedException(GetNotSupportedErrorMessage());
             }
 
-            IEnumerator IEnumerable.GetEnumerator()
+            void IList.Insert(int index, object? value)
+            {
+                throw new NotSupportedException(GetNotSupportedErrorMessage());
+            }
+
+            void IList.Remove(object? value)
+            {
+                throw new NotSupportedException(GetNotSupportedErrorMessage());
+            }
+
+            void IList.RemoveAt(int index)
+            {
+                throw new NotSupportedException(GetNotSupportedErrorMessage());
+            }
+
+            int IList.Add(object? value)
+            {
+                throw new NotSupportedException(GetNotSupportedErrorMessage());
+            }
+
+            void IList.Clear()
+            {
+                throw new NotSupportedException(GetNotSupportedErrorMessage());
+            }
+
+            private string GetNotSupportedErrorMessage()
             {
-                return new OrderedDictionaryEnumerator(_objects, _isKeys == true ? OrderedDictionaryEnumerator.Keys : OrderedDictionaryEnumerator.Values);
+                return IsKeys ? SR.NotSupported_KeyCollectionSet : SR.NotSupported_ValueCollectionSet;
             }
         }
     }

src/libraries/System.Collections.Specialized/tests/OrderedDictionary/OrderedDictionaryTests.cs

@@ -563,5 +563,81 @@ public void RemoveAtTests()
                 Assert.Equal(1000 - i - 1, d.Count);
             }
         }
+
+        // (IList)Keys { get; }
+        // (IList)Values { get; }
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void KeysAndValuesPropertiesImplementIList(bool testKeysProperty)
+        {
+            var orderedDictionary = new OrderedDictionary();
+            var itemCount = 1000;
+            string prefix = testKeysProperty ? "k" : "v";
+            for (int i = 0; i < itemCount; i++)
+            {
+                string item = prefix + i;
+                orderedDictionary.Add(item, item);
+            }
+
+            IList list = Assert.IsAssignableFrom<IList>(testKeysProperty ? orderedDictionary.Keys : orderedDictionary.Values);
+
+            Assert.True(list.IsFixedSize);
+            Assert.True(list.IsReadOnly);
+
+            for (int i = 0; i < itemCount; i++)
+            {
+                string item = prefix + i;
+                Assert.Equal(item, list[i]);
+                Assert.True(list.Contains(item));
+                Assert.Equal(i, list.IndexOf(item));
+            }
+
+            Assert.Throws<ArgumentOutOfRangeException>(() => list[-1]);
+            Assert.Throws<ArgumentOutOfRangeException>(() => list[itemCount]);
+            Assert.False(list.Contains(prefix + itemCount));
+            Assert.False(list.Contains(null));
+            Assert.Equal(-1, list.IndexOf(prefix + itemCount));
+            Assert.Equal(-1, list.IndexOf(null));
+
+            if (!testKeysProperty)
+            {
+                orderedDictionary.Add(prefix + itemCount, null);
+                Assert.True(list.Contains(null));
+                Assert.Equal(itemCount, list.IndexOf(null));
+            }
+        }
+
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void KeysAndValuesPropertiesNotSupportWritableIList(bool testKeysProperty)
+        {
+            var orderedDictionary = new OrderedDictionary();
+            orderedDictionary.Add("foo", "bar");
+            IList list = Assert.IsAssignableFrom<IList>(testKeysProperty ? orderedDictionary.Keys : orderedDictionary.Values);
+
+            Assert.Throws<NotSupportedException>(() =>
+            {
+                list[0] = "a";
+            });
+            Assert.Throws<NotSupportedException>(() => list.Add("a"));
+            Assert.Throws<NotSupportedException>(() => list.Clear());
+            Assert.Throws<NotSupportedException>(() => list.Insert(0, "a"));
+            Assert.Throws<NotSupportedException>(() => list.Remove("a"));
+            Assert.Throws<NotSupportedException>(() => list.RemoveAt(0));
+        }
+
+        [Fact]
+        public void IListedKeysPropertyCanUseCustomEqualityComparer()
+        {
+            var eqComp = new CaseInsensitiveEqualityComparer();
+            var orderedDictionary = new OrderedDictionary(eqComp);
+            orderedDictionary.Add("KeY", null);
+
+            IList list = (IList)orderedDictionary.Keys;
+            Assert.True(list.Contains("key"));
+            Assert.Equal(0, list.IndexOf("key"));
+        }
     }
 }