From 7d273fe02c397c64aa73985cabfa856f472d8dc8 Mon Sep 17 00:00:00 2001 From: Eric StJohn Date: Mon, 29 Apr 2024 08:46:53 -0700 Subject: [PATCH] Prefer most derived member in Configuration Binder source generator (#101316) * Prefer most derived member in Configuration Binder source generator * Skip overridden properties in config source generator - include only definitions --- .../ConfigurationBindingGenerator.Parser.cs | 6 ++ .../ConfigurationBinderTests.TestClasses.cs | 59 ++++++++++++++++++ .../tests/Common/ConfigurationBinderTests.cs | 62 +++++++++++++++++++ 3 files changed, 127 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 a569be384367e..cb39cd788cfd7 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs @@ -660,6 +660,12 @@ private ObjectSpec CreateObjectSpec(TypeParseInfo typeParseInfo) if (member is IPropertySymbol { IsIndexer: false, IsImplicitlyDeclared: false } property && !IsUnsupportedType(property.Type)) { string propertyName = property.Name; + + if (property.IsOverride || 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..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 @@ -930,5 +930,64 @@ public class SimplePoco 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 + { + // 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 : IntermediateDerivedClass + { + 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 7b0a8ca7e7e38..95585b7623923 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs @@ -2471,5 +2471,67 @@ public MockConfigurationRoot(IList providers) : base(pro IConfigurationSection IConfiguration.GetSection(string key) => this[key] is null ? null : new ConfigurationSection(this, key); } + + [Fact] + public void CanBindToClassWithNewProperties() + { + /// 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(); + + config.Bind(obj); + + 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); +#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); + } } }