Skip to content

Make src gen for property setters consistent with reflection #91899

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
steveharter merged 6 commits into from
Sep 14, 2023
Merged

Make src gen for property setters consistent with reflection #91899

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a2318b1
Make src gen for property setters consistent with reflection
steveharter Sep 11, 2023
aa1bc8c
Don't default value during initialization
steveharter Sep 13, 2023
260f31a
Remove unnecessary bang operator
steveharter Sep 13, 2023
7855e6b
Don't default a property when it is a collection or child type
steveharter Sep 13, 2023
984fb95
Simply use of new variable; add to test
steveharter Sep 14, 2023
88e1ced
Review feedback
steveharter Sep 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Review feedback
  • Loading branch information
@steveharter
steveharter committed Sep 14, 2023
commit 88e1ced0c3a4470b7eced2092f2542e464a9b5ae
2 changes: 1 addition & 1 deletion src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/ConfigurationBinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ void EmitMethods(MethodsToGen_ConfigurationBinder method, string additionalParam
EmitCheckForNullArgument_WithBlankLine(Identifier.instance, voidReturn: true);
_writer.WriteLine($$"""
var {{Identifier.typedObj}} = ({{type.EffectiveType.DisplayString}}){{Identifier.instance}};
{{nameof(MethodsToGen_CoreBindingHelper.BindCore)}}({{configExpression}}, ref {{Identifier.typedObj}}, false, {{binderOptionsArg}});
{{nameof(MethodsToGen_CoreBindingHelper.BindCore)}}({{configExpression}}, ref {{Identifier.typedObj}}, defaultValueIfNotFound: false, {{binderOptionsArg}});
""");
}

Expand Down
21 changes: 8 additions & 13 deletions src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,6 @@ private void EmitInitializeMethod(ObjectSpec type)
{
if (property.ShouldBindTo && property.MatchingCtorParam is null)
{
// Currently properties don't have an 'else' failure case. Doing that may collide with the
// feature to set properties to their default values if they don't exist in the config section.
Debug.Assert(property.ErrorOnFailedBinding == false);

EmitBindImplForMember(property);
}
}
Expand Down Expand Up @@ -340,8 +336,6 @@ private void EmitInitializeMethod(ObjectSpec type)
void EmitBindImplForMember(MemberSpec member)
{
TypeSpec memberType = member.Type;
bool errorOnFailedBinding = member.ErrorOnFailedBinding;

string parsedMemberDeclarationLhs = $"{memberType.DisplayString} {member.Name}";
string configKeyName = member.ConfigurationKeyName;
string parsedMemberAssignmentLhsExpr;
Expand All @@ -350,7 +344,7 @@ void EmitBindImplForMember(MemberSpec member)
{
case ParsableFromStringSpec { StringParsableTypeKind: StringParsableTypeKind.AssignFromSectionValue }:
{
if (errorOnFailedBinding)
if (member is ParameterSpec parameter && parameter.ErrorOnFailedBinding)
{
string condition = $@"if ({Identifier.configuration}[""{configKeyName}""] is not {parsedMemberDeclarationLhs})";
EmitThrowBlock(condition);
Expand Down Expand Up @@ -384,11 +378,11 @@ void EmitBindImplForMember(MemberSpec member)
parsedMemberAssignmentLhsExpr,
sectionPathExpr: GetSectionPathFromConfigurationExpression(configKeyName),
canSet: true,
firstTimeInitialization: true);
InitializationKind.None);

if (canBindToMember)
{
if (errorOnFailedBinding)
if (member is ParameterSpec parameter && parameter.ErrorOnFailedBinding)
{
// Add exception logic for parameter ctors; must be present in configuration object.
EmitThrowBlock(condition: "else");
Expand Down Expand Up @@ -785,7 +779,7 @@ private void EmitBindCoreImplForObject(ObjectSpec type)
memberAccessExpr: $"{containingTypeRef}.{property.Name}",
GetSectionPathFromConfigurationExpression(property.ConfigurationKeyName),
canSet: property.CanSet,
firstTimeInitialization: false);
InitializationKind.Declaration);
}
}
}
Expand All @@ -795,7 +789,7 @@ private bool EmitBindImplForMember(
string memberAccessExpr,
string sectionPathExpr,
bool canSet,
bool firstTimeInitialization)
InitializationKind initializationKind)
{
TypeSpec effectiveMemberType = member.Type.EffectiveType;

Expand All @@ -807,7 +801,8 @@ private bool EmitBindImplForMember(
{
if (canSet)
{
bool useDefaultValueIfSectionValueIsNull = !firstTimeInitialization &&
bool useDefaultValueIfSectionValueIsNull =
initializationKind == InitializationKind.Declaration &&
member is PropertySpec &&
member.Type.IsValueType &&
member.Type.SpecKind is not TypeSpecKind.Nullable;
Expand Down Expand Up @@ -968,7 +963,7 @@ private void EmitBindingLogic(

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

if (type.CanInstantiate)
{
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/Helpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ private enum InitializationKind
/// The type of defaulting for a property if it does not have a config entry.
/// This should only be applied for "Get" cases, not "Bind" and is also conditioned
/// on the source generated for a particular property as to whether it uses this value.
/// Note this is different than "InitializationKind.Declaration" since it only applied to
/// complex types and not arrays\enumerables.
/// </summary>
private enum ValueDefaulting
{
Expand Down
1 change: 0 additions & 1 deletion src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Members/MemberSpec.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ public MemberSpec(ISymbol member)
}

public string Name { get; }
public bool ErrorOnFailedBinding { get; protected set; }
public string DefaultValueExpr { get; protected set; }

public required TypeSpec Type { get; init; }
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Members/ParameterSpec.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ public ParameterSpec(IParameterSymbol parameter) : base(parameter)
}
}

public bool ErrorOnFailedBinding { get; private set; }

public RefKind RefKind { get; }

public override bool CanGet => false;
Expand Down
10 changes: 5 additions & 5 deletions ...ions.Configuration.Binder/tests/SourceGenerationTests/Baselines/Collections.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
if (type == typeof(Program.MyClassWithCustomCollections))
{
var instance = new Program.MyClassWithCustomCollections();
BindCore(configuration, ref instance, true, binderOptions);
BindCore(configuration, ref instance, defaultValueIfNotFound: true, binderOptions);
return instance;
}

Expand Down Expand Up @@ -169,31 +169,31 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
Program.CustomDictionary<string, int>? temp3 = instance.CustomDictionary;
temp3 ??= new Program.CustomDictionary<string, int>();
BindCore(section1, ref temp3, false, binderOptions);
BindCore(section1, ref temp3, defaultValueIfNotFound: false, binderOptions);
instance.CustomDictionary = temp3;
}

if (AsConfigWithChildren(configuration.GetSection("CustomList")) is IConfigurationSection section4)
{
Program.CustomList? temp6 = instance.CustomList;
temp6 ??= new Program.CustomList();
BindCore(section4, ref temp6, false, binderOptions);
BindCore(section4, ref temp6, defaultValueIfNotFound: false, binderOptions);
instance.CustomList = temp6;
}

if (AsConfigWithChildren(configuration.GetSection("IReadOnlyList")) is IConfigurationSection section7)
{
IReadOnlyList<int>? temp9 = instance.IReadOnlyList;
temp9 = temp9 is null ? new List<int>() : new List<int>(temp9);
BindCore(section7, ref temp9, false, binderOptions);
BindCore(section7, ref temp9, defaultValueIfNotFound: false, binderOptions);
instance.IReadOnlyList = temp9;
}

if (AsConfigWithChildren(configuration.GetSection("IReadOnlyDictionary")) is IConfigurationSection section10)
{
IReadOnlyDictionary<string, int>? temp12 = instance.IReadOnlyDictionary;
temp12 = temp12 is null ? new Dictionary<string, int>() : temp12.ToDictionary(pair => pair.Key, pair => pair.Value);
BindCore(section10, ref temp12, false, binderOptions);
BindCore(section10, ref temp12, defaultValueIfNotFound: false, binderOptions);
instance.IReadOnlyDictionary = temp12;
}
}
Expand Down
12 changes: 6 additions & 6 deletions ...ation.Binder/tests/SourceGenerationTests/Baselines/ConfigurationBinder/Bind.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}

var typedObj = (Program.MyClass)instance;
BindCore(configuration, ref typedObj, false, binderOptions: null);
BindCore(configuration, ref typedObj, defaultValueIfNotFound: false, binderOptions: null);
}

/// <summary>Attempts to bind the given object instance to configuration values by matching property names against configuration keys recursively.</summary>
Expand All @@ -63,7 +63,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}

var typedObj = (Program.MyClass)instance;
BindCore(configuration, ref typedObj, false, GetBinderOptions(configureOptions));
BindCore(configuration, ref typedObj, defaultValueIfNotFound: false, GetBinderOptions(configureOptions));
}

/// <summary>Attempts to bind the given object instance to configuration values by matching property names against configuration keys recursively.</summary>
Expand All @@ -81,7 +81,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}

var typedObj = (Program.MyClass)instance;
BindCore(configuration.GetSection(key), ref typedObj, false, binderOptions: null);
BindCore(configuration.GetSection(key), ref typedObj, defaultValueIfNotFound: false, binderOptions: null);
}
#endregion IConfiguration extensions.

Expand Down Expand Up @@ -144,23 +144,23 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
List<int>? temp4 = instance.MyList;
temp4 ??= new List<int>();
BindCore(section2, ref temp4, false, binderOptions);
BindCore(section2, ref temp4, defaultValueIfNotFound: false, binderOptions);
instance.MyList = temp4;
}

if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section5)
{
Dictionary<string, string>? temp7 = instance.MyDictionary;
temp7 ??= new Dictionary<string, string>();
BindCore(section5, ref temp7, false, binderOptions);
BindCore(section5, ref temp7, defaultValueIfNotFound: false, binderOptions);
instance.MyDictionary = temp7;
}

if (AsConfigWithChildren(configuration.GetSection("MyComplexDictionary")) is IConfigurationSection section8)
{
Dictionary<string, Program.MyClass2>? temp10 = instance.MyComplexDictionary;
temp10 ??= new Dictionary<string, Program.MyClass2>();
BindCore(section8, ref temp10, false, binderOptions);
BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
instance.MyComplexDictionary = temp10;
}
}
Expand Down
8 changes: 4 additions & 4 deletions ...der/tests/SourceGenerationTests/Baselines/ConfigurationBinder/Bind_Instance.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}

var typedObj = (Program.MyClass)instance;
BindCore(configuration, ref typedObj, false, binderOptions: null);
BindCore(configuration, ref typedObj, defaultValueIfNotFound: false, binderOptions: null);
}
#endregion IConfiguration extensions.

Expand Down Expand Up @@ -108,23 +108,23 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
List<int>? temp4 = instance.MyList;
temp4 ??= new List<int>();
BindCore(section2, ref temp4, false, binderOptions);
BindCore(section2, ref temp4, defaultValueIfNotFound: false, binderOptions);
instance.MyList = temp4;
}

if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section5)
{
Dictionary<string, string>? temp7 = instance.MyDictionary;
temp7 ??= new Dictionary<string, string>();
BindCore(section5, ref temp7, false, binderOptions);
BindCore(section5, ref temp7, defaultValueIfNotFound: false, binderOptions);
instance.MyDictionary = temp7;
}

if (AsConfigWithChildren(configuration.GetSection("MyComplexDictionary")) is IConfigurationSection section8)
{
Dictionary<string, Program.MyClass2>? temp10 = instance.MyComplexDictionary;
temp10 ??= new Dictionary<string, Program.MyClass2>();
BindCore(section8, ref temp10, false, binderOptions);
BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
instance.MyComplexDictionary = temp10;
}
}
Expand Down
8 changes: 4 additions & 4 deletions ...ceGenerationTests/Baselines/ConfigurationBinder/Bind_Instance_BinderOptions.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}

var typedObj = (Program.MyClass)instance;
BindCore(configuration, ref typedObj, false, GetBinderOptions(configureOptions));
BindCore(configuration, ref typedObj, defaultValueIfNotFound: false, GetBinderOptions(configureOptions));
}
#endregion IConfiguration extensions.

Expand Down Expand Up @@ -108,23 +108,23 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
List<int>? temp4 = instance.MyList;
temp4 ??= new List<int>();
BindCore(section2, ref temp4, false, binderOptions);
BindCore(section2, ref temp4, defaultValueIfNotFound: false, binderOptions);
instance.MyList = temp4;
}

if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section5)
{
Dictionary<string, string>? temp7 = instance.MyDictionary;
temp7 ??= new Dictionary<string, string>();
BindCore(section5, ref temp7, false, binderOptions);
BindCore(section5, ref temp7, defaultValueIfNotFound: false, binderOptions);
instance.MyDictionary = temp7;
}

if (AsConfigWithChildren(configuration.GetSection("MyComplexDictionary")) is IConfigurationSection section8)
{
Dictionary<string, Program.MyClass2>? temp10 = instance.MyComplexDictionary;
temp10 ??= new Dictionary<string, Program.MyClass2>();
BindCore(section8, ref temp10, false, binderOptions);
BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
instance.MyComplexDictionary = temp10;
}
}
Expand Down
8 changes: 4 additions & 4 deletions ...tests/SourceGenerationTests/Baselines/ConfigurationBinder/Bind_Key_Instance.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}

var typedObj = (Program.MyClass)instance;
BindCore(configuration.GetSection(key), ref typedObj, false, binderOptions: null);
BindCore(configuration.GetSection(key), ref typedObj, defaultValueIfNotFound: false, binderOptions: null);
}
#endregion IConfiguration extensions.

Expand Down Expand Up @@ -108,23 +108,23 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
List<int>? temp4 = instance.MyList;
temp4 ??= new List<int>();
BindCore(section2, ref temp4, false, binderOptions);
BindCore(section2, ref temp4, defaultValueIfNotFound: false, binderOptions);
instance.MyList = temp4;
}

if (AsConfigWithChildren(configuration.GetSection("MyDictionary")) is IConfigurationSection section5)
{
Dictionary<string, string>? temp7 = instance.MyDictionary;
temp7 ??= new Dictionary<string, string>();
BindCore(section5, ref temp7, false, binderOptions);
BindCore(section5, ref temp7, defaultValueIfNotFound: false, binderOptions);
instance.MyDictionary = temp7;
}

if (AsConfigWithChildren(configuration.GetSection("MyComplexDictionary")) is IConfigurationSection section8)
{
Dictionary<string, Program.MyClass2>? temp10 = instance.MyComplexDictionary;
temp10 ??= new Dictionary<string, Program.MyClass2>();
BindCore(section8, ref temp10, false, binderOptions);
BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
instance.MyComplexDictionary = temp10;
}
}
Expand Down
Loading