Skip to content

Commit a2318b1

Browse files
stevehartersteveharter
committed
Make src gen for property setters consistent with reflection
1 parent a6cf8d6 commit a2318b1

21 files changed

+308
-35
lines changed

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

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ private void EmitGetCoreMethod()
9797
Expression.sectionPath,
9898
writeOnSuccess: parsedValueExpr => _writer.WriteLine($"return {parsedValueExpr};"),
9999
checkForNullSectionValue: stringParsableType.StringParsableTypeKind is not StringParsableTypeKind.AssignFromSectionValue,
100+
useDefaultValueIfSectionValueIsNull : false,
100101
useIncrementalStringValueIdentifier: false);
101102
}
102103
break;
@@ -173,6 +174,7 @@ private void EmitGetValueCoreMethod()
173174
Expression.sectionPath,
174175
writeOnSuccess: (parsedValueExpr) => _writer.WriteLine($"return {parsedValueExpr};"),
175176
checkForNullSectionValue: false,
177+
useDefaultValueIfSectionValueIsNull: false,
176178
useIncrementalStringValueIdentifier: false);
177179

178180
EmitEndBlock();
@@ -307,6 +309,10 @@ private void EmitInitializeMethod(ObjectSpec type)
307309
{
308310
if (property.ShouldBindTo && property.MatchingCtorParam is null)
309311
{
312+
// Currently properties don't have an 'else' failure case. Doing that may collide with the
313+
// feature to set properties to their default values if they don't exist in the config section.
314+
Debug.Assert(property.ErrorOnFailedBinding == false);
315+
310316
EmitBindImplForMember(property);
311317
}
312318
}
@@ -661,6 +667,7 @@ private void EmitPopulationImplForEnumerableWithAdd(EnumerableSpec type)
661667
Expression.sectionPath,
662668
(parsedValueExpr) => _writer.WriteLine($"{addExpr}({parsedValueExpr});"),
663669
checkForNullSectionValue: true,
670+
useDefaultValueIfSectionValueIsNull: false,
664671
useIncrementalStringValueIdentifier: false);
665672
}
666673
break;
@@ -696,6 +703,7 @@ private void EmitBindCoreImplForDictionary(DictionarySpec type)
696703
Expression.sectionPath,
697704
Emit_BindAndAddLogic_ForElement,
698705
checkForNullSectionValue: false,
706+
useDefaultValueIfSectionValueIsNull: false,
699707
useIncrementalStringValueIdentifier: false);
700708

701709
void Emit_BindAndAddLogic_ForElement(string parsedKeyExpr)
@@ -710,6 +718,7 @@ void Emit_BindAndAddLogic_ForElement(string parsedKeyExpr)
710718
Expression.sectionPath,
711719
writeOnSuccess: parsedValueExpr => _writer.WriteLine($"{instanceIdentifier}[{parsedKeyExpr}] = {parsedValueExpr};"),
712720
checkForNullSectionValue: true,
721+
useDefaultValueIfSectionValueIsNull: false,
713722
useIncrementalStringValueIdentifier: false);
714723
}
715724
break;
@@ -786,6 +795,7 @@ private bool EmitBindImplForMember(
786795
bool canSet)
787796
{
788797
TypeSpec effectiveMemberType = member.Type.EffectiveType;
798+
789799
string sectionParseExpr = GetSectionFromConfigurationExpression(member.ConfigurationKeyName);
790800

791801
switch (effectiveMemberType)
@@ -794,19 +804,20 @@ private bool EmitBindImplForMember(
794804
{
795805
if (canSet)
796806
{
797-
bool checkForNullSectionValue = member is ParameterSpec
798-
? true
799-
: stringParsableType.StringParsableTypeKind is not StringParsableTypeKind.AssignFromSectionValue;
800-
801-
string nullBangExpr = checkForNullSectionValue ? string.Empty : "!";
807+
string nullBangExpr = member.Type.IsValueType ? string.Empty : "!";
808+
bool useDefaultValueIfSectionValueIsNull = member is PropertySpec &&
809+
//stringParsableType.StringParsableTypeKind is not StringParsableTypeKind.AssignFromSectionValue &&
810+
member.Type.IsValueType &&
811+
member.Type.SpecKind is not TypeSpecKind.Nullable;
802812

803813
EmitBlankLineIfRequired();
804814
EmitBindingLogic(
805815
stringParsableType,
806816
$@"{Identifier.configuration}[""{member.ConfigurationKeyName}""]",
807817
sectionPathExpr,
808818
writeOnSuccess: parsedValueExpr => _writer.WriteLine($"{memberAccessExpr} = {parsedValueExpr}{nullBangExpr};"),
809-
checkForNullSectionValue,
819+
checkForNullSectionValue: true,
820+
useDefaultValueIfSectionValueIsNull,
810821
useIncrementalStringValueIdentifier: true);
811822
}
812823

@@ -993,6 +1004,7 @@ private void EmitBindingLogic(
9931004
string sectionPathExpr,
9941005
Action<string>? writeOnSuccess,
9951006
bool checkForNullSectionValue,
1007+
bool useDefaultValueIfSectionValueIsNull,
9961008
bool useIncrementalStringValueIdentifier)
9971009
{
9981010
StringParsableTypeKind typeKind = type.StringParsableTypeKind;
@@ -1018,6 +1030,14 @@ private void EmitBindingLogic(
10181030
EmitEndBlock();
10191031
}
10201032

1033+
if (useDefaultValueIfSectionValueIsNull)
1034+
{
1035+
parsedValueExpr = $"default";
1036+
EmitStartBlock($"else");
1037+
InvokeWriteOnSuccess();
1038+
EmitEndBlock();
1039+
}
1040+
10211041
void InvokeWriteOnSuccess() => writeOnSuccess?.Invoke(parsedValueExpr);
10221042
}
10231043

‎src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Types/SimpleTypeSpec.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ internal enum StringParsableTypeKind
5454
None = 0,
5555

5656
/// <summary>
57-
/// Declared types that can be assigned directly from IConfigurationSection.Value, i.e. string and tyepof(object).
57+
/// Declared types that can be assigned directly from IConfigurationSection.Value, i.e. string and typeof(object).
5858
/// </summary>
5959
AssignFromSectionValue = 1,
6060
Enum = 2,

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,9 @@ public record NestedConfig(string MyProp);
443443
public class OptionWithCollectionProperties
444444
{
445445
private int _otherCode;
446+
private int _otherCodeNullable;
447+
private object _otherCodeNull;
448+
private Uri _otherCodeUri;
446449
private ICollection<string> blacklist = new HashSet<string>();
447450

448451
public ICollection<string> Blacklist
@@ -460,12 +463,31 @@ public ICollection<string> Blacklist
460463
// ParsedBlacklist initialized using the setter of Blacklist.
461464
public ICollection<string> ParsedBlacklist { get; private set; } = new HashSet<string>();
462465

463-
// This property not having any match in the configuration. Still the setter need to be called during the binding.
466+
// This does not have a match in the configuration, however the setter should be called during the binding:
464467
public int OtherCode
465468
{
466469
get => _otherCode;
467470
set => _otherCode = value == 0 ? 2 : value;
468471
}
472+
473+
// These do not have any match in the configuration, and the setters should not be called during the binding:
474+
public int? OtherCodeNullable
475+
{
476+
get => _otherCodeNullable;
477+
set => _otherCodeNullable = !value.HasValue ? 3 : value.Value;
478+
}
479+
480+
public object? OtherCodeNull
481+
{
482+
get => _otherCodeNull;
483+
set => _otherCodeNull = value is null ? 4 : value;
484+
}
485+
486+
public Uri OtherCodeUri
487+
{
488+
get => _otherCodeUri;
489+
set => _otherCodeUri = value is null ? new Uri("hello") : value;
490+
}
469491
}
470492

471493
public interface ISomeInterface

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,13 +1712,12 @@ public void EnsureCallingThePropertySetter()
17121712
Assert.Equal(2, options.ParsedBlacklist.Count); // should be initialized when calling the options.Blacklist setter.
17131713

17141714
Assert.Equal(401, options.HttpStatusCode); // exists in configuration and properly sets the property
1715-
#if BUILDING_SOURCE_GENERATOR_TESTS
1716-
// Setter not called if there's no matching configuration value.
1717-
Assert.Equal(0, options.OtherCode);
1718-
#else
1719-
// doesn't exist in configuration. the setter sets default value '2'
1720-
Assert.Equal(2, options.OtherCode);
1721-
#endif
1715+
Assert.Equal(2, options.OtherCode); // doesn't exist in configuration. the setter sets default value '2'
1716+
1717+
// These don't exist in configuration and setters are not called.
1718+
Assert.Equal(0, options.OtherCodeNullable);
1719+
Assert.Null(options.OtherCodeNull);
1720+
Assert.Null(options.OtherCodeUri);
17221721
}
17231722

17241723
[Fact]

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,19 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
126126
{
127127
ValidateConfigurationKeys(typeof(Program.MyClass), s_configKeys_ProgramMyClass, configuration, binderOptions);
128128

129-
instance.MyString = configuration["MyString"]!;
129+
if (configuration["MyString"] is string value0)
130+
{
131+
instance.MyString = value0!;
132+
}
130133

131134
if (configuration["MyInt"] is string value1)
132135
{
133136
instance.MyInt = ParseInt(value1, () => configuration.GetSection("MyInt").Path);
134137
}
138+
else
139+
{
140+
instance.MyInt = default;
141+
}
135142

136143
if (AsConfigWithChildren(configuration.GetSection("MyList")) is IConfigurationSection section2)
137144
{

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,19 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
9090
{
9191
ValidateConfigurationKeys(typeof(Program.MyClass), s_configKeys_ProgramMyClass, configuration, binderOptions);
9292

93-
instance.MyString = configuration["MyString"]!;
93+
if (configuration["MyString"] is string value0)
94+
{
95+
instance.MyString = value0!;
96+
}
9497

9598
if (configuration["MyInt"] is string value1)
9699
{
97100
instance.MyInt = ParseInt(value1, () => configuration.GetSection("MyInt").Path);
98101
}
102+
else
103+
{
104+
instance.MyInt = default;
105+
}
99106

100107
if (AsConfigWithChildren(configuration.GetSection("MyList")) is IConfigurationSection section2)
101108
{

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,19 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
9090
{
9191
ValidateConfigurationKeys(typeof(Program.MyClass), s_configKeys_ProgramMyClass, configuration, binderOptions);
9292

93-
instance.MyString = configuration["MyString"]!;
93+
if (configuration["MyString"] is string value0)
94+
{
95+
instance.MyString = value0!;
96+
}
9497

9598
if (configuration["MyInt"] is string value1)
9699
{
97100
instance.MyInt = ParseInt(value1, () => configuration.GetSection("MyInt").Path);
98101
}
102+
else
103+
{
104+
instance.MyInt = default;
105+
}
99106

100107
if (AsConfigWithChildren(configuration.GetSection("MyList")) is IConfigurationSection section2)
101108
{

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,19 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
9090
{
9191
ValidateConfigurationKeys(typeof(Program.MyClass), s_configKeys_ProgramMyClass, configuration, binderOptions);
9292

93-
instance.MyString = configuration["MyString"]!;
93+
if (configuration["MyString"] is string value0)
94+
{
95+
instance.MyString = value0!;
96+
}
9497

9598
if (configuration["MyInt"] is string value1)
9699
{
97100
instance.MyInt = ParseInt(value1, () => configuration.GetSection("MyInt").Path);
98101
}
102+
else
103+
{
104+
instance.MyInt = default;
105+
}
99106

100107
if (AsConfigWithChildren(configuration.GetSection("MyList")) is IConfigurationSection section2)
101108
{

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,19 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
116116
{
117117
ValidateConfigurationKeys(typeof(Program.MyClass), s_configKeys_ProgramMyClass, configuration, binderOptions);
118118

119-
instance.MyString = configuration["MyString"]!;
119+
if (configuration["MyString"] is string value4)
120+
{
121+
instance.MyString = value4!;
122+
}
120123

121124
if (configuration["MyInt"] is string value5)
122125
{
123126
instance.MyInt = ParseInt(value5, () => configuration.GetSection("MyInt").Path);
124127
}
128+
else
129+
{
130+
instance.MyInt = default;
131+
}
125132

126133
if (AsConfigWithChildren(configuration.GetSection("MyList")) is IConfigurationSection section6)
127134
{
@@ -156,6 +163,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
156163
{
157164
instance.MyInt = ParseInt(value15, () => configuration.GetSection("MyInt").Path);
158165
}
166+
else
167+
{
168+
instance.MyInt = default;
169+
}
159170
}
160171

161172

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,19 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
9797
{
9898
ValidateConfigurationKeys(typeof(Program.MyClass), s_configKeys_ProgramMyClass, configuration, binderOptions);
9999

100-
instance.MyString = configuration["MyString"]!;
100+
if (configuration["MyString"] is string value3)
101+
{
102+
instance.MyString = value3!;
103+
}
101104

102105
if (configuration["MyInt"] is string value4)
103106
{
104107
instance.MyInt = ParseInt(value4, () => configuration.GetSection("MyInt").Path);
105108
}
109+
else
110+
{
111+
instance.MyInt = default;
112+
}
106113

107114
if (AsConfigWithChildren(configuration.GetSection("MyList")) is IConfigurationSection section5)
108115
{

0 commit comments

Comments
 (0)