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 42979bf896c101..9315f01b98c40c 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs @@ -887,8 +887,10 @@ private bool EmitBindImplForMember( { case ParsableFromStringSpec stringParsableType: { - // Reflection binder does not support binding to set-only properties - if (canSet && canGet) + // Bind when the property has a public setter AND either has a public getter + // or is truly set-only (no getter at all). Properties with non-public + // getters are skipped to match the reflection binder behavior. + if (canSet && (canGet || !member.HasAnyGetter)) { EmitBlankLineIfRequired(); string valueIdentifier = GetIncrementalIdentifier(Identifier.value); @@ -923,7 +925,9 @@ private bool EmitBindImplForMember( EmitEndBlock(); // End if-check for input type. - if (initializationKind == InitializationKind.Declaration) + // The defaultValueIfNotFound block reads the current value via the getter, + // so it can only be emitted when there is a getter. + if (canGet && initializationKind == InitializationKind.Declaration) { EmitStartBlock($"else if (defaultValueIfNotFound)"); if (!stringParsableType.TypeRef.CanBeNull) @@ -979,12 +983,24 @@ complexType is not CollectionSpec && // In this case, we will assign an empty array to the member. Otherwise, we will skip the binding logic. if ((complexType is ArraySpec || complexType.IsExactIEnumerableOfT) && canSet) { - // Either we have an array or we have an IEnumerable both these types can be assigned an empty array when having empty string configuration value. + // Either we have an array or we have an IEnumerable; both these types can be assigned an empty array when having empty string configuration value. Debug.Assert(complexType is ArraySpec || complexType is EnumerableSpec); string valueIdentifier = GetIncrementalIdentifier(Identifier.value); - EmitStartBlock($@"if ({memberAccessExpr} is null && {Identifier.TryGetConfigurationValue}({configSection}, {Identifier.key}: null, out string? {valueIdentifier}) && {valueIdentifier} == string.Empty)"); - _writer.WriteLine($"{memberAccessExpr} = global::System.{Identifier.Array}.Empty<{((CollectionSpec)complexType).ElementTypeRef.FullyQualifiedName}>();"); - EmitEndBlock(); + + if (canGet) + { + // For properties with getters, only assign the empty array when the current value is null. + EmitStartBlock($@"if ({memberAccessExpr} is null && {Identifier.TryGetConfigurationValue}({configSection}, {Identifier.key}: null, out string? {valueIdentifier}) && {valueIdentifier} == string.Empty)"); + _writer.WriteLine($"{memberAccessExpr} = global::System.{Identifier.Array}.Empty<{((CollectionSpec)complexType).ElementTypeRef.FullyQualifiedName}>();"); + EmitEndBlock(); + } + else + { + // For true set-only properties (no getter), we cannot read the current value; assign the empty array when the configuration value is empty string. + EmitStartBlock($@"if ({Identifier.TryGetConfigurationValue}({configSection}, {Identifier.key}: null, out string? {valueIdentifier}) && {valueIdentifier} == string.Empty)"); + _writer.WriteLine($"{memberAccessExpr} = global::System.{Identifier.Array}.Empty<{((CollectionSpec)complexType).ElementTypeRef.FullyQualifiedName}>();"); + EmitEndBlock(); + } } return _typeIndex.CanInstantiate(complexType); @@ -1019,7 +1035,22 @@ private void EmitBindingLogicForComplexMember( string effectiveMemberTypeFQN = effectiveMemberType.TypeRef.FullyQualifiedName; initKind = InitializationKind.None; - if (memberType is NullableSpec) + if (!member.CanGet) + { + if (member.HasAnyGetter) + { + // Property has a non-public getter that cannot be used here; skip binding to + // match the behavior of the reflection-based binder. + return; + } + + // Truly set-only property (no getter at all): declare a temp for the value so that + // the binding logic can initialize it appropriately (for value types, using a + // constructor call to match Activator.CreateInstance behavior, which honors + // user-defined parameterless constructors). + initKind = InitializationKind.Declaration; + } + else if (memberType is NullableSpec) { string nullableTempIdentifier = GetIncrementalIdentifier(Identifier.temp); @@ -1040,10 +1071,23 @@ private void EmitBindingLogicForComplexMember( targetObjAccessExpr = memberAccessExpr; initKind = InitializationKind.AssignmentWithNullCheck; } + else if (!member.HasAnyGetter) + { + // When there is no getter at all, the property can't be passed by ref (CS0206). + // Use a temp variable and assign back after binding. + if (!_typeIndex.CanInstantiate(effectiveMemberType)) + { + return; + } + + targetObjAccessExpr = tempIdentifier; + initKind = InitializationKind.Declaration; + } else { - targetObjAccessExpr = memberAccessExpr; - initKind = InitializationKind.SimpleAssignment; + // Property has a non-public getter that cannot be used here; skip binding to + // match the behavior of the reflection-based binder. + return; } Action? writeOnSuccess = !canSet diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Members/MemberSpec.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Members/MemberSpec.cs index cbc205ec2976ff..bdf5378a4eadce 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Members/MemberSpec.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Members/MemberSpec.cs @@ -25,5 +25,12 @@ public MemberSpec(ISymbol member, TypeRef typeRef) public abstract bool CanGet { get; } public abstract bool CanSet { get; } + + /// + /// Whether the member has a getter of any accessibility. + /// Used to distinguish true set-only properties (no getter at all) from + /// properties with non-public getters. + /// + public virtual bool HasAnyGetter => CanGet; } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Members/PropertySpec.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Members/PropertySpec.cs index 66257d06cab891..361fd0f39f5d49 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Members/PropertySpec.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Members/PropertySpec.cs @@ -18,6 +18,7 @@ public PropertySpec(IPropertySymbol property, TypeRef typeRef) : base(property, SetOnInit = setterIsPublic && (property.IsRequired || isInitOnly); CanSet = setterIsPublic && !isInitOnly; CanGet = property.GetMethod?.DeclaredAccessibility is Accessibility.Public; + HasAnyGetter = property.GetMethod is not null; } public ParameterSpec? MatchingCtorParam { get; set; } @@ -29,5 +30,7 @@ public PropertySpec(IPropertySymbol property, TypeRef typeRef) : base(property, public override bool CanGet { get; } public override bool CanSet { get; } + + public override bool HasAnyGetter { get; } } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 8166b108da3f31..bf257498b5f669 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -270,7 +270,8 @@ private static void BindProperties(object instance, IConfiguration configuration [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] private static void ResetPropertyValue(PropertyInfo property, object instance, BinderOptions options) { - // We don't support set only, non public, or indexer properties + // We don't support indexer properties, or properties without both a getter and a setter. + // Access to non-public accessors is controlled by BindNonPublicProperties. if (property.GetMethod is null || property.SetMethod is null || (!options.BindNonPublicProperties && (!property.GetMethod.IsPublic || !property.SetMethod.IsPublic)) || @@ -286,17 +287,36 @@ property.SetMethod is null || [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] private static void BindProperty(PropertyInfo property, object instance, IConfiguration config, BinderOptions options) { - // We don't support set only, non public, or indexer properties - if (property.GetMethod == null || - (!options.BindNonPublicProperties && !property.GetMethod.IsPublic) || - property.GetMethod.GetParameters().Length > 0) + // Indexer properties are not supported. Access to non-public accessors is controlled by BindNonPublicProperties. + if (property.GetMethod is { } getMethod) { - return; + if ((!options.BindNonPublicProperties && !getMethod.IsPublic) || + getMethod.GetParameters().Length > 0) + { + return; + } + } + else + { + // Set-only property: need an accessible setter to be useful. + // Also filter out set-only indexer properties (setter has more than just the value parameter). + if (property.SetMethod is null || + (!options.BindNonPublicProperties && !property.SetMethod.IsPublic) || + property.SetMethod.GetParameters().Length > 1) + { + return; + } } - var propertyBindingPoint = new BindingPoint( - initialValueProvider: () => property.GetValue(instance), - isReadOnly: property.SetMethod is null || (!property.SetMethod.IsPublic && !options.BindNonPublicProperties)); + bool hasGetter = property.GetMethod is not null; + + var propertyBindingPoint = hasGetter + ? new BindingPoint( + initialValueProvider: () => property.GetValue(instance), + isReadOnly: property.SetMethod is null || (!property.SetMethod.IsPublic && !options.BindNonPublicProperties)) + : new BindingPoint( + initialValue: null, + isReadOnly: false); BindInstance( property.PropertyType, 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 78f7cc9e27a554..19d7d8e7b5b530 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 @@ -630,11 +630,64 @@ public class SimplePocoWithOnlyDefaults public class SetOnlyPoco { - private bool _AnyCalled; - public bool AnyCalled => _AnyCalled; - public string SetOnly { set => _AnyCalled |= true; } - public string PrivateGetter { private get => "foo"; set => _AnyCalled |= true; } - public string InitOnly { init => _AnyCalled |= true; } + private bool _setOnlyCalled; + private bool _privateGetterCalled; + private bool _initOnlyCalled; + public bool SetOnlyCalled => _setOnlyCalled; + public bool PrivateGetterCalled => _privateGetterCalled; + public bool InitOnlyCalled => _initOnlyCalled; + public string SetOnly { set => _setOnlyCalled = true; } + public string PrivateGetter { private get => "foo"; set => _privateGetterCalled = true; } + public string InitOnly { init => _initOnlyCalled = true; } + } + + public class ComplexSetOnlyPoco + { + private SimplePoco _complex; + public SimplePoco GetComplex() => _complex; + public SimplePoco Complex { set => _complex = value; } + } + + public struct SetOnlyComplexStruct + { + public string A { get; set; } + } + + public class StructSetOnlyPoco + { + private SetOnlyComplexStruct _complex; + + public SetOnlyComplexStruct Complex + { + set => _complex = value; + } + + public SetOnlyComplexStruct GetComplex() + { + return _complex; + } + } + + public class SetOnlyWithTypeConversionPoco + { + public double TimeoutSeconds { set => Timeout = TimeSpan.FromSeconds(value); } + public TimeSpan Timeout { get; private set; } + } + + public class SetOnlyValueTypePoco + { + private bool _countSet; + private int _count; + public int Count { set { _countSet = true; _count = value; } } + public bool CountSet => _countSet; + public int GetCount() => _count; + } + + public class SetOnlyArrayPoco + { + private string[] _items; + public string[] GetItems() => _items; + public string[] Items { set => _items = value; } } public interface ISomeInterface 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 15514263abea49..de8b432c7d2758 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs @@ -1376,7 +1376,7 @@ public void CanBindSemiImmutableClass_WithInitProperties() } [Fact] - public void DoesNotCallSetOnly() + public void BindsSetOnlyProperties() { var dic = new Dictionary { @@ -1389,7 +1389,108 @@ public void DoesNotCallSetOnly() var config = configurationBuilder.Build(); var options = config.Get(); - Assert.False(options.AnyCalled); + Assert.True(options.SetOnlyCalled); + Assert.False(options.PrivateGetterCalled); +#if BUILDING_SOURCE_GENERATOR_TESTS + // Source generator treats init-only properties separately (SetOnInit); + // they are not bound through the normal property binding path. + Assert.False(options.InitOnlyCalled); +#else + // Reflection binder binds init-only set-only properties via PropertyInfo.SetValue. + Assert.True(options.InitOnlyCalled); +#endif + } + + [Fact] + public void BindsSetOnlyPropertiesWithTypeConversion() + { + var dic = new Dictionary + { + {"TimeoutSeconds", "30.5"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(TimeSpan.FromSeconds(30.5), options.Timeout); + } + + [Fact] + public void BindsSetOnlyComplexProperties() + { + var dic = new Dictionary + { + {"Complex:A", "Test"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.NotNull(options.GetComplex()); + Assert.Equal("Test", options.GetComplex().A); + } + + [Fact] + public void BindsSetOnlyComplexStructProperties() + { + var dic = new Dictionary + { + {"Complex:A", "Test"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal("Test", options.GetComplex().A); + } + + [Fact] + public void BindsSetOnlyArrayProperty_WithElements() + { + var dic = new Dictionary + { + {"Items:0", "a"}, + {"Items:1", "b"}, + }; + var config = new ConfigurationBuilder().AddInMemoryCollection(dic).Build(); + + var options = config.Get(); + Assert.Equal(new[] { "a", "b" }, options.GetItems()); + } + + [Fact] + public void BindsSetOnlyArrayProperty_WithEmptyString() + { + var dic = new Dictionary { { "Items", "" } }; + var config = new ConfigurationBuilder().AddInMemoryCollection(dic).Build(); + + var options = config.Get(); + Assert.NotNull(options.GetItems()); + Assert.Empty(options.GetItems()); + } + + [Fact] + public void BindsSetOnlyNonNullableValueType_WithValidConfig() + { + var dic = new Dictionary { { "Count", "42" } }; + var config = new ConfigurationBuilder().AddInMemoryCollection(dic).Build(); + + var options = config.Get(); + Assert.Equal(42, options.GetCount()); + } + + [Fact] + public void BindsSetOnlyProperties_ViaBind() + { + var dic = new Dictionary { { "SetOnly", "hello" } }; + var config = new ConfigurationBuilder().AddInMemoryCollection(dic).Build(); + + var target = new SetOnlyPoco(); + config.Bind(target); + Assert.True(target.SetOnlyCalled); } [Fact] @@ -1939,7 +2040,7 @@ public void CanBindVirtualProperties() Assert.Equal("3", test.TestGetOverridden); Assert.Equal("4", test.TestSetOverridden); Assert.Equal("5", test.TestNoOverridden); - Assert.Null(test.ExposeTestVirtualSet()); + Assert.Equal("6", test.ExposeTestVirtualSet()); } [Fact]