Skip to content

Commit

Permalink
[release/8.0-staging] Prefer most derived member in Configuration Bin…
Browse files Browse the repository at this point in the history
…der source generator (#101686)

* Prefer most derived member in Configuration Binder source generator

* Skip overridden properties in config source generator - include only definitions

* Enable shipping Microsoft.Extensions.Configuration.Binder

---------

Co-authored-by: Eric StJohn <ericstj@microsoft.com>
  • Loading branch information
github-actions[bot] and ericstj authored Apr 30, 2024
1 parent 8acc1b5 commit 47f25fb
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 (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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<EnableDefaultItems>true</EnableDefaultItems>
<IsPackable>true</IsPackable>
<GeneratePackageOnBuild>false</GeneratePackageOnBuild>
<ServicingVersion>1</ServicingVersion>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<ServicingVersion>2</ServicingVersion>
<PackageDescription>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.</PackageDescription>
</PropertyGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2486,5 +2486,67 @@ public MockConfigurationRoot(IList<IConfigurationProvider> 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);
}
}
}

0 comments on commit 47f25fb

Please sign in to comment.