Skip to content

Commit 017ef35

Browse files
tarekghjtschuster
tarekgh
authored and
jtschuster
committed
Fix calling property setter In in the Configuration source generator (dotnet#107057)
* Fix calling property setterIn in the Configuration source generator * Fix indentation * delta change
1 parent 02b1f37 commit 017ef35

File tree

37 files changed

+431
-14
lines changed

37 files changed

+431
-14
lines changed

‎src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -968,15 +968,31 @@ private void EmitBindingLogicForComplexMember(
968968
initKind = InitializationKind.SimpleAssignment;
969969
}
970970

971-
Action<string>? writeOnSuccess = !canSet
972-
? null
973-
: bindedValueIdentifier =>
974-
{
975-
if (memberAccessExpr != bindedValueIdentifier)
976-
{
977-
_writer.WriteLine($"{memberAccessExpr} = {bindedValueIdentifier};");
978-
}
979-
};
971+
Action<string, string?>? writeOnSuccess = !canSet
972+
? null
973+
: (bindedValueIdentifier, tempIdentifierStoringExpr) =>
974+
{
975+
if (memberAccessExpr != bindedValueIdentifier)
976+
{
977+
_writer.WriteLine($"{memberAccessExpr} = {bindedValueIdentifier};");
978+
979+
if (tempIdentifierStoringExpr is not null)
980+
{
981+
_writer.WriteLine($"{tempIdentifierStoringExpr}");
982+
}
983+
984+
if (member.CanGet && _typeIndex.CanInstantiate(effectiveMemberType))
985+
{
986+
EmitEndBlock();
987+
EmitStartBlock("else");
988+
_writer.WriteLine($"{memberAccessExpr} = {memberAccessExpr};");
989+
}
990+
}
991+
else
992+
{
993+
_writer.WriteLine($"{tempIdentifierStoringExpr}");
994+
}
995+
};
980996

981997
EmitBindingLogic(
982998
effectiveMemberType,
@@ -994,7 +1010,7 @@ private void EmitBindingLogic(
9941010
string configArgExpr,
9951011
InitializationKind initKind,
9961012
ValueDefaulting valueDefaulting,
997-
Action<string>? writeOnSuccess = null)
1013+
Action<string, string?>? writeOnSuccess = null)
9981014
{
9991015
if (!_typeIndex.HasBindableMembers(type))
10001016
{
@@ -1022,15 +1038,14 @@ private void EmitBindingLogic(
10221038
}
10231039
else if (initKind is InitializationKind.None && type.IsValueType)
10241040
{
1025-
EmitBindingLogic(tempIdentifier, InitializationKind.Declaration);
1026-
_writer.WriteLine($"{memberAccessExpr} = {tempIdentifier};");
1041+
EmitBindingLogic(tempIdentifier, InitializationKind.Declaration, $"{memberAccessExpr} = {tempIdentifier};");
10271042
}
10281043
else
10291044
{
10301045
EmitBindingLogic(memberAccessExpr, initKind);
10311046
}
10321047

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

@@ -1060,7 +1075,7 @@ void EmitBindingLogic(string instanceToBindExpr, InitializationKind initKind)
10601075
void EmitBindCoreCall()
10611076
{
10621077
_writer.WriteLine(bindCoreCall);
1063-
writeOnSuccess?.Invoke(instanceToBindExpr);
1078+
writeOnSuccess?.Invoke(instanceToBindExpr, tempIdentifierStoringExpr);
10641079
}
10651080

10661081
string FormatDefaultValueIfNotFound() => valueDefaulting == ValueDefaulting.CallSetter ? "true" : "false";

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,20 @@ public record TypeWithRecursionThroughCollections
198198
public List<TreeElement>? List { get; set; }
199199
}
200200

201+
public class TypeWithValueMutatorPropertySetter
202+
{
203+
private string _value = "Uninitialized";
204+
public string Value
205+
{
206+
get { return _value; }
207+
set
208+
{
209+
_value = value == "Uninitialized" ? "Initialized" : value;
210+
}
211+
}
212+
public ISet<string> SomeSet { get; set; } = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
213+
}
214+
201215
public record RecordWithArrayParameter(string[] Array);
202216

203217
public readonly record struct ReadonlyRecordStructTypeOptions(string Color, int Length);

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1625,6 +1625,34 @@ public void BindToRecursiveTypesTest()
16251625
Assert.Equal(1, instance.List[1].Values.Count);
16261626
}
16271627

1628+
/// <summary>
1629+
/// This test ensures that the property setter is invoked during binding, even when there is no configuration for the property.
1630+
/// </summary>
1631+
[Fact]
1632+
public void PropertySetterCalledTest()
1633+
{
1634+
string jsonConfig = @"{
1635+
""Configuration"": {
1636+
""SomeSet"": [
1637+
""path""
1638+
]
1639+
}
1640+
}";
1641+
1642+
var configuration = new ConfigurationBuilder()
1643+
.AddJsonStream(new System.IO.MemoryStream(System.Text.Encoding.UTF8.GetBytes(jsonConfig)))
1644+
.Build();
1645+
1646+
TypeWithValueMutatorPropertySetter t1 = new();
1647+
Assert.Equal(0, t1.SomeSet.Count);
1648+
Assert.Equal("Uninitialized", t1.Value);
1649+
1650+
TypeWithValueMutatorPropertySetter t2 = configuration.GetSection("Configuration").Get<TypeWithValueMutatorPropertySetter>()!;
1651+
Assert.Equal(1, t2.SomeSet.Count);
1652+
Assert.True(t2.SomeSet.Contains("path"));
1653+
Assert.Equal("Initialized", t2.Value);
1654+
}
1655+
16281656
[Fact]
16291657
public void CanBindReadonlyRecordStructOptions()
16301658
{

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Bind.generated.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
159159
BindCore(section2, ref temp4, defaultValueIfNotFound: false, binderOptions);
160160
instance.MyList = temp4;
161161
}
162+
else
163+
{
164+
instance.MyList = instance.MyList;
165+
}
162166

163167
if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section5)
164168
{
@@ -167,6 +171,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
167171
BindCore(section5, ref temp7, defaultValueIfNotFound: false, binderOptions);
168172
instance.MyDictionary = temp7;
169173
}
174+
else
175+
{
176+
instance.MyDictionary = instance.MyDictionary;
177+
}
170178

171179
if (AsConfigWithChildren(configuration.GetSection("MyComplexDictionary")) is IConfigurationSection section8)
172180
{
@@ -175,6 +183,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
175183
BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
176184
instance.MyComplexDictionary = temp10;
177185
}
186+
else
187+
{
188+
instance.MyComplexDictionary = instance.MyComplexDictionary;
189+
}
178190
}
179191

180192

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Bind_Instance.generated.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
123123
BindCore(section2, ref temp4, defaultValueIfNotFound: false, binderOptions);
124124
instance.MyList = temp4;
125125
}
126+
else
127+
{
128+
instance.MyList = instance.MyList;
129+
}
126130

127131
if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section5)
128132
{
@@ -131,6 +135,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
131135
BindCore(section5, ref temp7, defaultValueIfNotFound: false, binderOptions);
132136
instance.MyDictionary = temp7;
133137
}
138+
else
139+
{
140+
instance.MyDictionary = instance.MyDictionary;
141+
}
134142

135143
if (AsConfigWithChildren(configuration.GetSection("MyComplexDictionary")) is IConfigurationSection section8)
136144
{
@@ -139,6 +147,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
139147
BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
140148
instance.MyComplexDictionary = temp10;
141149
}
150+
else
151+
{
152+
instance.MyComplexDictionary = instance.MyComplexDictionary;
153+
}
142154
}
143155

144156

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Bind_Instance_BinderOptions.generated.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
123123
BindCore(section2, ref temp4, defaultValueIfNotFound: false, binderOptions);
124124
instance.MyList = temp4;
125125
}
126+
else
127+
{
128+
instance.MyList = instance.MyList;
129+
}
126130

127131
if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section5)
128132
{
@@ -131,6 +135,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
131135
BindCore(section5, ref temp7, defaultValueIfNotFound: false, binderOptions);
132136
instance.MyDictionary = temp7;
133137
}
138+
else
139+
{
140+
instance.MyDictionary = instance.MyDictionary;
141+
}
134142

135143
if (AsConfigWithChildren(configuration.GetSection("MyComplexDictionary")) is IConfigurationSection section8)
136144
{
@@ -139,6 +147,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
139147
BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
140148
instance.MyComplexDictionary = temp10;
141149
}
150+
else
151+
{
152+
instance.MyComplexDictionary = instance.MyComplexDictionary;
153+
}
142154
}
143155

144156

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Bind_Key_Instance.generated.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
123123
BindCore(section2, ref temp4, defaultValueIfNotFound: false, binderOptions);
124124
instance.MyList = temp4;
125125
}
126+
else
127+
{
128+
instance.MyList = instance.MyList;
129+
}
126130

127131
if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section5)
128132
{
@@ -131,6 +135,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
131135
BindCore(section5, ref temp7, defaultValueIfNotFound: false, binderOptions);
132136
instance.MyDictionary = temp7;
133137
}
138+
else
139+
{
140+
instance.MyDictionary = instance.MyDictionary;
141+
}
134142

135143
if (AsConfigWithChildren(configuration.GetSection("MyComplexDictionary")) is IConfigurationSection section8)
136144
{
@@ -139,6 +147,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
139147
BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
140148
instance.MyComplexDictionary = temp10;
141149
}
150+
else
151+
{
152+
instance.MyComplexDictionary = instance.MyComplexDictionary;
153+
}
142154
}
143155

144156

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Get.generated.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
157157
BindCore(section5, ref temp7, defaultValueIfNotFound: false, binderOptions);
158158
instance.MyList = temp7;
159159
}
160+
else
161+
{
162+
instance.MyList = instance.MyList;
163+
}
160164

161165
if (AsConfigWithChildren(configuration.GetSection("MyArray")) is IConfigurationSection section8)
162166
{
@@ -165,6 +169,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
165169
BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
166170
instance.MyArray = temp10;
167171
}
172+
else
173+
{
174+
instance.MyArray = instance.MyArray;
175+
}
168176

169177
if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section11)
170178
{
@@ -173,6 +181,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
173181
BindCore(section11, ref temp13, defaultValueIfNotFound: false, binderOptions);
174182
instance.MyDictionary = temp13;
175183
}
184+
else
185+
{
186+
instance.MyDictionary = instance.MyDictionary;
187+
}
176188
}
177189

178190
public static void BindCore(IConfiguration configuration, ref global::Program.MyClass2 instance, bool defaultValueIfNotFound, BinderOptions? binderOptions)

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Get_T.generated.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
138138
BindCore(section4, ref temp6, defaultValueIfNotFound: false, binderOptions);
139139
instance.MyList = temp6;
140140
}
141+
else
142+
{
143+
instance.MyList = instance.MyList;
144+
}
141145

142146
if (AsConfigWithChildren(configuration.GetSection("MyArray")) is IConfigurationSection section7)
143147
{
@@ -146,6 +150,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
146150
BindCore(section7, ref temp9, defaultValueIfNotFound: false, binderOptions);
147151
instance.MyArray = temp9;
148152
}
153+
else
154+
{
155+
instance.MyArray = instance.MyArray;
156+
}
149157

150158
if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section10)
151159
{
@@ -154,6 +162,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
154162
BindCore(section10, ref temp12, defaultValueIfNotFound: false, binderOptions);
155163
instance.MyDictionary = temp12;
156164
}
165+
else
166+
{
167+
instance.MyDictionary = instance.MyDictionary;
168+
}
157169
}
158170

159171

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Get_T_BinderOptions.generated.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
138138
BindCore(section4, ref temp6, defaultValueIfNotFound: false, binderOptions);
139139
instance.MyList = temp6;
140140
}
141+
else
142+
{
143+
instance.MyList = instance.MyList;
144+
}
141145

142146
if (AsConfigWithChildren(configuration.GetSection("MyArray")) is IConfigurationSection section7)
143147
{
@@ -146,6 +150,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
146150
BindCore(section7, ref temp9, defaultValueIfNotFound: false, binderOptions);
147151
instance.MyArray = temp9;
148152
}
153+
else
154+
{
155+
instance.MyArray = instance.MyArray;
156+
}
149157

150158
if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section10)
151159
{
@@ -154,6 +162,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
154162
BindCore(section10, ref temp12, defaultValueIfNotFound: false, binderOptions);
155163
instance.MyDictionary = temp12;
156164
}
165+
else
166+
{
167+
instance.MyDictionary = instance.MyDictionary;
168+
}
157169
}
158170

159171

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/net462/OptionsBuilder/Version1/BindConfiguration.generated.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
141141
BindCore(section3, ref temp5, defaultValueIfNotFound: false, binderOptions);
142142
instance.MyList = temp5;
143143
}
144+
else
145+
{
146+
instance.MyList = instance.MyList;
147+
}
144148
}
145149

146150

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/net462/OptionsBuilder/Version1/BindConfigurationWithConfigureActions.generated.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
141141
BindCore(section3, ref temp5, defaultValueIfNotFound: false, binderOptions);
142142
instance.MyList = temp5;
143143
}
144+
else
145+
{
146+
instance.MyList = instance.MyList;
147+
}
144148
}
145149

146150

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/net462/OptionsBuilder/Version1/Bind_T.generated.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
147147
BindCore(section3, ref temp5, defaultValueIfNotFound: false, binderOptions);
148148
instance.MyList = temp5;
149149
}
150+
else
151+
{
152+
instance.MyList = instance.MyList;
153+
}
150154
}
151155

152156

0 commit comments

Comments
 (0)