[release/7.0-rc2] Fix Configuration.Binder for collection properties with no setters

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

@@ -299,43 +299,45 @@ 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
+                // 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))
                 {
                     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 (TypeIsASetInterface(type))
+                // for sets and read-only set interfaces, we clone what's there into a new collection, if we can
+                if (TypeIsASetInterface(type) && !bindingPoint.IsReadOnly)
                 {
-                    if (!bindingPoint.IsReadOnly)
+                    object? newValue = BindSet(type, (IEnumerable?)bindingPoint.Value, config, options);
+                    if (newValue != null)
                     {
-                        object? newValue = BindSet(type, (IEnumerable?)bindingPoint.Value, config, options);
-                        if (newValue != null)
-                        {
-                            bindingPoint.SetValue(newValue);
-                        }
+                        bindingPoint.SetValue(newValue);
                     }
+
                     return;
                 }
 
                 // For other mutable interfaces like ICollection<>, IDictionary<,> 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.
-                if (TypeIsADictionaryInterface(type))
+                if (TypeIsADictionaryInterface(type) && !bindingPoint.IsReadOnly)
                 {
-                    if (!bindingPoint.IsReadOnly)
+                    object? newValue = BindDictionaryInterface(bindingPoint.Value, type, config, options);
+                    if (newValue != null)
                     {
-                        object? newValue = BindDictionaryInterface(bindingPoint.Value, type, config, options);
-                        if (newValue != null)
-                        {
-                            bindingPoint.SetValue(newValue);
-                        }
+                        bindingPoint.SetValue(newValue);
                     }
+
                     return;
                 }
 
@@ -848,6 +850,16 @@ private static bool IsArrayCompatibleInterface(Type type)
                 || genericTypeDefinition == typeof(IReadOnlyList<>);
         }
 
+        private static bool IsImmutableArrayCompatibleInterface(Type type)
+        {
+            if (!type.IsInterface || !type.IsConstructedGenericType) { return false; }
+
+            Type genericTypeDefinition = type.GetGenericTypeDefinition();
+            return genericTypeDefinition == typeof(IEnumerable<>)
+                || genericTypeDefinition == typeof(IReadOnlyCollection<>)
+                || genericTypeDefinition == typeof(IReadOnlyList<>);
+        }
+
         private static bool TypeIsASetInterface(Type type)
         {
             if (!type.IsInterface || !type.IsConstructedGenericType) { return false; }

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

@@ -72,6 +72,8 @@ public string ReadOnly
 
             public ISet<string> InstantiatedISet { get; set; } = new HashSet<string>();
 
+            public ISet<string> ISetNoSetter { get; } = new HashSet<string>();
+
             public HashSet<string> InstantiatedHashSetWithSomeValues { get; set; } =
                 new HashSet<string>(new[] {"existing1", "existing2"});
 
@@ -662,6 +664,27 @@ public void CanBindNonInstantiatedISet()
             Assert.Equal("Yo2", options.NonInstantiatedISet.ElementAt(1));
         }
 
+        [Fact]
+        public void CanBindISetNoSetter()
+        {
+            var dic = new Dictionary<string, string>
+            {
+                {"ISetNoSetter:0", "Yo1"},
+                {"ISetNoSetter:1", "Yo2"},
+                {"ISetNoSetter:2", "Yo2"},
+            };
+            var configurationBuilder = new ConfigurationBuilder();
+            configurationBuilder.AddInMemoryCollection(dic);
+
+            var config = configurationBuilder.Build();
+
+            var options = config.Get<ComplexOptions>()!;
+
+            Assert.Equal(2, options.ISetNoSetter.Count);
+            Assert.Equal("Yo1", options.ISetNoSetter.ElementAt(0));
+            Assert.Equal("Yo2", options.ISetNoSetter.ElementAt(1));
+        }
+
 #if NETCOREAPP
         [Fact]
         public void CanBindInstantiatedIReadOnlySet()

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

@@ -579,7 +579,10 @@ public void AlreadyInitializedStringDictionaryBinding()
             {
                 {"AlreadyInitializedStringDictionaryInterface:abc", "val_1"},
                 {"AlreadyInitializedStringDictionaryInterface:def", "val_2"},
-                {"AlreadyInitializedStringDictionaryInterface:ghi", "val_3"}
+                {"AlreadyInitializedStringDictionaryInterface:ghi", "val_3"},
+
+                {"IDictionaryNoSetter:Key1", "Value1"},
+                {"IDictionaryNoSetter:Key2", "Value2"},
             };
 
             var configurationBuilder = new ConfigurationBuilder();
@@ -596,6 +599,10 @@ public void AlreadyInitializedStringDictionaryBinding()
             Assert.Equal("val_1", options.AlreadyInitializedStringDictionaryInterface["abc"]);
             Assert.Equal("val_2", options.AlreadyInitializedStringDictionaryInterface["def"]);
             Assert.Equal("val_3", options.AlreadyInitializedStringDictionaryInterface["ghi"]);
+
+            Assert.Equal(2, options.IDictionaryNoSetter.Count);
+            Assert.Equal("Value1", options.IDictionaryNoSetter["Key1"]);
+            Assert.Equal("Value2", options.IDictionaryNoSetter["Key2"]);
         }
 
         [Fact]
@@ -1059,7 +1066,10 @@ public void CanBindInitializedIEnumerableAndTheOriginalItemsAreNotMutated()
                 {"AlreadyInitializedIEnumerableInterface:0", "val0"},
                 {"AlreadyInitializedIEnumerableInterface:1", "val1"},
                 {"AlreadyInitializedIEnumerableInterface:2", "val2"},
-                {"AlreadyInitializedIEnumerableInterface:x", "valx"}
+                {"AlreadyInitializedIEnumerableInterface:x", "valx"},
+
+                {"ICollectionNoSetter:0", "val0"},
+                {"ICollectionNoSetter:1", "val1"},
             };
 
             var configurationBuilder = new ConfigurationBuilder();
@@ -1084,6 +1094,10 @@ public void CanBindInitializedIEnumerableAndTheOriginalItemsAreNotMutated()
             Assert.Equal(2, options.ListUsedInIEnumerableFieldAndShouldNotBeTouched.Count);
             Assert.Equal("This was here too", options.ListUsedInIEnumerableFieldAndShouldNotBeTouched.ElementAt(0));
             Assert.Equal("Don't touch me!", options.ListUsedInIEnumerableFieldAndShouldNotBeTouched.ElementAt(1));
+
+            Assert.Equal(2, options.ICollectionNoSetter.Count);
+            Assert.Equal("val0", options.ICollectionNoSetter.ElementAt(0));
+            Assert.Equal("val1", options.ICollectionNoSetter.ElementAt(1));
         }
 
         [Fact]
@@ -1424,6 +1438,8 @@ public InitializedCollectionsOptions()
                 new CustomListIndirectlyDerivedFromIEnumerable();
 
             public IReadOnlyDictionary<string, string> AlreadyInitializedDictionary { get; set; }
+
+            public ICollection<string> ICollectionNoSetter { get; } = new List<string>();
         }
 
         private class CustomList : List<string>
@@ -1564,6 +1580,8 @@ public OptionsWithDictionary()
 
             public Dictionary<string, string> StringDictionary { get; set; }
 
+            public IDictionary<string, string> IDictionaryNoSetter { get; } = new Dictionary<string, string>();
+
             public Dictionary<string, NestedOptions> ObjectDictionary { get; set; }
 
             public Dictionary<string, ISet<string>> ISetDictionary { get; set; }