From c6757859082a37cd48e6d43eb1fe1ec825d5b491 Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Mon, 20 Jul 2026 18:41:18 +0200 Subject: [PATCH 1/5] Fix config binding source generator duplicating collection ctor params Fixes #83803. For a type with a parameterized constructor, the generated code ran `Initialize(...)` (invoking the constructor, which binds the constructor parameters into their collections) and then `BindCore(...)`, which re-bound every property - re-appending to collections the constructor had already filled. This duplicated items for collection properties bound through a matching constructor parameter (e.g. `IList Ints` via ctor param `ints`/`Ints`); arrays and scalars were unaffected. The generated `BindCore` now takes an optional `boundThroughConstructor` parameter (emitted only for parameterized-ctor object types that actually have a re-bindable ctor-matched property). Constructor-matched properties are deferred into an `if (!boundThroughConstructor)` block so they are only bound when the instance was not created through the constructor (e.g. `Bind(existingInstance)`), matching the reflection binder. Call sites pass `true` when the instance is freshly constructed, and a runtime `wasNull` flag for the `x ??= Initialize()` case, which only constructs when the value was null. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a0d7ac2-f2a3-4526-93ab-36bf1a23933f --- .../gen/Emitter/CoreBindingHelpers.cs | 120 ++++++++++++++++-- .../gen/Emitter/Helpers.cs | 2 + .../ConfigurationBinderTests.TestClasses.cs | 16 +++ .../tests/Common/ConfigurationBinderTests.cs | 36 +++++- 4 files changed, 157 insertions(+), 17 deletions(-) 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 e89bad8ded56f7..9ad82f0e9cceb0 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs @@ -260,10 +260,17 @@ private void EmitBindCoreMethods() private void EmitBindCoreMethod(ComplexTypeSpec type) { string objParameterExpression = $"ref {type.TypeRef.FullyQualifiedName} {Identifier.instance}"; - EmitStartBlock(@$"public static void {nameof(MethodsToGen_CoreBindingHelper.BindCore)}({Identifier.IConfiguration} {Identifier.configuration}, {objParameterExpression}, bool defaultValueIfNotFound, {Identifier.BinderOptions}? {Identifier.binderOptions})"); - ComplexTypeSpec effectiveType = (ComplexTypeSpec)_typeIndex.GetEffectiveTypeSpec(type); + // Objects created through a parameterized constructor need an extra parameter that tells BindCore + // whether the instance was created through that constructor. When it was, properties that are bound + // by a matching constructor parameter must not be bound again (see EmitBindCoreImplForObject). + string boundThroughConstructorParam = ShouldEmitBoundThroughConstructorParameter(effectiveType) + ? $", bool {Identifier.boundThroughConstructor} = false" + : string.Empty; + + EmitStartBlock(@$"public static void {nameof(MethodsToGen_CoreBindingHelper.BindCore)}({Identifier.IConfiguration} {Identifier.configuration}, {objParameterExpression}, bool defaultValueIfNotFound, {Identifier.BinderOptions}? {Identifier.binderOptions}{boundThroughConstructorParam})"); + switch (effectiveType) { case ArraySpec arrayType: @@ -859,19 +866,87 @@ private void EmitBindCoreImplForObject(ObjectSpec type) string validateMethodCallExpr = $"{Identifier.ValidateConfigurationKeys}(typeof({type.TypeRef.FullyQualifiedName}), {keyCacheFieldName}, {Identifier.configuration}, {Identifier.binderOptions});"; _writer.WriteLine(validateMethodCallExpr); + List? ctorMatchedProperties = null; + foreach (PropertySpec property in type.Properties!) { - if (_typeIndex.ShouldBindTo(property)) + if (!_typeIndex.ShouldBindTo(property)) + { + continue; + } + + // A property that is bound through a matching constructor parameter is already populated when the + // instance is created through that constructor. Binding it again here would append to collections + // that the constructor already filled, duplicating their items. + // Defer such properties into a block guarded by !boundThroughConstructor so they are only bound when + // the instance was not created through the constructor (e.g. Bind(existingInstance)), matching the + // reflection binder. + if (property.MatchingCtorParam is not null && IsPropertyReboundInBindCore(property)) + { + (ctorMatchedProperties ??= new()).Add(property); + continue; + } + + EmitBindImplForProperty(property); + } + + if (ctorMatchedProperties is not null) + { + EmitStartBlock($"if (!{Identifier.boundThroughConstructor})"); + foreach (PropertySpec property in ctorMatchedProperties) { - string containingTypeRef = property.IsStatic ? type.TypeRef.FullyQualifiedName : Identifier.instance; - EmitBindImplForMember( - property, - memberAccessExpr: $"{containingTypeRef}.{property.Name}", - GetSectionPathFromConfigurationExpression(property.ConfigurationKeyName), - canSet: property.CanSet, - canGet: property.CanGet, - InitializationKind.Declaration); + EmitBindImplForProperty(property); } + EmitEndBlock(); + } + + void EmitBindImplForProperty(PropertySpec property) + { + string containingTypeRef = property.IsStatic ? type.TypeRef.FullyQualifiedName : Identifier.instance; + EmitBindImplForMember( + property, + memberAccessExpr: $"{containingTypeRef}.{property.Name}", + GetSectionPathFromConfigurationExpression(property.ConfigurationKeyName), + canSet: property.CanSet, + canGet: property.CanGet, + InitializationKind.Declaration); + } + } + + /// + /// Whether is an object created through a parameterized constructor that has at + /// least one property bound through a matching constructor parameter which would otherwise be re-bound in + /// its BindCore method. Such types receive an extra boundThroughConstructor parameter. + /// + private bool ShouldEmitBoundThroughConstructorParameter(ComplexTypeSpec type) => + type is ObjectSpec { InstantiationStrategy: ObjectInstantiationStrategy.ParameterizedConstructor, Properties: { } properties } && + properties.Any(property => + property.MatchingCtorParam is not null && + _typeIndex.ShouldBindTo(property) && + IsPropertyReboundInBindCore(property)); + + /// + /// Whether binding in a BindCore method emits code that reads from or + /// writes to the property. This mirrors the cases in + /// that actually emit binding logic; get-only value/string properties, for example, are never re-bound. + /// + private bool IsPropertyReboundInBindCore(PropertySpec property) + { + switch (_typeIndex.GetEffectiveTypeSpec(property.TypeRef)) + { + case ParsableFromStringSpec: + return property.CanGet && property.CanSet; + 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 }; + default: + return false; } } @@ -959,7 +1034,7 @@ private bool EmitBindImplForMember( { // Early detection of types we cannot bind to and skip it. if (!_typeIndex.HasBindableMembers(complexType) && - !_typeIndex.GetEffectiveTypeSpec(complexType).IsValueType && + !complexType.IsValueType && complexType is not CollectionSpec && ((ObjectSpec)complexType).InstantiationStrategy == ObjectInstantiationStrategy.ParameterizedConstructor) { @@ -1127,12 +1202,30 @@ private void EmitBindingLogic( void EmitBindingLogic(string instanceToBindExpr, InitializationKind initKind, string? tempIdentifierStoringExpr = null) { - string bindCoreCall = $@"{nameof(MethodsToGen_CoreBindingHelper.BindCore)}({configArgExpr}, ref {instanceToBindExpr}, defaultValueIfNotFound: {FormatDefaultValueIfNotFound()}, {Identifier.binderOptions});"; + string boundThroughConstructorArg = string.Empty; if (_typeIndex.CanInstantiate(type)) { if (initKind is not InitializationKind.None) { + // The instance is (re)created here. If it goes through a parameterized constructor, tell + // BindCore not to bind the properties that the constructor already bound. For a null-check + // assignment the constructor only runs when the existing value was null, so the decision + // has to be made at run time. + if (ShouldEmitBoundThroughConstructorParameter(type)) + { + if (initKind is InitializationKind.AssignmentWithNullCheck) + { + string wasNullIdentifier = GetIncrementalIdentifier(Identifier.wasNull); + _writer.WriteLine($"bool {wasNullIdentifier} = {instanceToBindExpr} is null;"); + boundThroughConstructorArg = $", {Identifier.boundThroughConstructor}: {wasNullIdentifier}"; + } + else + { + boundThroughConstructorArg = $", {Identifier.boundThroughConstructor}: true"; + } + } + EmitObjectInit(type, instanceToBindExpr, initKind, configArgExpr); } @@ -1154,6 +1247,7 @@ void EmitBindingLogic(string instanceToBindExpr, InitializationKind initKind, st void EmitBindCoreCall() { + string bindCoreCall = $@"{nameof(MethodsToGen_CoreBindingHelper.BindCore)}({configArgExpr}, ref {instanceToBindExpr}, defaultValueIfNotFound: {FormatDefaultValueIfNotFound()}, {Identifier.binderOptions}{boundThroughConstructorArg});"; _writer.WriteLine(bindCoreCall); writeOnSuccess?.Invoke(instanceToBindExpr, tempIdentifierStoringExpr); } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/Helpers.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/Helpers.cs index f80edafeced652..c0279e826f61c4 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/Helpers.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/Helpers.cs @@ -62,6 +62,7 @@ private static class TypeDisplayString private static class Identifier { public const string binderOptions = nameof(binderOptions); + public const string boundThroughConstructor = nameof(boundThroughConstructor); public const string config = nameof(config); public const string configureBinder = nameof(configureBinder); public const string configureOptions = nameof(configureOptions); @@ -86,6 +87,7 @@ private static class Identifier public const string typedObj = nameof(typedObj); public const string validateKeys = nameof(validateKeys); public const string value = nameof(value); + public const string wasNull = nameof(wasNull); public const string Add = nameof(Add); public const string AddSingleton = nameof(AddSingleton); diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs index bd2353717596f0..2e16b110e5a6b5 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs @@ -266,6 +266,22 @@ public class ParamsCollectionCtor public List Instances { get; } } + public sealed class SourceWithCollectionCtorParameters + { + public SourceWithCollectionCtorParameters(string Name, IEnumerable Addresses, IList Ints, string[] Strings) + { + this.Name = Name; + this.Addresses = Addresses; + this.Ints = Ints; + this.Strings = Strings; + } + + public string Name { get; } + public IEnumerable Addresses { get; } + public IList Ints { get; } + public string[] Strings { get; } + } + public readonly record struct ReadonlyRecordStructTypeOptions(string Color, int Length); public class ContainerWithNestedImmutableObject diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs index 034bea66dfb1bd..2ea8d0a9820631 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs @@ -1686,12 +1686,12 @@ public void CanBindOnParametersAndProperties_RecordWithArrayConstructorParameter } /// - /// When a constructor parameter name differs only by case from a matching collection property, - /// the binder must bind the collection once (through the constructor) and must not bind it again - /// through the property, which would otherwise duplicate the collection items. + /// When a constructor parameter populates a matching collection property, the binder must bind the collection + /// once (through the constructor) and must not bind it again through the property, which would otherwise + /// duplicate the collection items. This must hold whether the parameter name matches the property name exactly + /// or differs only by case. This test checks the different case scenario, the next one the same name scenario. /// [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/83803", typeof(TestHelpers), nameof(TestHelpers.SourceGenMode))] public void CanBindOnParametersAndProperties_GetterOnlyCollectionWithCaseMismatchedConstructorParameter() { string json = """ @@ -1709,6 +1709,34 @@ public void CanBindOnParametersAndProperties_GetterOnlyCollectionWithCaseMismatc Assert.Equal(expected, config.Get().Instances); } + /// + /// A type whose constructor parameters have the same name as the matching collection properties (as in the + /// canonical record/primary-constructor pattern) must also bind each collection only once. + /// + [Fact] + public void CanBindOnParametersAndProperties_CollectionsWithSameCasedConstructorParameters() + { + string json = """ + { + "source": { + "name": "DemoService", + "addresses": [ "127.0.0.1" ], + "ints": [ 1, 2, 3, 4, 5 ], + "strings": [ "one", "two", "three" ] + } + } + """; + + IConfiguration config = TestHelpers.GetConfigurationFromJsonString(json); + + SourceWithCollectionCtorParameters source = config.GetSection("source").Get(); + + Assert.Equal("DemoService", source.Name); + Assert.Equal(new[] { "127.0.0.1" }, source.Addresses); + Assert.Equal(new[] { 1, 2, 3, 4, 5 }, source.Ints); + Assert.Equal(new[] { "one", "two", "three" }, source.Strings); + } + public static IEnumerable Configuration_TestData() { yield return new object[] From 8848f7c211e057ed5841dbea48bf41d19c6ea57c Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Mon, 20 Jul 2026 19:40:42 +0200 Subject: [PATCH 2/5] Also skip constructor-bound properties for dictionary elements Dictionary value binding instantiates the element through a separate EmitObjectInit and then calls BindCore with InitializationKind.None, which bypassed the boundThroughConstructor flag. A dictionary whose value type is a parameterized-ctor type with a matching additive collection property would therefore still duplicate items. EmitBindingLogic now accepts an explicit run-time flag (constructedExpr) for callers that instantiate separately. The dictionary element path tracks whether the element was newly constructed (it is only created when not already present) and forwards that to BindCore. The enumerable-with-add and array paths already construct each element through InitializationKind.Declaration, so they were unaffected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a0d7ac2-f2a3-4526-93ab-36bf1a23933f --- .../gen/Emitter/CoreBindingHelpers.cs | 34 ++++++++++++++++--- .../tests/Common/ConfigurationBinderTests.cs | 26 ++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) 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 9ad82f0e9cceb0..e483cb6bbd1cc7 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs @@ -826,11 +826,27 @@ void Emit_BindAndAddLogic_ForElement(string parsedKeyExpr) if (_typeIndex.CanInstantiate(complexElementType)) { + // A reference-type element created through a parameterized constructor must not have + // its constructor-bound properties bound again. Since the element is only constructed + // when it isn't already present, track that at run time and forward it to BindCore. + // Value-type elements are always (re)constructed through the InitializationKind.None + // path below, so they don't need a separate flag. + string? constructedExpr = null; + if (!isValueType && ShouldEmitBoundThroughConstructorParameter(complexElementType)) + { + constructedExpr = GetIncrementalIdentifier(Identifier.boundThroughConstructor); + _writer.WriteLine($"bool {constructedExpr} = false;"); + } + EmitStartBlock($"if (!({conditionToUseExistingElement}))"); EmitObjectInit(complexElementType, Identifier.element, InitializationKind.SimpleAssignment, Identifier.section); + if (constructedExpr is not null) + { + _writer.WriteLine($"{constructedExpr} = true;"); + } EmitEndBlock(); - EmitBindingLogic(); + EmitBindingLogic(constructedExpr); } else { @@ -839,14 +855,15 @@ void Emit_BindAndAddLogic_ForElement(string parsedKeyExpr) EmitEndBlock(); } - void EmitBindingLogic() + void EmitBindingLogic(string? constructedExpr = null) { this.EmitBindingLogic( complexElementType, Identifier.element, Identifier.section, InitializationKind.None, - ValueDefaulting.None); + ValueDefaulting.None, + constructedExpr: constructedExpr); _writer.WriteLine($"{instanceIdentifier}[{parsedKeyExpr}] = {Identifier.element};"); } @@ -1165,7 +1182,8 @@ private void EmitBindingLogic( string configArgExpr, InitializationKind initKind, ValueDefaulting valueDefaulting, - Action? writeOnSuccess = null) + Action? writeOnSuccess = null, + string? constructedExpr = null) { if (!_typeIndex.HasBindableMembers(type)) { @@ -1228,6 +1246,14 @@ void EmitBindingLogic(string instanceToBindExpr, InitializationKind initKind, st EmitObjectInit(type, instanceToBindExpr, initKind, configArgExpr); } + else if (constructedExpr is not null) + { + // The caller instantiated the instance separately and provides a run-time flag telling + // whether it was created through its constructor (e.g. dictionary element binding). The + // caller only passes it for types that emit the parameter. + Debug.Assert(ShouldEmitBoundThroughConstructorParameter(type)); + boundThroughConstructorArg = $", {Identifier.boundThroughConstructor}: {constructedExpr}"; + } EmitBindCoreCall(); } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs index 2ea8d0a9820631..5f5fde593be85e 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs @@ -1737,6 +1737,32 @@ public void CanBindOnParametersAndProperties_CollectionsWithSameCasedConstructor Assert.Equal(new[] { "one", "two", "three" }, source.Strings); } + /// + /// A dictionary whose value type is created through a parameterized constructor that populates a collection + /// property must bind each element's collection only once, even though dictionary elements are bound through a + /// separate code path from top-level and property binding. + /// + [Fact] + public void CanBindOnParametersAndProperties_DictionaryValueWithCollectionConstructorParameter() + { + string json = """ + { + "map": { + "first": { "Instances": [ "a", "b" ] }, + "second": { "Instances": [ "c" ] } + } + } + """; + + IConfiguration config = TestHelpers.GetConfigurationFromJsonString(json); + + Dictionary map = + config.GetSection("map").Get>(); + + Assert.Equal(new[] { "a", "b" }, map["first"].Instances); + Assert.Equal(new[] { "c" }, map["second"].Instances); + } + public static IEnumerable Configuration_TestData() { yield return new object[] From 9f56ecf7fe7303fc7865fbacb36df7bfb419f3a2 Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Wed, 22 Jul 2026 14:38:46 +0200 Subject: [PATCH 3/5] Also skip init/required properties bound in Initialize An init-only or required property that is not backed by a constructor parameter is still bound while the instance is created, through the object initializer in the generated Initialize method. It was then bound again in BindCore, appending to collections that Initialize already filled and duplicating their items - the same class of bug as the constructor-parameter case, but a different root cause. Widen the BindCore deferral (and the boundThroughConstructor parameter emission) from ctor-matched properties to any property bound in Initialize, captured by the new IsBoundInInitialize helper. It is gated on the type being a parameterized-constructor type, since only those emit an Initialize method; parameterless-constructor types bind all their properties in BindCore. Also fix a CS1717 self-assignment (`x = x`) the generator emitted for a complex or collection init/required property in Initialize: the "preserve existing value" self-assignment is meaningful only for a property with a value-mutator setter, so skip it when binding into an Initialize local, which has no accessor to invoke. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a0d7ac2-f2a3-4526-93ab-36bf1a23933f --- .../gen/Emitter/CoreBindingHelpers.cs | 54 ++++++++++++------- .../ConfigurationBinderTests.TestClasses.cs | 7 +++ .../tests/Common/ConfigurationBinderTests.cs | 23 ++++++++ 3 files changed, 66 insertions(+), 18 deletions(-) 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 e483cb6bbd1cc7..cebe7d8a9705b0 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs @@ -415,7 +415,8 @@ void EmitBindImplForMember(MemberSpec member) // Since we're binding to local variables, we can always get and set canSet: true, canGet: true, - InitializationKind.None); + InitializationKind.None, + bindingToLocal: true); if (canBindToMember) { @@ -883,7 +884,7 @@ private void EmitBindCoreImplForObject(ObjectSpec type) string validateMethodCallExpr = $"{Identifier.ValidateConfigurationKeys}(typeof({type.TypeRef.FullyQualifiedName}), {keyCacheFieldName}, {Identifier.configuration}, {Identifier.binderOptions});"; _writer.WriteLine(validateMethodCallExpr); - List? ctorMatchedProperties = null; + List? initializeBoundProperties = null; foreach (PropertySpec property in type.Properties!) { @@ -892,25 +893,26 @@ private void EmitBindCoreImplForObject(ObjectSpec type) continue; } - // A property that is bound through a matching constructor parameter is already populated when the - // instance is created through that constructor. Binding it again here would append to collections - // that the constructor already filled, duplicating their items. + // A property that is populated while the instance is created through its Initialize method - either + // through a matching constructor parameter or as a required/init-only property assigned in the object + // initializer - is already bound at that point. Binding it again here would append to collections + // that Initialize already filled, duplicating their items. // Defer such properties into a block guarded by !boundThroughConstructor so they are only bound when - // the instance was not created through the constructor (e.g. Bind(existingInstance)), matching the + // the instance was not created through Initialize (e.g. Bind(existingInstance)), matching the // reflection binder. - if (property.MatchingCtorParam is not null && IsPropertyReboundInBindCore(property)) + if (IsBoundInInitialize(type, property) && IsPropertyReboundInBindCore(property)) { - (ctorMatchedProperties ??= new()).Add(property); + (initializeBoundProperties ??= new()).Add(property); continue; } EmitBindImplForProperty(property); } - if (ctorMatchedProperties is not null) + if (initializeBoundProperties is not null) { EmitStartBlock($"if (!{Identifier.boundThroughConstructor})"); - foreach (PropertySpec property in ctorMatchedProperties) + foreach (PropertySpec property in initializeBoundProperties) { EmitBindImplForProperty(property); } @@ -932,16 +934,27 @@ void EmitBindImplForProperty(PropertySpec property) /// /// Whether is an object created through a parameterized constructor that has at - /// least one property bound through a matching constructor parameter which would otherwise be re-bound in - /// its BindCore method. Such types receive an extra boundThroughConstructor parameter. + /// least one property bound while the instance is created (through its Initialize method) which would + /// otherwise be re-bound in its BindCore method. Such types receive an extra + /// boundThroughConstructor parameter. /// private bool ShouldEmitBoundThroughConstructorParameter(ComplexTypeSpec type) => - type is ObjectSpec { InstantiationStrategy: ObjectInstantiationStrategy.ParameterizedConstructor, Properties: { } properties } && + type is ObjectSpec { Properties: { } properties } objectType && properties.Any(property => - property.MatchingCtorParam is not null && + IsBoundInInitialize(objectType, property) && _typeIndex.ShouldBindTo(property) && IsPropertyReboundInBindCore(property)); + /// + /// Whether is populated while an instance of is created + /// through its Initialize method: either it flows through a matching constructor parameter, or it is a + /// required/init-only property assigned in the object initializer. Only parameterized-constructor types have + /// an Initialize method; parameterless-constructor types bind all their properties in BindCore. + /// + private static bool IsBoundInInitialize(ObjectSpec type, PropertySpec property) => + type.InstantiationStrategy is ObjectInstantiationStrategy.ParameterizedConstructor && + (property.MatchingCtorParam is not null || property.SetOnInit); + /// /// Whether binding in a BindCore method emits code that reads from or /// writes to the property. This mirrors the cases in @@ -973,7 +986,8 @@ private bool EmitBindImplForMember( string sectionPathExpr, bool canSet, bool canGet, - InitializationKind initializationKind) + InitializationKind initializationKind, + bool bindingToLocal = false) { string sectionParseExpr = GetSectionFromConfigurationExpression(member.ConfigurationKeyName); @@ -1066,7 +1080,7 @@ complexType is not CollectionSpec && string sectionIdentifier = GetIncrementalIdentifier(Identifier.section); EmitStartBlock($"if ({sectionValidationCall} is {Identifier.IConfigurationSection} {sectionIdentifier})"); - EmitBindingLogicForComplexMember(member, memberAccessExpr, sectionIdentifier, canSet); + EmitBindingLogicForComplexMember(member, memberAccessExpr, sectionIdentifier, canSet, bindingToLocal); EmitEndBlock(); // The current configuration section doesn't have any children, let's check if we are binding to an array and the configuration value is empty string. @@ -1092,7 +1106,8 @@ private void EmitBindingLogicForComplexMember( MemberSpec member, string memberAccessExpr, string configArgExpr, - bool canSet) + bool canSet, + bool bindingToLocal = false) { TypeSpec memberType = _typeIndex.GetTypeSpec(member.TypeRef); @@ -1153,7 +1168,10 @@ private void EmitBindingLogicForComplexMember( _writer.WriteLine($"{tempIdentifierStoringExpr}"); } - if (member.CanGet && _typeIndex.CanInstantiate(effectiveMemberType)) + // When the config section is absent, re-assign the member to itself so any value-mutator + // setter runs. This is unnecessary when binding into an Initialize local (no accessor to + // invoke) and would emit a CS1717 self-assignment, so skip it in that case. + if (!bindingToLocal && member.CanGet && _typeIndex.CanInstantiate(effectiveMemberType)) { EmitEndBlock(); EmitStartBlock("else"); diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs index 2e16b110e5a6b5..7b19907ad80f6a 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs @@ -282,6 +282,13 @@ public SourceWithCollectionCtorParameters(string Name, IEnumerable Addre public string[] Strings { get; } } + public sealed class ClassWithInitOnlyCollectionNoCtorParam + { + public ClassWithInitOnlyCollectionNoCtorParam(int Number) => this.Number = Number; + public int Number { get; } + public List Items { get; init; } + } + public readonly record struct ReadonlyRecordStructTypeOptions(string Color, int Length); public class ContainerWithNestedImmutableObject diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs index 5f5fde593be85e..c6ff7a42a6208d 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs @@ -1763,6 +1763,29 @@ public void CanBindOnParametersAndProperties_DictionaryValueWithCollectionConstr Assert.Equal(new[] { "c" }, map["second"].Instances); } + /// + /// An init-only (or required) collection property that is not backed by a constructor parameter is bound in the + /// generated Initialize method (through the object initializer). It must not be bound again in BindCore when the + /// instance was created there, or the collection items would be duplicated. + /// + [Fact] + public void CanBindOnParametersAndProperties_InitOnlyCollectionWithoutConstructorParameter() + { + string json = """ + { + "Number": 7, + "Items": [ "a", "b" ] + } + """; + + IConfiguration config = TestHelpers.GetConfigurationFromJsonString(json); + + ClassWithInitOnlyCollectionNoCtorParam result = config.Get(); + + Assert.Equal(7, result.Number); + Assert.Equal(new[] { "a", "b" }, result.Items); + } + public static IEnumerable Configuration_TestData() { yield return new object[] From e436669fb742b3f3a146a777663cb086e41907d4 Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Wed, 22 Jul 2026 16:04:15 +0200 Subject: [PATCH 4/5] Add nested-property and Bind(existingInstance) coverage Address review feedback on PR #131092 by covering two paths the existing tests missed: - CanBind_NestedTypeWithCollectionConstructorParameter: a container whose property type has a ctor-matched collection, bound through the nested null-check (??=) path. This exercises the runtime `wasNull` branch of boundThroughConstructor and duplicates on the unfixed generator (verified). - CanBindExistingInstance_BindsCtorMatchedCollection: binding into an existing instance still binds a settable ctor-matched collection. This passes with or without the fix; it guards the boundThroughConstructor:false branch so a future change cannot start over-skipping and silently stop binding such properties. Both live in tests/Common so they run under the reflection and source-generator binders. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a0d7ac2-f2a3-4526-93ab-36bf1a23933f --- .../ConfigurationBinderTests.TestClasses.cs | 5 +++ .../tests/Common/ConfigurationBinderTests.cs | 43 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs index 7b19907ad80f6a..0d9fb2a7ee6617 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs @@ -260,6 +260,11 @@ public class GetterOnlyInterfaceCollectionWithCaseMismatchedCtorParameter public IList Instances { get; } } + public sealed class ContainerWithCtorCollectionChild + { + public GetterOnlyInterfaceCollectionWithCaseMismatchedCtorParameter Child { get; set; } + } + public class ParamsCollectionCtor { public ParamsCollectionCtor(params List instances) => Instances = instances; diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs index c6ff7a42a6208d..27fc64d295a889 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs @@ -1763,6 +1763,49 @@ public void CanBindOnParametersAndProperties_DictionaryValueWithCollectionConstr Assert.Equal(new[] { "c" }, map["second"].Instances); } + /// + /// A nested property whose type has a collection populated through a constructor parameter is bound through the + /// null-check assignment (??=) path, where whether the instance was created through its constructor is + /// decided at run time. The collection must still be bound only once. + /// + [Fact] + public void CanBind_NestedTypeWithCollectionConstructorParameter() + { + string json = """ + { + "Child": { "Instances": [ "a", "b" ] } + } + """; + + IConfiguration config = TestHelpers.GetConfigurationFromJsonString(json); + + ContainerWithCtorCollectionChild result = config.Get(); + + Assert.Equal(new[] { "a", "b" }, result.Child.Instances); + } + + /// + /// When binding into an existing instance (which is not created through its constructor), a settable collection + /// property backed by a constructor parameter must still be bound. This guards against the deferral logic + /// over-skipping and silently not binding such properties. + /// + [Fact] + public void CanBindExistingInstance_BindsCtorMatchedCollection() + { + string json = """ + { + "Instances": [ "first", "second" ] + } + """; + + IConfiguration config = TestHelpers.GetConfigurationFromJsonString(json); + var instance = new SettableCollectionWithCaseMismatchedCtorParameter(new List()); + + config.Bind(instance); + + Assert.Equal(new[] { "first", "second" }, instance.Instances); + } + /// /// An init-only (or required) collection property that is not backed by a constructor parameter is bound in the /// generated Initialize method (through the object initializer). It must not be bound again in BindCore when the From 740d845049d24c4e77a28868a521c9d2a6507459 Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Wed, 22 Jul 2026 17:13:14 +0200 Subject: [PATCH 5/5] Add list-element and init-only-complex-property test coverage Cover two more paths affected by the fix: - CanBind_ListOfTypeWithCollectionConstructorParameter: a list whose element type has a ctor-matched collection, exercising the enumerable-with-add (and array) element construction path. This duplicates on the unfixed generator (verified), mirroring the dictionary-element regression test. - CanBindOnParametersAndProperties_InitOnlyComplexPropertyWithoutConstructorParameter: an init-only complex (non-collection) property with no ctor param, exercising the CS1717 self-assignment fix for the object case (the existing init-only test covers only the collection case). Verified it fails to compile (CS1717) without the bindingToLocal guard. Both live in tests/Common so they run under the reflection and source-generator binders. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a0d7ac2-f2a3-4526-93ab-36bf1a23933f --- .../ConfigurationBinderTests.TestClasses.cs | 12 +++++ .../tests/Common/ConfigurationBinderTests.cs | 46 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs index 0d9fb2a7ee6617..3ba6742be4153c 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs @@ -294,6 +294,18 @@ public sealed class ClassWithInitOnlyCollectionNoCtorParam public List Items { get; init; } } + public sealed class ClassWithInitOnlyComplexNoCtorParam + { + public ClassWithInitOnlyComplexNoCtorParam(int Number) => this.Number = Number; + public int Number { get; } + public NestedForInitOnly Child { get; init; } + } + + public sealed class NestedForInitOnly + { + public string Value { get; set; } + } + public readonly record struct ReadonlyRecordStructTypeOptions(string Color, int Length); public class ContainerWithNestedImmutableObject diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs index 27fc64d295a889..ae16017765ba8b 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs @@ -1763,6 +1763,29 @@ public void CanBindOnParametersAndProperties_DictionaryValueWithCollectionConstr Assert.Equal(new[] { "c" }, map["second"].Instances); } + /// + /// A list whose element type has a collection populated through a constructor parameter must bind each + /// element's collection only once. List and array elements are constructed through a separate code path + /// (enumerable-with-add) from top-level, property, and dictionary binding. + /// + [Fact] + public void CanBind_ListOfTypeWithCollectionConstructorParameter() + { + string json = """ + { + "Items": [ { "Instances": [ "a", "b" ] }, { "Instances": [ "c" ] } ] + } + """; + + IConfiguration config = TestHelpers.GetConfigurationFromJsonString(json); + + List result = + config.GetSection("Items").Get>(); + + Assert.Equal(new[] { "a", "b" }, result[0].Instances); + Assert.Equal(new[] { "c" }, result[1].Instances); + } + /// /// A nested property whose type has a collection populated through a constructor parameter is bound through the /// null-check assignment (??=) path, where whether the instance was created through its constructor is @@ -1829,6 +1852,29 @@ public void CanBindOnParametersAndProperties_InitOnlyCollectionWithoutConstructo Assert.Equal(new[] { "a", "b" }, result.Items); } + /// + /// An init-only complex (non-collection) property that is not backed by a constructor parameter is bound in the + /// generated Initialize method. The generator must not emit a self-assignment for it (which would produce a + /// CS1717 compile error), and the property must still be bound correctly. + /// + [Fact] + public void CanBindOnParametersAndProperties_InitOnlyComplexPropertyWithoutConstructorParameter() + { + string json = """ + { + "Number": 7, + "Child": { "Value": "hello" } + } + """; + + IConfiguration config = TestHelpers.GetConfigurationFromJsonString(json); + + ClassWithInitOnlyComplexNoCtorParam result = config.Get(); + + Assert.Equal(7, result.Number); + Assert.Equal("hello", result.Child.Value); + } + public static IEnumerable Configuration_TestData() { yield return new object[]