Skip to content

Commit 7d273fe

Browse files
ericstjmichaelgsharp
authored andcommitted
Prefer most derived member in Configuration Binder source generator (dotnet#101316)
* Prefer most derived member in Configuration Binder source generator * Skip overridden properties in config source generator - include only definitions
1 parent 54d9083 commit 7d273fe

File tree

3 files changed

+127
-0
lines changed

3 files changed

+127
-0
lines changed

src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,12 @@ private ObjectSpec CreateObjectSpec(TypeParseInfo typeParseInfo)
660660
if (member is IPropertySymbol { IsIndexer: false, IsImplicitlyDeclared: false } property && !IsUnsupportedType(property.Type))
661661
{
662662
string propertyName = property.Name;
663+
664+
if (property.IsOverride || properties?.ContainsKey(propertyName) is true)
665+
{
666+
continue;
667+
}
668+
663669
TypeRef propertyTypeRef = EnqueueTransitiveType(typeParseInfo, property.Type, DiagnosticDescriptors.PropertyNotSupported, propertyName);
664670

665671
AttributeData? attributeData = property.GetAttributes().FirstOrDefault(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, _typeSymbols.ConfigurationKeyNameAttribute));

src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -930,5 +930,64 @@ public class SimplePoco
930930
public string B { get; set; }
931931
}
932932

933+
public class BaseForHiddenMembers
934+
{
935+
public string A { get; set; }
936+
public string B { get; set; }
937+
public TestSettingsEnum E {get; set;}
938+
939+
public virtual string C { get => CBase; set => CBase = value; }
940+
941+
public string CBase;
942+
943+
public virtual string D { get; }
944+
945+
public virtual string F { get => FBase; set => FBase = value; }
946+
public string FBase;
947+
948+
949+
public virtual int X { get => XBase; set => XBase = value; }
950+
public int XBase;
951+
}
952+
953+
public enum TestSettingsEnum2
954+
{
955+
// Note - the reflection binder will try to bind to every member
956+
Option1 = TestSettingsEnum.Option1,
957+
Option2 = TestSettingsEnum.Option2,
958+
}
959+
960+
public class IntermediateDerivedClass : BaseForHiddenMembers
961+
{
962+
public new virtual string D { get => DBase; set => DBase = value; }
963+
public string DBase;
964+
965+
public override string F { get => "IF"; }
966+
967+
}
968+
969+
public class DerivedClassWithHiddenMembers : IntermediateDerivedClass
970+
{
971+
public new string A { get; } = "ADerived";
972+
public new int B { get; set; }
973+
public new TestSettingsEnum2 E
974+
{
975+
get => (TestSettingsEnum2)base.E;
976+
set => base.E = (TestSettingsEnum)value;
977+
}
978+
979+
// only override get
980+
public override string C { get => "DC"; }
981+
982+
// override new only get
983+
public override string D { get => "DD"; }
984+
985+
// two overrides of only get
986+
public override string F { get => "DF"; }
987+
988+
// override only set
989+
public override int X { set => base.X = value + 1; }
990+
}
991+
933992
}
934993
}

src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2471,5 +2471,67 @@ public MockConfigurationRoot(IList<IConfigurationProvider> providers) : base(pro
24712471
IConfigurationSection IConfiguration.GetSection(string key) =>
24722472
this[key] is null ? null : new ConfigurationSection(this, key);
24732473
}
2474+
2475+
[Fact]
2476+
public void CanBindToClassWithNewProperties()
2477+
{
2478+
/// the source generator will bind to the most derived property only.
2479+
/// the reflection binder will bind the same data to all properties (including hidden).
2480+
2481+
var config = TestHelpers.GetConfigurationFromJsonString("""
2482+
{
2483+
"A": "AVal",
2484+
"B": "5",
2485+
"C": "CVal",
2486+
"D": "DVal",
2487+
"E": "Option2",
2488+
"F": "FVal",
2489+
"X": "52"
2490+
}
2491+
""");
2492+
var obj = new DerivedClassWithHiddenMembers();
2493+
2494+
config.Bind(obj);
2495+
2496+
BaseForHiddenMembers baseObj = obj;
2497+
IntermediateDerivedClass intermediateObj = obj;
2498+
2499+
Assert.Equal("ADerived", obj.A);
2500+
#if BUILDING_SOURCE_GENERATOR_TESTS
2501+
// source generator will not set hidden property
2502+
Assert.Null(baseObj.A);
2503+
#else
2504+
// reflection binder will set hidden property
2505+
Assert.Equal("AVal", baseObj.A);
2506+
#endif
2507+
2508+
Assert.Equal(5, obj.B);
2509+
#if BUILDING_SOURCE_GENERATOR_TESTS
2510+
// source generator will not set hidden property
2511+
Assert.Null(baseObj.B);
2512+
#else
2513+
// reflection binder will set hidden property
2514+
Assert.Equal("5", baseObj.B);
2515+
#endif
2516+
2517+
Assert.Equal(TestSettingsEnum2.Option2, obj.E);
2518+
Assert.Equal(TestSettingsEnum.Option2, baseObj.E);
2519+
2520+
Assert.Equal("DC", obj.C);
2521+
// The setter should still be called, even when only getter is overridden.
2522+
Assert.Equal("CVal", obj.CBase);
2523+
2524+
// can hide a readonly property with r/w property
2525+
Assert.Null(baseObj.D);
2526+
Assert.Equal("DD", obj.D);
2527+
// The setter should still be called, even when only getter is overridden.
2528+
Assert.Equal("DVal", obj.DBase);
2529+
2530+
Assert.Equal("DF", obj.F);
2531+
Assert.Equal("FVal", obj.FBase);
2532+
2533+
Assert.Equal(53, obj.X);
2534+
Assert.Equal(53, obj.XBase);
2535+
}
24742536
}
24752537
}

0 commit comments

Comments
 (0)