Skip to content

Commit

Permalink
[release/9.0] Fix calling property setter In in the Configuration sou…
Browse files Browse the repository at this point in the history 
…rce generator (#107329)

* Fix calling property setterIn in the Configuration source generator

* Fix indentation

* delta change

---------

Co-authored-by: Tarek Mahmoud Sayed <[email protected]>
  • Loading branch information
@github-actions @tarekgh
github-actions[bot] and tarekgh authored Sep 4, 2024
1 parent a0847e7 commit 2875fda
Show file tree
Hide file tree
Showing 37 changed files with 431 additions and 14 deletions.
43 changes: 29 additions & 14 deletions src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -968,15 +968,31 @@ private void EmitBindingLogicForComplexMember(
initKind = InitializationKind.SimpleAssignment;
}

Action<string>? writeOnSuccess = !canSet
? null
: bindedValueIdentifier =>
{
if (memberAccessExpr != bindedValueIdentifier)
{
_writer.WriteLine($"{memberAccessExpr} = {bindedValueIdentifier};");
}
};
Action<string, string?>? writeOnSuccess = !canSet
? null
: (bindedValueIdentifier, tempIdentifierStoringExpr) =>
{
if (memberAccessExpr != bindedValueIdentifier)
{
_writer.WriteLine($"{memberAccessExpr} = {bindedValueIdentifier};");

if (tempIdentifierStoringExpr is not null)
{
_writer.WriteLine($"{tempIdentifierStoringExpr}");
}

if (member.CanGet && _typeIndex.CanInstantiate(effectiveMemberType))
{
EmitEndBlock();
EmitStartBlock("else");
_writer.WriteLine($"{memberAccessExpr} = {memberAccessExpr};");
}
}
else
{
_writer.WriteLine($"{tempIdentifierStoringExpr}");
}
};

EmitBindingLogic(
effectiveMemberType,
Expand All @@ -994,7 +1010,7 @@ private void EmitBindingLogic(
string configArgExpr,
InitializationKind initKind,
ValueDefaulting valueDefaulting,
Action<string>? writeOnSuccess = null)
Action<string, string?>? writeOnSuccess = null)
{
if (!_typeIndex.HasBindableMembers(type))
{
Expand Down Expand Up @@ -1022,15 +1038,14 @@ private void EmitBindingLogic(
}
else if (initKind is InitializationKind.None && type.IsValueType)
{
EmitBindingLogic(tempIdentifier, InitializationKind.Declaration);
_writer.WriteLine($"{memberAccessExpr} = {tempIdentifier};");
EmitBindingLogic(tempIdentifier, InitializationKind.Declaration, $"{memberAccessExpr} = {tempIdentifier};");
}
else
{
EmitBindingLogic(memberAccessExpr, initKind);
}

void EmitBindingLogic(string instanceToBindExpr, InitializationKind initKind)
void EmitBindingLogic(string instanceToBindExpr, InitializationKind initKind, string? tempIdentifierStoringExpr = null)
{
string bindCoreCall = $@"{nameof(MethodsToGen_CoreBindingHelper.BindCore)}({configArgExpr}, ref {instanceToBindExpr}, defaultValueIfNotFound: {FormatDefaultValueIfNotFound()}, {Identifier.binderOptions});";

Expand Down Expand Up @@ -1060,7 +1075,7 @@ void EmitBindingLogic(string instanceToBindExpr, InitializationKind initKind)
void EmitBindCoreCall()
{
_writer.WriteLine(bindCoreCall);
writeOnSuccess?.Invoke(instanceToBindExpr);
writeOnSuccess?.Invoke(instanceToBindExpr, tempIdentifierStoringExpr);
}

string FormatDefaultValueIfNotFound() => valueDefaulting == ValueDefaulting.CallSetter ? "true" : "false";
Expand Down
14 changes: 14 additions & 0 deletions ...soft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,20 @@ public record TypeWithRecursionThroughCollections
public List<TreeElement>? List { get; set; }
}

public class TypeWithValueMutatorPropertySetter
{
private string _value = "Uninitialized";
public string Value
{
get { return _value; }
set
{
_value = value == "Uninitialized" ? "Initialized" : value;
}
}
public ISet<string> SomeSet { get; set; } = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
}

public record RecordWithArrayParameter(string[] Array);

public readonly record struct ReadonlyRecordStructTypeOptions(string Color, int Length);
Expand Down
28 changes: 28 additions & 0 deletions ...raries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1625,6 +1625,34 @@ public void BindToRecursiveTypesTest()
Assert.Equal(1, instance.List[1].Values.Count);
}

/// <summary>
/// This test ensures that the property setter is invoked during binding, even when there is no configuration for the property.
/// </summary>
[Fact]
public void PropertySetterCalledTest()
{
string jsonConfig = @"{
""Configuration"": {
""SomeSet"": [
""path""
]
}
}";

var configuration = new ConfigurationBuilder()
.AddJsonStream(new System.IO.MemoryStream(System.Text.Encoding.UTF8.GetBytes(jsonConfig)))
.Build();

TypeWithValueMutatorPropertySetter t1 = new();
Assert.Equal(0, t1.SomeSet.Count);
Assert.Equal("Uninitialized", t1.Value);

TypeWithValueMutatorPropertySetter t2 = configuration.GetSection("Configuration").Get<TypeWithValueMutatorPropertySetter>()!;
Assert.Equal(1, t2.SomeSet.Count);
Assert.True(t2.SomeSet.Contains("path"));
Assert.Equal("Initialized", t2.Value);
}

[Fact]
public void CanBindReadonlyRecordStructOptions()
{
Expand Down
12 changes: 12 additions & 0 deletions ...ts/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Bind.generated.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section2, ref temp4, defaultValueIfNotFound: false, binderOptions);
instance.MyList = temp4;
}
else
{
instance.MyList = instance.MyList;
}

if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section5)
{
Expand All @@ -167,6 +171,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section5, ref temp7, defaultValueIfNotFound: false, binderOptions);
instance.MyDictionary = temp7;
}
else
{
instance.MyDictionary = instance.MyDictionary;
}

if (AsConfigWithChildren(configuration.GetSection("MyComplexDictionary")) is IConfigurationSection section8)
{
Expand All @@ -175,6 +183,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
instance.MyComplexDictionary = temp10;
}
else
{
instance.MyComplexDictionary = instance.MyComplexDictionary;
}
}


Expand Down
12 changes: 12 additions & 0 deletions ...GenerationTests/Baselines/net462/ConfigurationBinder/Version1/Bind_Instance.generated.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section2, ref temp4, defaultValueIfNotFound: false, binderOptions);
instance.MyList = temp4;
}
else
{
instance.MyList = instance.MyList;
}

if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section5)
{
Expand All @@ -131,6 +135,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section5, ref temp7, defaultValueIfNotFound: false, binderOptions);
instance.MyDictionary = temp7;
}
else
{
instance.MyDictionary = instance.MyDictionary;
}

if (AsConfigWithChildren(configuration.GetSection("MyComplexDictionary")) is IConfigurationSection section8)
{
Expand All @@ -139,6 +147,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
instance.MyComplexDictionary = temp10;
}
else
{
instance.MyComplexDictionary = instance.MyComplexDictionary;
}
}


Expand Down
12 changes: 12 additions & 0 deletions ...s/Baselines/net462/ConfigurationBinder/Version1/Bind_Instance_BinderOptions.generated.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section2, ref temp4, defaultValueIfNotFound: false, binderOptions);
instance.MyList = temp4;
}
else
{
instance.MyList = instance.MyList;
}

if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section5)
{
Expand All @@ -131,6 +135,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section5, ref temp7, defaultValueIfNotFound: false, binderOptions);
instance.MyDictionary = temp7;
}
else
{
instance.MyDictionary = instance.MyDictionary;
}

if (AsConfigWithChildren(configuration.GetSection("MyComplexDictionary")) is IConfigurationSection section8)
{
Expand All @@ -139,6 +147,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
instance.MyComplexDictionary = temp10;
}
else
{
instance.MyComplexDictionary = instance.MyComplexDictionary;
}
}


Expand Down
12 changes: 12 additions & 0 deletions ...rationTests/Baselines/net462/ConfigurationBinder/Version1/Bind_Key_Instance.generated.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section2, ref temp4, defaultValueIfNotFound: false, binderOptions);
instance.MyList = temp4;
}
else
{
instance.MyList = instance.MyList;
}

if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section5)
{
Expand All @@ -131,6 +135,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section5, ref temp7, defaultValueIfNotFound: false, binderOptions);
instance.MyDictionary = temp7;
}
else
{
instance.MyDictionary = instance.MyDictionary;
}

if (AsConfigWithChildren(configuration.GetSection("MyComplexDictionary")) is IConfigurationSection section8)
{
Expand All @@ -139,6 +147,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
instance.MyComplexDictionary = temp10;
}
else
{
instance.MyComplexDictionary = instance.MyComplexDictionary;
}
}


Expand Down
12 changes: 12 additions & 0 deletions ...sts/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Get.generated.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section5, ref temp7, defaultValueIfNotFound: false, binderOptions);
instance.MyList = temp7;
}
else
{
instance.MyList = instance.MyList;
}

if (AsConfigWithChildren(configuration.GetSection("MyArray")) is IConfigurationSection section8)
{
Expand All @@ -165,6 +169,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
instance.MyArray = temp10;
}
else
{
instance.MyArray = instance.MyArray;
}

if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section11)
{
Expand All @@ -173,6 +181,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section11, ref temp13, defaultValueIfNotFound: false, binderOptions);
instance.MyDictionary = temp13;
}
else
{
instance.MyDictionary = instance.MyDictionary;
}
}

public static void BindCore(IConfiguration configuration, ref global::Program.MyClass2 instance, bool defaultValueIfNotFound, BinderOptions? binderOptions)
Expand Down
12 changes: 12 additions & 0 deletions ...s/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Get_T.generated.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section4, ref temp6, defaultValueIfNotFound: false, binderOptions);
instance.MyList = temp6;
}
else
{
instance.MyList = instance.MyList;
}

if (AsConfigWithChildren(configuration.GetSection("MyArray")) is IConfigurationSection section7)
{
Expand All @@ -146,6 +150,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section7, ref temp9, defaultValueIfNotFound: false, binderOptions);
instance.MyArray = temp9;
}
else
{
instance.MyArray = instance.MyArray;
}

if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section10)
{
Expand All @@ -154,6 +162,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section10, ref temp12, defaultValueIfNotFound: false, binderOptions);
instance.MyDictionary = temp12;
}
else
{
instance.MyDictionary = instance.MyDictionary;
}
}


Expand Down
12 changes: 12 additions & 0 deletions ...tionTests/Baselines/net462/ConfigurationBinder/Version1/Get_T_BinderOptions.generated.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section4, ref temp6, defaultValueIfNotFound: false, binderOptions);
instance.MyList = temp6;
}
else
{
instance.MyList = instance.MyList;
}

if (AsConfigWithChildren(configuration.GetSection("MyArray")) is IConfigurationSection section7)
{
Expand All @@ -146,6 +150,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section7, ref temp9, defaultValueIfNotFound: false, binderOptions);
instance.MyArray = temp9;
}
else
{
instance.MyArray = instance.MyArray;
}

if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section10)
{
Expand All @@ -154,6 +162,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section10, ref temp12, defaultValueIfNotFound: false, binderOptions);
instance.MyDictionary = temp12;
}
else
{
instance.MyDictionary = instance.MyDictionary;
}
}


Expand Down
4 changes: 4 additions & 0 deletions ...eGenerationTests/Baselines/net462/OptionsBuilder/Version1/BindConfiguration.generated.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section3, ref temp5, defaultValueIfNotFound: false, binderOptions);
instance.MyList = temp5;
}
else
{
instance.MyList = instance.MyList;
}
}


Expand Down
4 changes: 4 additions & 0 deletions ...elines/net462/OptionsBuilder/Version1/BindConfigurationWithConfigureActions.generated.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section3, ref temp5, defaultValueIfNotFound: false, binderOptions);
instance.MyList = temp5;
}
else
{
instance.MyList = instance.MyList;
}
}


Expand Down
4 changes: 4 additions & 0 deletions ...tests/SourceGenerationTests/Baselines/net462/OptionsBuilder/Version1/Bind_T.generated.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
BindCore(section3, ref temp5, defaultValueIfNotFound: false, binderOptions);
instance.MyList = temp5;
}
else
{
instance.MyList = instance.MyList;
}
}


Expand Down
Loading

0 comments on commit 2875fda

Please sign in to comment.