Skip to content

Commit

Permalink
[release/8.0] Make src gen for property setters consistent with refle…
Browse files Browse the repository at this point in the history 
…ction (dotnet#92167)

* Make src gen for property setters consistent with reflection

* Don't default value during initialization

* Remove unnecessary bang operator

* Don't default a property when it is a collection or child type

* Simply use of new variable; add to test

* Review feedback

---------

Co-authored-by: Steve Harter <steveharter@users.noreply.github.com>
  • Loading branch information
@github-actions @steveharter
github-actions[bot] and steveharter committed Sep 18, 2023
1 parent 644856c commit 1cbf763
Show file tree
Hide file tree
Showing 27 changed files with 632 additions and 185 deletions.
2 changes: 1 addition & 1 deletion src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/ConfigurationBinder.cs
View file
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}}, {{binderOptionsArg}});
{{nameof(MethodsToGen_CoreBindingHelper.BindCore)}}({{configExpression}}, ref {{Identifier.typedObj}}, defaultValueIfNotFound: false, {{binderOptionsArg}});
""");
}

Expand Down
66 changes: 44 additions & 22 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 @@ -97,6 +97,7 @@ private void EmitGetCoreMethod()
Expression.sectionPath,
writeOnSuccess: parsedValueExpr => _writer.WriteLine($"return {parsedValueExpr};"),
checkForNullSectionValue: stringParsableType.StringParsableTypeKind is not StringParsableTypeKind.AssignFromSectionValue,
useDefaultValueIfSectionValueIsNull: false,
useIncrementalStringValueIdentifier: false);
}
break;
Expand All @@ -110,7 +111,7 @@ private void EmitGetCoreMethod()
{
if (complexType.CanInstantiate)
{
EmitBindingLogic(complexType, Identifier.instance, Identifier.configuration, InitializationKind.Declaration);
EmitBindingLogic(complexType, Identifier.instance, Identifier.configuration, InitializationKind.Declaration, ValueDefaulting.CallSetter);
_writer.WriteLine($"return {Identifier.instance};");
}
else if (type is ObjectSpec { InitExceptionMessage: string exMsg })
Expand Down Expand Up @@ -173,6 +174,7 @@ private void EmitGetValueCoreMethod()
Expression.sectionPath,
writeOnSuccess: (parsedValueExpr) => _writer.WriteLine($"return {parsedValueExpr};"),
checkForNullSectionValue: false,
useDefaultValueIfSectionValueIsNull: false,
useIncrementalStringValueIdentifier: false);

EmitEndBlock();
Expand Down Expand Up @@ -207,7 +209,7 @@ private void EmitBindCoreMainMethod()

EmitStartBlock($"{conditionKindExpr} ({Identifier.type} == typeof({type.DisplayString}))");
_writer.WriteLine($"var {Identifier.temp} = ({effectiveType.DisplayString}){Identifier.instance};");
EmitBindingLogic(type, Identifier.temp, Identifier.configuration, InitializationKind.None);
EmitBindingLogic(type, Identifier.temp, Identifier.configuration, InitializationKind.None, ValueDefaulting.None);
_writer.WriteLine($"return;");
EmitEndBlock();
}
Expand Down Expand Up @@ -235,7 +237,7 @@ private void EmitBindCoreMethods()
private void EmitBindCoreMethod(ComplexTypeSpec type)
{
string objParameterExpression = $"ref {type.DisplayString} {Identifier.instance}";
EmitStartBlock(@$"public static void {nameof(MethodsToGen_CoreBindingHelper.BindCore)}({Identifier.IConfiguration} {Identifier.configuration}, {objParameterExpression}, {Identifier.BinderOptions}? {Identifier.binderOptions})");
EmitStartBlock(@$"public static void {nameof(MethodsToGen_CoreBindingHelper.BindCore)}({Identifier.IConfiguration} {Identifier.configuration}, {objParameterExpression}, bool defaultValueIfNotFound, {Identifier.BinderOptions}? {Identifier.binderOptions})");

ComplexTypeSpec effectiveType = (ComplexTypeSpec)type.EffectiveType;
if (effectiveType is EnumerableSpec enumerable)
Expand Down Expand Up @@ -334,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 @@ -344,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 @@ -377,11 +377,12 @@ void EmitBindImplForMember(MemberSpec member)
member,
parsedMemberAssignmentLhsExpr,
sectionPathExpr: GetSectionPathFromConfigurationExpression(configKeyName),
canSet: true);
canSet: 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 @@ -633,7 +634,7 @@ private void EmitPopulationImplForArray(EnumerableSpec type)

// Create list and bind elements.
string tempIdentifier = GetIncrementalIdentifier(Identifier.temp);
EmitBindingLogic(typeToInstantiate, tempIdentifier, Identifier.configuration, InitializationKind.Declaration);
EmitBindingLogic(typeToInstantiate, tempIdentifier, Identifier.configuration, InitializationKind.Declaration, ValueDefaulting.None);

// Resize array and add binded elements.
_writer.WriteLine($$"""
Expand Down Expand Up @@ -661,6 +662,7 @@ private void EmitPopulationImplForEnumerableWithAdd(EnumerableSpec type)
Expression.sectionPath,
(parsedValueExpr) => _writer.WriteLine($"{addExpr}({parsedValueExpr});"),
checkForNullSectionValue: true,
useDefaultValueIfSectionValueIsNull: false,
useIncrementalStringValueIdentifier: false);
}
break;
Expand All @@ -671,7 +673,7 @@ private void EmitPopulationImplForEnumerableWithAdd(EnumerableSpec type)
break;
case ComplexTypeSpec { CanInstantiate: true } complexType:
{
EmitBindingLogic(complexType, Identifier.value, Identifier.section, InitializationKind.Declaration);
EmitBindingLogic(complexType, Identifier.value, Identifier.section, InitializationKind.Declaration, ValueDefaulting.None);
_writer.WriteLine($"{addExpr}({Identifier.value});");
}
break;
Expand All @@ -696,6 +698,7 @@ private void EmitBindCoreImplForDictionary(DictionarySpec type)
Expression.sectionPath,
Emit_BindAndAddLogic_ForElement,
checkForNullSectionValue: false,
useDefaultValueIfSectionValueIsNull: false,
useIncrementalStringValueIdentifier: false);

void Emit_BindAndAddLogic_ForElement(string parsedKeyExpr)
Expand All @@ -710,6 +713,7 @@ void Emit_BindAndAddLogic_ForElement(string parsedKeyExpr)
Expression.sectionPath,
writeOnSuccess: parsedValueExpr => _writer.WriteLine($"{instanceIdentifier}[{parsedKeyExpr}] = {parsedValueExpr};"),
checkForNullSectionValue: true,
useDefaultValueIfSectionValueIsNull: false,
useIncrementalStringValueIdentifier: false);
}
break;
Expand Down Expand Up @@ -746,7 +750,7 @@ void Emit_BindAndAddLogic_ForElement(string parsedKeyExpr)
EmitObjectInit(complexElementType, Identifier.element, InitializationKind.SimpleAssignment, Identifier.section);
EmitEndBlock();

EmitBindingLogic(complexElementType, Identifier.element, Identifier.section, InitializationKind.None);
EmitBindingLogic(complexElementType, Identifier.element, Identifier.section, InitializationKind.None, ValueDefaulting.None);
_writer.WriteLine($"{instanceIdentifier}[{parsedKeyExpr}] = {Identifier.element};");
}
break;
Expand Down Expand Up @@ -774,7 +778,8 @@ private void EmitBindCoreImplForObject(ObjectSpec type)
property,
memberAccessExpr: $"{containingTypeRef}.{property.Name}",
GetSectionPathFromConfigurationExpression(property.ConfigurationKeyName),
canSet: property.CanSet);
canSet: property.CanSet,
InitializationKind.Declaration);
}
}
}
Expand All @@ -783,9 +788,11 @@ private bool EmitBindImplForMember(
MemberSpec member,
string memberAccessExpr,
string sectionPathExpr,
bool canSet)
bool canSet,
InitializationKind initializationKind)
{
TypeSpec effectiveMemberType = member.Type.EffectiveType;

string sectionParseExpr = GetSectionFromConfigurationExpression(member.ConfigurationKeyName);

switch (effectiveMemberType)
Expand All @@ -794,19 +801,20 @@ private bool EmitBindImplForMember(
{
if (canSet)
{
bool checkForNullSectionValue = member is ParameterSpec
? true
: stringParsableType.StringParsableTypeKind is not StringParsableTypeKind.AssignFromSectionValue;

string nullBangExpr = checkForNullSectionValue ? string.Empty : "!";
bool useDefaultValueIfSectionValueIsNull =
initializationKind == InitializationKind.Declaration &&
member is PropertySpec &&
member.Type.IsValueType &&
member.Type.SpecKind is not TypeSpecKind.Nullable;

EmitBlankLineIfRequired();
EmitBindingLogic(
stringParsableType,
$@"{Identifier.configuration}[""{member.ConfigurationKeyName}""]",
sectionPathExpr,
writeOnSuccess: parsedValueExpr => _writer.WriteLine($"{memberAccessExpr} = {parsedValueExpr}{nullBangExpr};"),
checkForNullSectionValue,
writeOnSuccess: parsedValueExpr => _writer.WriteLine($"{memberAccessExpr} = {parsedValueExpr};"),
checkForNullSectionValue: true,
useDefaultValueIfSectionValueIsNull,
useIncrementalStringValueIdentifier: true);
}

Expand Down Expand Up @@ -906,14 +914,17 @@ private void EmitBindingLogicForComplexMember(
targetObjAccessExpr,
configArgExpr,
initKind,
writeOnSuccess);
ValueDefaulting.None,
writeOnSuccess
);
}

private void EmitBindingLogic(
ComplexTypeSpec type,
string memberAccessExpr,
string configArgExpr,
InitializationKind initKind,
ValueDefaulting valueDefaulting,
Action<string>? writeOnSuccess = null)
{
if (!type.HasBindableMembers)
Expand Down Expand Up @@ -952,7 +963,7 @@ private void EmitBindingLogic(

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

if (type.CanInstantiate)
{
Expand Down Expand Up @@ -984,6 +995,8 @@ void EmitBindCoreCall()
_writer.WriteLine(bindCoreCall);
writeOnSuccess?.Invoke(instanceToBindExpr);
}

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

Expand All @@ -993,6 +1006,7 @@ private void EmitBindingLogic(
string sectionPathExpr,
Action<string>? writeOnSuccess,
bool checkForNullSectionValue,
bool useDefaultValueIfSectionValueIsNull,
bool useIncrementalStringValueIdentifier)
{
StringParsableTypeKind typeKind = type.StringParsableTypeKind;
Expand All @@ -1018,6 +1032,14 @@ private void EmitBindingLogic(
EmitEndBlock();
}

if (useDefaultValueIfSectionValueIsNull)
{
parsedValueExpr = $"default";
EmitStartBlock($"else if (defaultValueIfNotFound)");
InvokeWriteOnSuccess();
EmitEndBlock();
}

void InvokeWriteOnSuccess() => writeOnSuccess?.Invoke(parsedValueExpr);
}

Expand Down
17 changes: 17 additions & 0 deletions src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/Helpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,23 @@ private enum InitializationKind
Declaration = 3,
}

/// <summary>
/// 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
{
None = 0,

/// <summary>
/// Call the setter with the default value for the property's Type.
/// </summary>
CallSetter = 1,
}

private static class Expression
{
public const string configurationGetSection = "configuration.GetSection";
Expand Down
1 change: 0 additions & 1 deletion src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Members/MemberSpec.cs
View file
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
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
2 changes: 1 addition & 1 deletion src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Types/SimpleTypeSpec.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ internal enum StringParsableTypeKind
None = 0,

/// <summary>
/// Declared types that can be assigned directly from IConfigurationSection.Value, i.e. string and tyepof(object).
/// Declared types that can be assigned directly from IConfigurationSection.Value, i.e. string and typeof(object).
/// </summary>
AssignFromSectionValue = 1,
Enum = 2,
Expand Down
Loading

0 comments on commit 1cbf763

Please sign in to comment.