Fix Configuration with IList and ICollection (#77857)

src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs

@@ -299,20 +299,16 @@ private static void BindInstance(
 
             if (config != null && config.GetChildren().Any())
             {
-                // for arrays, collections, and read-only list-like interfaces, we concatenate on to what is already there, if we can
-                if (type.IsArray || IsArrayCompatibleInterface(type))
+                // for arrays and read-only list-like interfaces, we concatenate on to what is already there, if we can
+                if (type.IsArray || IsImmutableArrayCompatibleInterface(type))
                 {
                     if (!bindingPoint.IsReadOnly)
                     {
                         bindingPoint.SetValue(BindArray(type, (IEnumerable?)bindingPoint.Value, config, options));
-                        return;
                     }
 
                     // for getter-only collection properties that we can't add to, nothing more we can do
-                    if (type.IsArray || IsImmutableArrayCompatibleInterface(type))
-                    {
-                        return;
-                    }
+                    return;
                 }
 
                 // for sets and read-only set interfaces, we clone what's there into a new collection, if we can
@@ -350,12 +346,19 @@ private static void BindInstance(
                         return;
                     }
 
-                    // For other mutable interfaces like ICollection<> and ISet<>, we prefer copying values and setting them
-                    // on a new instance of the interface over populating the existing instance implementing the interface.
-                    // This has already been done, so there's not need to check again. For dictionaries, we fill the existing
-                    // instance if there is one (which hasn't happened yet), and only create a new instance if necessary.
+                    Type? interfaceGenericType = type.IsInterface && type.IsConstructedGenericType ?  type.GetGenericTypeDefinition() : null;
 
-                    bindingPoint.SetValue(CreateInstance(type, config, options));
+                    if (interfaceGenericType is not null &&
+                        (interfaceGenericType == typeof(ICollection<>) || interfaceGenericType == typeof(IList<>)))
+                    {
+                        // For ICollection<T> and IList<T> we bind them to mutable List<T> type.
+                        Type genericType = typeof(List<>).MakeGenericType(type.GenericTypeArguments[0]);
+                        bindingPoint.SetValue(Activator.CreateInstance(genericType));
+                    }
+                    else
+                    {
+                        bindingPoint.SetValue(CreateInstance(type, config, options));
+                    }
                 }
 
                 // At this point we know that we have a non-null bindingPoint.Value, we just have to populate the items
@@ -584,11 +587,14 @@ private static void BindConcreteDictionary(
                 return;
             }
 
-            MethodInfo tryGetValue = dictionaryType.GetMethod("TryGetValue", BindingFlags.Public | BindingFlags.Instance)!;
-
             Debug.Assert(dictionary is not null);
+
+            Type dictionaryObjectType = dictionary.GetType();
+
+            MethodInfo tryGetValue = dictionaryObjectType.GetMethod("TryGetValue", BindingFlags.Public | BindingFlags.Instance)!;
+
             // dictionary should be of type Dictionary<,> or of type implementing IDictionary<,>
-            PropertyInfo? setter = dictionary.GetType().GetProperty("Item", BindingFlags.Public | BindingFlags.Instance);
+            PropertyInfo? setter = dictionaryObjectType.GetProperty("Item", BindingFlags.Public | BindingFlags.Instance);
             if (setter is null || !setter.CanWrite)
             {
                 // Cannot set any item on the dictionary object.
@@ -845,18 +851,6 @@ private static bool TypeIsADictionaryInterface(Type type)
                 || genericTypeDefinition == typeof(IReadOnlyDictionary<,>);
         }
 
-        private static bool IsArrayCompatibleInterface(Type type)
-        {
-            if (!type.IsInterface || !type.IsConstructedGenericType) { return false; }
-
-            Type genericTypeDefinition = type.GetGenericTypeDefinition();
-            return genericTypeDefinition == typeof(IEnumerable<>)
-                || genericTypeDefinition == typeof(ICollection<>)
-                || genericTypeDefinition == typeof(IList<>)
-                || genericTypeDefinition == typeof(IReadOnlyCollection<>)
-                || genericTypeDefinition == typeof(IReadOnlyList<>);
-        }
-
         private static bool IsImmutableArrayCompatibleInterface(Type type)
         {
             if (!type.IsInterface || !type.IsConstructedGenericType) { return false; }

src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationCollectionBindingTests.cs

@@ -393,6 +393,11 @@ public void AlreadyInitializedListInterfaceBinding()
             Assert.Equal("val1", list[2]);
             Assert.Equal("val2", list[3]);
             Assert.Equal("valx", list[4]);
+
+            // Ensure expandability of the returned list
+            options.AlreadyInitializedListInterface.Add("ExtraItem");
+            Assert.Equal(6, options.AlreadyInitializedListInterface.Count);
+            Assert.Equal("ExtraItem", options.AlreadyInitializedListInterface[5]);
         }
 
         [Fact]
@@ -1098,6 +1103,11 @@ public void CanBindInitializedIEnumerableAndTheOriginalItemsAreNotMutated()
             Assert.Equal(2, options.ICollectionNoSetter.Count);
             Assert.Equal("val0", options.ICollectionNoSetter.ElementAt(0));
             Assert.Equal("val1", options.ICollectionNoSetter.ElementAt(1));
+
+            // Ensure expandability of the returned collection
+            options.ICollectionNoSetter.Add("ExtraItem");
+            Assert.Equal(3, options.ICollectionNoSetter.Count);
+            Assert.Equal("ExtraItem", options.ICollectionNoSetter.ElementAt(2));
         }
 
         [Fact]
@@ -1218,6 +1228,11 @@ public void CanBindUninitializedICollection()
             Assert.Equal("val1", array[1]);
             Assert.Equal("val2", array[2]);
             Assert.Equal("valx", array[3]);
+
+            // Ensure expandability of the returned collection
+            options.ICollection.Add("ExtraItem");
+            Assert.Equal(5, options.ICollection.Count);
+            Assert.Equal("ExtraItem", options.ICollection.ElementAt(4));
         }
 
         [Fact]
@@ -1246,6 +1261,11 @@ public void CanBindUninitializedIList()
             Assert.Equal("val1", list[1]);
             Assert.Equal("val2", list[2]);
             Assert.Equal("valx", list[3]);
+
+            // Ensure expandability of the returned list
+            options.IList.Add("ExtraItem");
+            Assert.Equal(5, options.IList.Count);
+            Assert.Equal("ExtraItem", options.IList[4]);
         }
 
         [Fact]