diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs index b93489126c3984..eb122a2f8e85e0 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs @@ -966,17 +966,28 @@ private bool IsPropertyReboundInBindCore(PropertySpec property) case ConfigurationSectionSpec: return property.CanSet; case ComplexTypeSpec complexType: - // EmitBindImplForMember skips a complex member only when it is a - // parameterized-constructor object with no bindable members. Every other complex member is bound. - return _typeIndex.HasBindableMembers(complexType) || - complexType.IsValueType || - complexType is CollectionSpec || - complexType is not ObjectSpec { InstantiationStrategy: ObjectInstantiationStrategy.ParameterizedConstructor }; + return IsBindableAsMember(complexType, property.CanSet); default: return false; } } + /// + /// Whether binding a member of type emits any binding logic. An object + /// created through a parameterized constructor binds its constructor parameters in its Initialize method, + /// so it is bindable even without any bindable member of its own (e.g. its only member is a constructor + /// parameter backed by a read-only collection type) - but only where the member can be assigned the + /// instance that Initialize creates. One that has neither bindable members nor an instance to assign has + /// nothing to bind, and is skipped. A value-type member is bindable only when it can be set at all, since + /// binding one in place would only mutate the copy its getter returns. + /// + private bool IsBindableAsMember(ComplexTypeSpec complexType, bool canSet) => + complexType.IsValueType + ? canSet && (_typeIndex.HasBindableMembers(complexType) || _typeIndex.CanInstantiate(complexType)) + : _typeIndex.HasBindableMembers(complexType) || + complexType is not ObjectSpec { InstantiationStrategy: ObjectInstantiationStrategy.ParameterizedConstructor } || + (canSet && _typeIndex.CanInstantiate(complexType)); + private bool EmitBindImplForMember( MemberSpec member, string memberAccessExpr, @@ -1082,10 +1093,7 @@ private bool EmitBindImplForMember( case ComplexTypeSpec complexType: { // Early detection of types we cannot bind to and skip it. - if (!_typeIndex.HasBindableMembers(complexType) && - !complexType.IsValueType && - complexType is not CollectionSpec && - ((ObjectSpec)complexType).InstantiationStrategy == ObjectInstantiationStrategy.ParameterizedConstructor) + if (!IsBindableAsMember(complexType, canSet)) { return false; } @@ -1142,7 +1150,17 @@ private void EmitBindingLogicForComplexMember( return; } - Debug.Assert(canSet); + // A value type with nothing to bind beyond its constructor parameters is created outright by its + // Initialize method. There is nothing to bind in place, so assign the created instance straight to + // the member rather than through the temporary the in-place binding path below needs. + if (!_typeIndex.HasBindableMembers(effectiveMemberType) && + effectiveMemberType is ObjectSpec { InstantiationStrategy: ObjectInstantiationStrategy.ParameterizedConstructor } && + _typeIndex.CanInstantiate(effectiveMemberType)) + { + EmitObjectInit(effectiveMemberType, memberAccessExpr, InitializationKind.SimpleAssignment, configArgExpr); + return; + } + string effectiveMemberTypeFQN = effectiveMemberType.TypeRef.FullyQualifiedName; initKind = InitializationKind.None; diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Collections.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Collections.cs index b79d83e7597810..a73de27dd4d143 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Collections.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Collections.cs @@ -2479,5 +2479,25 @@ public void BindIEnumerableOfPositionalRecordWithNullProperty() Assert.Equal(2, users[1].Id); Assert.Null(users[1].Name); } + + [Fact] + public void CanBindNestedTypeWhoseSoleMemberIsAReadOnlyCollectionConstructorParameter() + { + IConfiguration config = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + { "Class:Values:0", "a" }, + { "Struct:Values:0", "b" }, + { "NullableStruct:Values:0", "c" }, + }) + .Build(); + + var options = config.Get(); + + Assert.NotNull(options); + Assert.Equal(new[] { "a" }, options.Class?.Values); + Assert.Equal(new[] { "b" }, options.Struct.Values); + Assert.Equal(new[] { "c" }, options.NullableStruct?.Values); + } } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.Collections.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.Collections.cs index 422abed9b2bbc3..c0ff29470bf736 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.Collections.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.Collections.cs @@ -515,5 +515,16 @@ public class OptionsWithPositionalRecordCollection public string? EnableFeatureX { get; set; } public List Users { get; set; } = new(); } + + public record SoleReadOnlyCollectionParamRecord(IReadOnlyList Values); + + public readonly record struct SoleReadOnlyCollectionParamStruct(IReadOnlyList Values); + + public class SoleReadOnlyCollectionParamHolder + { + public SoleReadOnlyCollectionParamRecord? Class { get; set; } + public SoleReadOnlyCollectionParamStruct Struct { get; set; } + public SoleReadOnlyCollectionParamStruct? NullableStruct { get; set; } + } } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.cs index a6874b6a91b966..1dfd100e92ac3a 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.cs @@ -553,9 +553,8 @@ public async Task SoleReadOnlyCollectionConstructorParameterIsBindable(string co // constructor parameter (no other bindable property) used to make the generator emit a // call to an Initialize method that was never generated, producing CS0103 at compile time. // - // This only covers the top-level GetCore path. Binding this same shape as a *nested* - // member (reached via BindCore/EmitObjectInit) silently produces null instead of the - // real value, a separate pre-existing bug tracked in dotnet/runtime#131399. + // This only covers the top-level GetCore path; NestedSoleReadOnlyCollectionConstructorParameterIsBindable + // covers the same shape reached as a nested member. string source = $$""" using Microsoft.Extensions.Configuration; using System.Collections.Generic; @@ -591,6 +590,160 @@ public record Options({{collectionType}} Values); Assert.Equal(new[] { "a", "b" }, boundValues); } + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))] + [InlineData("IReadOnlyList")] + [InlineData("IReadOnlyCollection")] + [InlineData("IReadOnlySet")] + [InlineData("IEnumerable")] + public async Task NestedSoleReadOnlyCollectionConstructorParameterIsBindable(string collectionType) + { + // Regression test: the same shape as SoleReadOnlyCollectionConstructorParameterIsBindable, but + // reached as a nested member rather than as the top-level bound type. The member was silently + // left at null, since the emitter skipped every complex member without bindable members - even + // one whose constructor parameters its Initialize method binds. Covers every way such a member is + // reached: a constructor parameter (bound in Initialize), a settable property (bound in BindCore), + // and a settable property with a matching constructor parameter (bound in Initialize when the + // instance is created, and in BindCore when binding an existing one). + string source = $$""" + using Microsoft.Extensions.Configuration; + using System.Collections.Generic; + + public class Program + { + public static object? Result; + + public static void Main() + { + ConfigurationBuilder configurationBuilder = new(); + configurationBuilder.AddInMemoryCollection(new Dictionary + { + ["Nested:Values:0"] = "a", + ["Nested:Values:1"] = "b", + }); + IConfiguration config = configurationBuilder.Build(); + + Outer outer = config.Get(); + Holder holder = config.Get(); + Rebindable rebindable = config.Get(); + + Rebindable existing = new(null!); + config.Bind(existing); + + Result = new object?[] + { + outer.Nested?.Values, + holder.Nested?.Values, + rebindable.Nested?.Values, + existing.Nested?.Values, + }; + } + } + + public record Inner({{collectionType}} Values); + + public record Outer(Inner Nested); + + public class Holder + { + public Inner Nested { get; set; } + } + + public class Rebindable + { + public Rebindable(Inner nested) => Nested = nested; + + public Inner Nested { get; set; } + } + """; + + ConfigBindingGenRunResult result = await RunGeneratorAndUpdateCompilation(source, assemblyReferences: GetAssemblyRefsWithAdditional(typeof(ConfigurationBuilder), typeof(List<>))); + Assert.NotNull(result.GeneratedSource); + Assert.Empty(result.Diagnostics); + + var boundValues = (object?[])LoadAndInvokeMain(result.OutputCompilation, "Result")!; + Assert.All(boundValues, boundValue => Assert.Equal(new[] { "a", "b" }, (IEnumerable?)boundValue)); + } + + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))] + [InlineData("IReadOnlyList")] + [InlineData("IReadOnlyCollection")] + [InlineData("IReadOnlySet")] + [InlineData("IEnumerable")] + public async Task NestedSoleReadOnlyCollectionConstructorParameterOfStructIsBindable(string collectionType) + { + // The value-type counterpart of NestedSoleReadOnlyCollectionConstructorParameterIsBindable. A struct + // member is bound through a temporary (binding one in place would only mutate the copy its getter + // returns), and that path never instantiated a type without bindable members - so a struct whose only + // member is a read-only collection constructor parameter was left at its default. Covers a settable + // property, a nullable one, a constructor parameter, and a settable property with a matching + // constructor parameter. + string source = $$""" + using Microsoft.Extensions.Configuration; + using System.Collections.Generic; + + public class Program + { + public static object? Result; + + public static void Main() + { + ConfigurationBuilder configurationBuilder = new(); + configurationBuilder.AddInMemoryCollection(new Dictionary + { + ["Nested:Values:0"] = "a", + ["Nested:Values:1"] = "b", + }); + IConfiguration config = configurationBuilder.Build(); + + Holder holder = config.Get(); + NullableHolder nullableHolder = config.Get(); + Outer outer = config.Get(); + Rebindable rebindable = config.Get(); + + Rebindable existing = new(default); + config.Bind(existing); + + Result = new object?[] + { + holder.Nested.Values, + nullableHolder.Nested?.Values, + outer.Nested.Values, + rebindable.Nested.Values, + existing.Nested.Values, + }; + } + } + + public readonly record struct Inner({{collectionType}} Values); + + public class Holder + { + public Inner Nested { get; set; } + } + + public class NullableHolder + { + public Inner? Nested { get; set; } + } + + public record Outer(Inner Nested); + + public class Rebindable + { + public Rebindable(Inner nested) => Nested = nested; + + public Inner Nested { get; set; } + } + """; + + ConfigBindingGenRunResult result = await RunGeneratorAndUpdateCompilation(source, assemblyReferences: GetAssemblyRefsWithAdditional(typeof(ConfigurationBuilder), typeof(List<>))); + Assert.NotNull(result.GeneratedSource); + Assert.Empty(result.Diagnostics); + + var boundValues = (object?[])LoadAndInvokeMain(result.OutputCompilation, "Result")!; + Assert.All(boundValues, boundValue => Assert.Equal(new[] { "a", "b" }, (IEnumerable?)boundValue)); + } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))] public async Task SoleReadOnlyCollectionConstructorParameterOfComplexElementIsBindable() {