From c63ff5a11d81e2024a7bf6db7d8be0750568bca3 Mon Sep 17 00:00:00 2001 From: Eric StJohn Date: Fri, 19 Apr 2024 15:37:05 -0700 Subject: [PATCH 1/3] Prefer most derived member in Configuration Binder source generator --- .../ConfigurationBindingGenerator.Parser.cs | 6 ++++++ .../ConfigurationBinderTests.TestClasses.cs | 18 ++++++++++++++++++ .../tests/Common/ConfigurationBinderTests.cs | 14 ++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs index 2f6a221de9092..a9aa515903f41 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs @@ -654,6 +654,12 @@ private ObjectSpec CreateObjectSpec(TypeParseInfo typeParseInfo) if (member is IPropertySymbol { IsIndexer: false, IsImplicitlyDeclared: false } property && !IsUnsupportedType(property.Type)) { string propertyName = property.Name; + + if (properties?.ContainsKey(propertyName) is true) + { + continue; + } + TypeRef propertyTypeRef = EnqueueTransitiveType(typeParseInfo, property.Type, DiagnosticDescriptors.PropertyNotSupported, propertyName); AttributeData? attributeData = property.GetAttributes().FirstOrDefault(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, _typeSymbols.ConfigurationKeyNameAttribute)); 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 9aad9566463af..e6bca29044ff8 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 @@ -928,6 +928,24 @@ public class SimplePoco { public string A { get; set; } public string B { get; set; } + public TestSettingsEnum E {get; set;} + } + + public enum TestSettingsEnum2 + { + OptionA = TestSettingsEnum.Option1, + OptionB = TestSettingsEnum.Option2, + } + + public class DerivedClassWithHiddenMembers : SimplePoco + { + public new string A { get; } = "Derived"; + public new int B { get; set; } + public new TestSettingsEnum2 E + { + get => (TestSettingsEnum2)base.E; + set => base.E = (TestSettingsEnum)value; + } } } 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 2103749861348..18f0aa7198f35 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs @@ -2486,5 +2486,19 @@ public MockConfigurationRoot(IList providers) : base(pro IConfigurationSection IConfiguration.GetSection(string key) => this[key] is null ? null : new ConfigurationSection(this, key); } + + [Fact] + public void CanBindToClassWithNewProperties() + { + var config = TestHelpers.GetConfigurationFromJsonString(@"{""A"":""Should not bind"", ""B"":""5"", ""E"":""OptionB""}"); + var obj = new DerivedClassWithHiddenMembers(); + string initialA = obj.A; + + config.Bind(obj); + + Assert.Equal(initialA, obj.A); + Assert.Equal(5, obj.B); + Assert.Equal(TestSettingsEnum2.OptionB, obj.E); + } } } From 9914a0ce039a0592cbd1f4cc89f227c04ff455b1 Mon Sep 17 00:00:00 2001 From: Eric StJohn Date: Fri, 26 Apr 2024 14:39:05 -0700 Subject: [PATCH 2/3] Skip overridden properties in config source generator - include only definitions --- .../ConfigurationBindingGenerator.Parser.cs | 2 +- .../ConfigurationBinderTests.TestClasses.cs | 49 ++++++++++++++-- .../tests/Common/ConfigurationBinderTests.cs | 56 +++++++++++++++++-- 3 files changed, 98 insertions(+), 9 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs index a9aa515903f41..9522eeb83983d 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs @@ -655,7 +655,7 @@ private ObjectSpec CreateObjectSpec(TypeParseInfo typeParseInfo) { string propertyName = property.Name; - if (properties?.ContainsKey(propertyName) is true) + if (property.IsOverride || properties?.ContainsKey(propertyName) is true) { continue; } 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 e6bca29044ff8..02f3a74f317b1 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 @@ -925,27 +925,68 @@ public int MyIntProperty } public class SimplePoco + { + public string A { get; set; } + public string B { get; set; } + } + + public class BaseForHiddenMembers { public string A { get; set; } public string B { get; set; } public TestSettingsEnum E {get; set;} + + public virtual string C { get => CBase; set => CBase = value; } + + public string CBase; + + public virtual string D { get; } + + public virtual string F { get => FBase; set => FBase = value; } + public string FBase; + + + public virtual int X { get => XBase; set => XBase = value; } + public int XBase; } public enum TestSettingsEnum2 { - OptionA = TestSettingsEnum.Option1, - OptionB = TestSettingsEnum.Option2, + // Note - the reflection binder will try to bind to every member + Option1 = TestSettingsEnum.Option1, + Option2 = TestSettingsEnum.Option2, + } + + public class IntermediateDerivedClass : BaseForHiddenMembers + { + public new virtual string D { get => DBase; set => DBase = value; } + public string DBase; + + public override string F { get => "IF"; } + } - public class DerivedClassWithHiddenMembers : SimplePoco + public class DerivedClassWithHiddenMembers : IntermediateDerivedClass { - public new string A { get; } = "Derived"; + public new string A { get; } = "ADerived"; public new int B { get; set; } public new TestSettingsEnum2 E { get => (TestSettingsEnum2)base.E; set => base.E = (TestSettingsEnum)value; } + + // only override get + public override string C { get => "DC"; } + + // override new only get + public override string D { get => "DD"; } + + // two overrides of only get + public override string F { get => "DF"; } + + // override only set + public override int X { set => base.X = value + 1; } } } 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 18f0aa7198f35..c45ce4d836bec 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs @@ -2490,15 +2490,63 @@ IConfigurationSection IConfiguration.GetSection(string key) => [Fact] public void CanBindToClassWithNewProperties() { - var config = TestHelpers.GetConfigurationFromJsonString(@"{""A"":""Should not bind"", ""B"":""5"", ""E"":""OptionB""}"); + /// the source generator will bind to the most derived property only. + /// the reflection binder will bind the same data to all properties (including hidden). + + var config = TestHelpers.GetConfigurationFromJsonString(""" + { + "A": "AVal", + "B": "5", + "C": "CVal", + "D": "DVal", + "E": "Option2", + "F": "FVal", + "X": "52" + } + """); var obj = new DerivedClassWithHiddenMembers(); - string initialA = obj.A; config.Bind(obj); - Assert.Equal(initialA, obj.A); + BaseForHiddenMembers baseObj = obj; + IntermediateDerivedClass intermediateObj = obj; + + Assert.Equal("ADerived", obj.A); +#if BUILDING_SOURCE_GENERATOR_TESTS + // source generator will not set hidden property + Assert.Null(baseObj.A); +#else + // reflection binder will set hidden property + Assert.Equal("AVal", baseObj.A); +#endif + Assert.Equal(5, obj.B); - Assert.Equal(TestSettingsEnum2.OptionB, obj.E); +#if BUILDING_SOURCE_GENERATOR_TESTS + // source generator will not set hidden property + Assert.Null(baseObj.B); +#else + // reflection binder will set hidden property + Assert.Equal("5", baseObj.B); +#endif + + Assert.Equal(TestSettingsEnum2.Option2, obj.E); + Assert.Equal(TestSettingsEnum.Option2, baseObj.E); + + Assert.Equal("DC", obj.C); + // The setter should still be called, even when only getter is overridden. + Assert.Equal("CVal", obj.CBase); + + // can hide a readonly property with r/w property + Assert.Null(baseObj.D); + Assert.Equal("DD", obj.D); + // The setter should still be called, even when only getter is overridden. + Assert.Equal("DVal", obj.DBase); + + Assert.Equal("DF", obj.F); + Assert.Equal("FVal", obj.FBase); + + Assert.Equal(53, obj.X); + Assert.Equal(53, obj.XBase); } } } From bcc28ccde477c99a043f7c048e51218783af70f9 Mon Sep 17 00:00:00 2001 From: Eric StJohn Date: Mon, 29 Apr 2024 08:52:34 -0700 Subject: [PATCH 3/3] Enable shipping Microsoft.Extensions.Configuration.Binder --- .../src/Microsoft.Extensions.Configuration.Binder.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj index 4d20115e2f710..8ac03929b0740 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj @@ -4,8 +4,8 @@ $(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum) true true - false - 1 + true + 2 Provides the functionality to bind an object to data in configuration providers for Microsoft.Extensions.Configuration. This package enables you to represent the configuration data as strongly-typed classes defined in the application code. To bind a configuration, use the Microsoft.Extensions.Configuration.ConfigurationBinder.Get extension method on the IConfiguration object. To use this package, you also need to install a package for the configuration provider, for example, Microsoft.Extensions.Configuration.Json for the JSON provider.