From b2c6255ad5bd447f8f4c38b3b66bb8b863a27144 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Mon, 28 Mar 2022 21:09:05 +0100 Subject: [PATCH 01/41] Implementation --- .../src/ConfigurationBinder.cs | 95 +++++++++- .../tests/ConfigurationBinderTests.cs | 170 ++++++++++++++++++ 2 files changed, 257 insertions(+), 8 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index e56a0875bd46ec..65646b906e8160 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Buffers; using System.Collections; using System.Collections.Generic; using System.ComponentModel; @@ -381,7 +382,7 @@ private static void BindInstance( return; // We are already done if binding to a new collection instance worked } - bindingPoint.SetValue(CreateInstance(type)); + bindingPoint.SetValue(CreateInstance(type, config, options)); } // See if it's a Dictionary @@ -408,21 +409,54 @@ private static void BindInstance( } [RequiresUnreferencedCode("In case type is a Nullable, cannot statically analyze what the underlying type is so its members may be trimmed.")] - private static object CreateInstance([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type type) + private static object CreateInstance([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type type, + IConfiguration config, + BinderOptions options) { Debug.Assert(!type.IsArray); - if (type.IsAbstract) + if (type.IsInterface || type.IsAbstract) { throw new InvalidOperationException(SR.Format(SR.Error_CannotActivateAbstractOrInterface, type)); } if (!type.IsValueType) { - bool hasDefaultConstructor = type.GetConstructors(DeclaredOnlyLookup).Any(ctor => ctor.IsPublic && ctor.GetParameters().Length == 0); - if (!hasDefaultConstructor) + ConstructorInfo[] constructors = type.GetConstructors(DeclaredOnlyLookup); + + bool hasParameterlessPublicConstructor = constructors.Any(ctor => ctor.IsPublic && ctor.GetParameters().Length == 0); + + if (!hasParameterlessPublicConstructor) { - throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); + // if the only constructor is private, then throw + if (constructors.Length == 1 && !constructors[0].IsPublic) + { + throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); + } + + // find the biggest constructor so that we can bind to the most parameters + ParameterInfo[] parameters = constructors[0].GetParameters(); + + int indexOfChosenConstructor = 0; + + for (int index = 1; index < constructors.Length; index++) + { + ParameterInfo[] constructorParameters = constructors[index].GetParameters(); + if (constructorParameters.Length > parameters.Length) + { + parameters = constructorParameters; + indexOfChosenConstructor = index; + } + } + + object?[] parameterValues = new object?[parameters.Length]; + + for (int index = 0; index < parameters.Length; index++) + { + parameterValues[index] = GetParameterValue(parameters[index], config, options); + } + + return constructors[indexOfChosenConstructor].Invoke(parameterValues); } } @@ -687,10 +721,55 @@ private static List GetAllProperties( return allProperties; } - private static string GetPropertyName(MemberInfo property) + [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] + private static object? GetParameterValue(ParameterInfo property, IConfiguration config, BinderOptions options) { - ThrowHelper.ThrowIfNull(property); + string parameterName = GetParameterName(property); + BindingPoint bindingPoint = new BindingPoint(); + + BindInstance( + property.ParameterType, + bindingPoint, + config.GetSection(parameterName), + options); + + return bindingPoint.Value; + } + + // todo: steve - we might not need this; currently, these attributes are only applicable to properties and + // not parameters. We might be able to get rid of this method once confirm whether it's needed or not. + + private static string GetParameterName(ParameterInfo parameter) + { + // Check for a custom property name used for configuration key binding + foreach (var attributeData in parameter.GetCustomAttributesData()) + { + if (attributeData.AttributeType != typeof(ConfigurationKeyNameAttribute)) + { + continue; + } + + // Ensure ConfigurationKeyName constructor signature matches expectations + if (attributeData.ConstructorArguments.Count != 1) + { + break; + } + + // Assumes ConfigurationKeyName constructor first arg is the string key name + string? name = attributeData + .ConstructorArguments[0] + .Value? + .ToString(); + + return !string.IsNullOrWhiteSpace(name) ? name : parameter.Name!; + } + + return parameter.Name!; + } + + private static string GetPropertyName(MemberInfo property) + { // Check for a custom property name used for configuration key binding foreach (var attributeData in property.GetCustomAttributesData()) { diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 04e3ca51977627..9c25fc9453282c 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -112,6 +112,75 @@ public class DerivedOptionsWithIConfigurationSection : DerivedOptions public IConfigurationSection DerivedSection { get; set; } } +#if NETCOREAPP + public record RecordTypeOptions(string Color, int Length); +#endif + + public class ContainerWithNestedImmutableObject + { + public string ContainerName { get; set; } + public ImmutableLengthAndColorClass LengthAndColor { get; set; } + } + + public class ImmutableLengthAndColorClass + { + public ImmutableLengthAndColorClass(string color, int length) + { + Color = color; + Length = length; + } + + public string Color { get; } + public int Length { get; } + } + + public class ImmutableClassWithConstructorOverloads + { + public ImmutableClassWithConstructorOverloads(string string1, int int1) + { + String1 = string1; + Int1 = int1; + } + + public ImmutableClassWithConstructorOverloads(string string1, int int1, string string2) + { + String1 = string1; + Int1 = int1; + String2 = string2; + } + + public ImmutableClassWithConstructorOverloads(string string1, int int1, string string2, int int2) + { + String1 = string1; + Int1 = int1; + String2 = string2; + Int2 = int2; + } + + public ImmutableClassWithConstructorOverloads(string string1) + { + String1 = string1; + } + + public string String1 { get; } + public string String2 { get; } + public int Int1 { get; } + public int Int2 { get; } + } + + public class SemiImmutableType + { + public SemiImmutableType(string color, int length) + { + Color = color; + Length = length; + } + + public string Color { get; } + public int Length { get; } + public decimal Thickness { get; set; } + } + public struct ValueTypeOptions { public int MyInt32 { get; set; } @@ -994,6 +1063,107 @@ public void CanBindValueTypeOptions() Assert.Equal("hello world", options.MyString); } + [Fact] + public void CanBindImmutableClass() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("Green", options.Color); + } + + [Fact] + public void CanBindMutableClassWitNestedImmutableObject() + { + var dic = new Dictionary + { + {"ContainerName", "Container123"}, + {"LengthAndColor:Length", "42"}, + {"LengthAndColor:Color", "Green"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal("Container123", options.ContainerName); + Assert.Equal(42, options.LengthAndColor.Length); + Assert.Equal("Green", options.LengthAndColor.Color); + } + + // If the immutable type has multiple constructors, + // then pick the one with the most parameters + // and try to bind as many as possible. + // The type used in example below has multiple constructors in + // an arbitrary order, but/ with the biggest (chosen) one + // deliberately not first or last. + [Fact] + public void CanBindImmutableClass_PicksBiggestNonParameterlessConstructor() + { + var dic = new Dictionary + { + {"String1", "s1"}, + {"Int1", "1"}, + {"String2", "s2"}, + {"Int2", "2"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal("s1", options.String1); + Assert.Equal("s2", options.String2); + Assert.Equal(1, options.Int1); + Assert.Equal(2, options.Int2); + } + + [Fact] + public void CanBindSemiImmutableClass() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + {"Thickness", "1.23"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("Green", options.Color); + Assert.Equal(1.23m, options.Thickness); + } + +#if NETCOREAPP + [Fact] + public void CanBindRecordOptions() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("Green", options.Color); + } +#endif + [Fact] public void CanBindByteArray() { From 1d8d70b04292725a0b637a2175cf96d76663e3b8 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Sat, 2 Apr 2022 14:19:33 +0100 Subject: [PATCH 02/41] PR feedback: refactor out duplication getting name from attributes --- .../src/ConfigurationBinder.cs | 38 ++++++------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 65646b906e8160..5ce3314db9b11e 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -740,38 +740,24 @@ private static List GetAllProperties( // todo: steve - we might not need this; currently, these attributes are only applicable to properties and // not parameters. We might be able to get rid of this method once confirm whether it's needed or not. - private static string GetParameterName(ParameterInfo parameter) + private static string GetParameterName(ParameterInfo parameter!!) { - // Check for a custom property name used for configuration key binding - foreach (var attributeData in parameter.GetCustomAttributesData()) - { - if (attributeData.AttributeType != typeof(ConfigurationKeyNameAttribute)) - { - continue; - } - - // Ensure ConfigurationKeyName constructor signature matches expectations - if (attributeData.ConstructorArguments.Count != 1) - { - break; - } + var customAttributesData = parameter.GetCustomAttributesData(); - // Assumes ConfigurationKeyName constructor first arg is the string key name - string? name = attributeData - .ConstructorArguments[0] - .Value? - .ToString(); + return TryGetNameFromAttributes(customAttributesData) ?? parameter.Name!; + } - return !string.IsNullOrWhiteSpace(name) ? name : parameter.Name!; - } + private static string GetPropertyName(MemberInfo property) + { + var customAttributesData = property.GetCustomAttributesData(); - return parameter.Name!; + return TryGetNameFromAttributes(customAttributesData) ?? property.Name; } - private static string GetPropertyName(MemberInfo property) + private static string? TryGetNameFromAttributes(IList customAttributesData) { // Check for a custom property name used for configuration key binding - foreach (var attributeData in property.GetCustomAttributesData()) + foreach (var attributeData in customAttributesData) { if (attributeData.AttributeType != typeof(ConfigurationKeyNameAttribute)) { @@ -790,10 +776,10 @@ private static string GetPropertyName(MemberInfo property) .Value? .ToString(); - return !string.IsNullOrWhiteSpace(name) ? name : property.Name; + return !string.IsNullOrWhiteSpace(name) ? name : null; } - return property.Name; + return null; } } } From a51db1a91f8ee1b744b9895192b677f89b3cc19d Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Mon, 4 Apr 2022 21:57:11 +0100 Subject: [PATCH 03/41] PR feedback --- .../tests/ConfigurationBinderTests.cs | 4 ---- .../Microsoft.Extensions.Configuration.Binder.Tests.csproj | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 9c25fc9453282c..3c83e48ea89e12 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -112,9 +112,7 @@ public class DerivedOptionsWithIConfigurationSection : DerivedOptions public IConfigurationSection DerivedSection { get; set; } } -#if NETCOREAPP public record RecordTypeOptions(string Color, int Length); -#endif public class ContainerWithNestedImmutableObject { @@ -1145,7 +1143,6 @@ public void CanBindSemiImmutableClass() Assert.Equal(1.23m, options.Thickness); } -#if NETCOREAPP [Fact] public void CanBindRecordOptions() { @@ -1162,7 +1159,6 @@ public void CanBindRecordOptions() Assert.Equal(42, options.Length); Assert.Equal("Green", options.Color); } -#endif [Fact] public void CanBindByteArray() diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Microsoft.Extensions.Configuration.Binder.Tests.csproj b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Microsoft.Extensions.Configuration.Binder.Tests.csproj index 8fcc6f146609b7..c15ba1c1c69d1b 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Microsoft.Extensions.Configuration.Binder.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Microsoft.Extensions.Configuration.Binder.Tests.csproj @@ -10,6 +10,7 @@ Link="Common\ConfigurationProviderExtensions.cs" /> + From 0bdd214c3a10dc0e2d21599228175eac5f1999cd Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Mon, 4 Apr 2022 22:10:08 +0100 Subject: [PATCH 04/41] PR feedback: allow creation of value types --- .../src/ConfigurationBinder.cs | 50 ++++++++++--------- .../tests/ConfigurationBinderTests.cs | 48 ++++++++++++++++++ 2 files changed, 74 insertions(+), 24 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 5ce3314db9b11e..324c2864850550 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -423,41 +423,43 @@ private static object CreateInstance([DynamicallyAccessedMembers(DynamicallyAcce if (!type.IsValueType) { ConstructorInfo[] constructors = type.GetConstructors(DeclaredOnlyLookup); + { + ConstructorInfo[] constructors = type.GetConstructors(DeclaredOnlyLookup); - bool hasParameterlessPublicConstructor = constructors.Any(ctor => ctor.IsPublic && ctor.GetParameters().Length == 0); + bool hasParameterlessPublicConstructor = + constructors.Any(ctor => ctor.IsPublic && ctor.GetParameters().Length == 0); - if (!hasParameterlessPublicConstructor) + if (constructors.Length > 0 && !hasParameterlessPublicConstructor) + { + // if the only constructor is private, then throw + if (constructors.Length == 1 && !constructors[0].IsPublic) { - // if the only constructor is private, then throw - if (constructors.Length == 1 && !constructors[0].IsPublic) - { - throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); - } + throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); + } - // find the biggest constructor so that we can bind to the most parameters - ParameterInfo[] parameters = constructors[0].GetParameters(); + // find the biggest constructor so that we can bind to the most parameters + ParameterInfo[] parameters = constructors[0].GetParameters(); - int indexOfChosenConstructor = 0; + int indexOfChosenConstructor = 0; - for (int index = 1; index < constructors.Length; index++) + for (int index = 1; index < constructors.Length; index++) + { + ParameterInfo[] constructorParameters = constructors[index].GetParameters(); + if (constructorParameters.Length > parameters.Length) { - ParameterInfo[] constructorParameters = constructors[index].GetParameters(); - if (constructorParameters.Length > parameters.Length) - { - parameters = constructorParameters; - indexOfChosenConstructor = index; - } + parameters = constructorParameters; + indexOfChosenConstructor = index; } + } - object?[] parameterValues = new object?[parameters.Length]; + object?[] parameterValues = new object?[parameters.Length]; - for (int index = 0; index < parameters.Length; index++) - { - parameterValues[index] = GetParameterValue(parameters[index], config, options); - } - - return constructors[indexOfChosenConstructor].Invoke(parameterValues); + for (int index = 0; index < parameters.Length; index++) + { + parameterValues[index] = GetParameterValue(parameters[index], config, options); } + + return constructors[indexOfChosenConstructor].Invoke(parameterValues); } object? instance; diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 3c83e48ea89e12..63deffc705ba92 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -114,6 +114,20 @@ public class DerivedOptionsWithIConfigurationSection : DerivedOptions public record RecordTypeOptions(string Color, int Length); + public record struct RecordStructTypeOptions(string Color, int Length); + + public struct StructTypeOptions + { + public StructTypeOptions(string color, int length) + { + Color = color; + Length = length; + } + + public string Color { get; } + public int Length { get; } + } + public class ContainerWithNestedImmutableObject { public string ContainerName { get; set; } @@ -1160,6 +1174,40 @@ public void CanBindRecordOptions() Assert.Equal("Green", options.Color); } + [Fact] + public void CanBindStructOptions() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("Green", options.Color); + } + + [Fact] + public void CanBindRecordStructOptions() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("Green", options.Color); + } + [Fact] public void CanBindByteArray() { From d71d14ae4afb8892621487bb37171b21bc838972 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Mon, 4 Apr 2022 22:56:12 +0100 Subject: [PATCH 05/41] Post merge fixes --- .../src/ConfigurationBinder.cs | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 324c2864850550..673dae35173408 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -329,9 +329,10 @@ private static object BindToCollection(Type type, IConfiguration config, BinderO [RequiresUnreferencedCode(TrimmingWarningMessage)] private static void BindInstance( - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] - Type type, - BindingPoint bindingPoint, IConfiguration config, BinderOptions options) + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type type, + BindingPoint bindingPoint, + IConfiguration config, + BinderOptions options) { // if binding IConfigurationSection, break early if (type == typeof(IConfigurationSection)) @@ -351,7 +352,6 @@ private static void BindInstance( // Leaf nodes are always reinitialized bindingPoint.TrySetValue(convertedValue); - return; } if (config != null && config.GetChildren().Any()) @@ -408,8 +408,12 @@ private static void BindInstance( } } - [RequiresUnreferencedCode("In case type is a Nullable, cannot statically analyze what the underlying type is so its members may be trimmed.")] - private static object CreateInstance([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type type, + [RequiresUnreferencedCode( + "In case type is a Nullable, cannot statically analyze what the underlying type is so its members may be trimmed.")] + private static object CreateInstance( + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | + DynamicallyAccessedMemberTypes.NonPublicConstructors)] + Type type, IConfiguration config, BinderOptions options) { @@ -420,11 +424,7 @@ private static object CreateInstance([DynamicallyAccessedMembers(DynamicallyAcce throw new InvalidOperationException(SR.Format(SR.Error_CannotActivateAbstractOrInterface, type)); } - if (!type.IsValueType) - { - ConstructorInfo[] constructors = type.GetConstructors(DeclaredOnlyLookup); - { - ConstructorInfo[] constructors = type.GetConstructors(DeclaredOnlyLookup); + ConstructorInfo[] constructors = type.GetConstructors(DeclaredOnlyLookup); bool hasParameterlessPublicConstructor = constructors.Any(ctor => ctor.IsPublic && ctor.GetParameters().Length == 0); @@ -728,21 +728,24 @@ private static List GetAllProperties( { string parameterName = GetParameterName(property); - BindingPoint bindingPoint = new BindingPoint(); + var propertyBindingPoint = new BindingPoint( + initialValueProvider: () => config.GetSection(parameterName).Value, + isReadOnly: false); BindInstance( property.ParameterType, - bindingPoint, + propertyBindingPoint, config.GetSection(parameterName), options); - return bindingPoint.Value; + return propertyBindingPoint.Value; } + // todo: steve - we might not need this; currently, these attributes are only applicable to properties and // not parameters. We might be able to get rid of this method once confirm whether it's needed or not. - private static string GetParameterName(ParameterInfo parameter!!) + private static string GetParameterName(ParameterInfo parameter) { var customAttributesData = parameter.GetCustomAttributesData(); From a90ae612a4632afb57e065d3975b8f46773d8867 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Mon, 4 Apr 2022 22:58:52 +0100 Subject: [PATCH 06/41] Post merge fixes and remove unused method. --- .../src/ConfigurationBinder.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 673dae35173408..1e73d433836e3d 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Buffers; using System.Collections; using System.Collections.Generic; using System.ComponentModel; @@ -726,7 +725,7 @@ private static List GetAllProperties( [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] private static object? GetParameterValue(ParameterInfo property, IConfiguration config, BinderOptions options) { - string parameterName = GetParameterName(property); + string parameterName = property.Name!; var propertyBindingPoint = new BindingPoint( initialValueProvider: () => config.GetSection(parameterName).Value, From 9bb4cbf83d854ae4a703adcf8a27e19670582eda Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Tue, 5 Apr 2022 05:49:40 +0100 Subject: [PATCH 07/41] PR feedback: use explicit value when binding to constructor params --- .../src/ConfigurationBinder.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 1e73d433836e3d..884ed1b039dcf3 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -727,9 +727,7 @@ private static List GetAllProperties( { string parameterName = property.Name!; - var propertyBindingPoint = new BindingPoint( - initialValueProvider: () => config.GetSection(parameterName).Value, - isReadOnly: false); + var propertyBindingPoint = new BindingPoint(initialValue: config.GetSection(parameterName).Value, isReadOnly: false); BindInstance( property.ParameterType, From e59e8efbb4d228c74cc0e72e58d484614fb12cd2 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Tue, 5 Apr 2022 07:10:54 +0100 Subject: [PATCH 08/41] PR feedback: filter out `out` and `ref` --- .../src/ConfigurationBinder.cs | 46 +++++++++++++---- .../tests/ConfigurationBinderTests.cs | 50 +++++++++++++++++++ 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 884ed1b039dcf3..30a936f22d7184 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -437,28 +437,54 @@ private static object CreateInstance( } // find the biggest constructor so that we can bind to the most parameters - ParameterInfo[] parameters = constructors[0].GetParameters(); + ParameterInfo[]? parameters = null; int indexOfChosenConstructor = 0; - for (int index = 1; index < constructors.Length; index++) + for (int index = 0; index < constructors.Length; index++) { - ParameterInfo[] constructorParameters = constructors[index].GetParameters(); - if (constructorParameters.Length > parameters.Length) + ConstructorInfo constructor = constructors[index]; + + ParameterInfo[] constructorParameters = constructor.GetParameters(); + + bool canUse = true; + + foreach (ParameterInfo p in constructorParameters) + { + if (p.IsOut) + { + canUse = false; + break; + } + + // `in` is OK, but filter out `ref` + if (!p.IsIn && p.ParameterType.IsByRef) + { + canUse = false; + break; + } + } + + if (!canUse) continue; + + if (parameters is null || constructorParameters.Length > parameters?.Length) { parameters = constructorParameters; indexOfChosenConstructor = index; } } - object?[] parameterValues = new object?[parameters.Length]; - - for (int index = 0; index < parameters.Length; index++) + if (parameters != null) { - parameterValues[index] = GetParameterValue(parameters[index], config, options); - } + object?[] parameterValues = new object?[parameters.Length]; - return constructors[indexOfChosenConstructor].Invoke(parameterValues); + for (int index = 0; index < parameters.Length; index++) + { + parameterValues[index] = GetParameterValue(parameters[index], config, options); + } + + return constructors[indexOfChosenConstructor].Invoke(parameterValues); + } } object? instance; diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 63deffc705ba92..0f1672b232b14a 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -112,6 +112,39 @@ public class DerivedOptionsWithIConfigurationSection : DerivedOptions public IConfigurationSection DerivedSection { get; set; } } + public class ClassWithInAndOutAndRefParameters + { + public ClassWithInAndOutAndRefParameters(in string color, int length) + { + Color = color; + Length = length; + } + + public ClassWithInAndOutAndRefParameters(in string color, ref int length) + { + Color = color; + Length = length; + } + + public ClassWithInAndOutAndRefParameters(in string color, int length, out string colorUpperCase) + { + Color = color; + Length = length; + colorUpperCase = color.ToUpper(); + } + + public ClassWithInAndOutAndRefParameters(in string color, int length, out string colorUpperCase, ref int alternativeLength) + { + Color = color; + Length = length < 100 ? length : alternativeLength; + colorUpperCase = color.ToUpper(); + } + + + public string Color { get; } + public int Length { get; } + } + public record RecordTypeOptions(string Color, int Length); public record struct RecordStructTypeOptions(string Color, int Length); @@ -1092,6 +1125,23 @@ public void CanBindImmutableClass() Assert.Equal("Green", options.Color); } + [Fact] + public void CanBindImmutableClassAndSkipsConstructorsWithOutAndRefParameters() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("Green", options.Color); + } + [Fact] public void CanBindMutableClassWitNestedImmutableObject() { From 94e5c2b5b872fb738231d3ce0e9ea0dc446fb44e Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Tue, 5 Apr 2022 07:14:55 +0100 Subject: [PATCH 09/41] Refactor --- .../src/ConfigurationBinder.cs | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 30a936f22d7184..9875da6201bc7b 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -437,9 +437,8 @@ private static object CreateInstance( } // find the biggest constructor so that we can bind to the most parameters - ParameterInfo[]? parameters = null; - - int indexOfChosenConstructor = 0; + ParameterInfo[]? chosenParameters = null; + ConstructorInfo? chosenConstructor = null; for (int index = 0; index < constructors.Length; index++) { @@ -467,23 +466,23 @@ private static object CreateInstance( if (!canUse) continue; - if (parameters is null || constructorParameters.Length > parameters?.Length) + if (chosenParameters is null || constructorParameters.Length > chosenParameters.Length) { - parameters = constructorParameters; - indexOfChosenConstructor = index; + chosenParameters = constructorParameters; + chosenConstructor = constructor; } } - if (parameters != null) + if (chosenParameters != null && chosenConstructor != null) { - object?[] parameterValues = new object?[parameters.Length]; + object?[] parameterValues = new object?[chosenParameters.Length]; - for (int index = 0; index < parameters.Length; index++) + for (int index = 0; index < chosenParameters.Length; index++) { - parameterValues[index] = GetParameterValue(parameters[index], config, options); + parameterValues[index] = GetParameterValue(chosenParameters[index], config, options); } - return constructors[indexOfChosenConstructor].Invoke(parameterValues); + return chosenConstructor.Invoke(parameterValues); } } From 13f6c37ff82b04b0efddadde853b62d520efff3d Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Tue, 5 Apr 2022 07:39:56 +0100 Subject: [PATCH 10/41] Refactor --- .../src/ConfigurationBinder.cs | 33 +++++++++---------- .../tests/ConfigurationBinderTests.cs | 12 ++++--- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 9875da6201bc7b..a9b4e1aaa22789 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -443,29 +443,13 @@ private static object CreateInstance( for (int index = 0; index < constructors.Length; index++) { ConstructorInfo constructor = constructors[index]; - ParameterInfo[] constructorParameters = constructor.GetParameters(); - bool canUse = true; - - foreach (ParameterInfo p in constructorParameters) + if (!CanBindToTheseConstructorParameters(constructorParameters)) { - if (p.IsOut) - { - canUse = false; - break; - } - - // `in` is OK, but filter out `ref` - if (!p.IsIn && p.ParameterType.IsByRef) - { - canUse = false; - break; - } + continue; } - if (!canUse) continue; - if (chosenParameters is null || constructorParameters.Length > chosenParameters.Length) { chosenParameters = constructorParameters; @@ -499,6 +483,19 @@ private static object CreateInstance( return instance ?? throw new InvalidOperationException(SR.Format(SR.Error_FailedToActivate, type)); } + private static bool CanBindToTheseConstructorParameters(ParameterInfo[] constructorParameters) + { + foreach (ParameterInfo p in constructorParameters) + { + if (p.IsOut) return false; + + // `in` is OK, but filter out `ref` + if (!p.IsIn && p.ParameterType.IsByRef) return false; + } + + return true; + } + [RequiresUnreferencedCode("Cannot statically analyze what the element type is of the value objects in the dictionary so its members may be trimmed.")] private static void BindDictionary( object dictionary, diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 0f1672b232b14a..09c43dbf684070 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -114,25 +114,29 @@ public class DerivedOptionsWithIConfigurationSection : DerivedOptions public class ClassWithInAndOutAndRefParameters { - public ClassWithInAndOutAndRefParameters(in string color, int length) + // will not choose because ref parameters are not considered + public ClassWithInAndOutAndRefParameters(in string color, ref int length) { Color = color; Length = length; } - public ClassWithInAndOutAndRefParameters(in string color, ref int length) + // will not choose this one because out parameters are not considered + public ClassWithInAndOutAndRefParameters(in string color, int length, out string colorUpperCase) { Color = color; Length = length; + colorUpperCase = color.ToUpper(); } - public ClassWithInAndOutAndRefParameters(in string color, int length, out string colorUpperCase) + // WILL choose this one; in parameters are considered + public ClassWithInAndOutAndRefParameters(in string color, int length) { Color = color; Length = length; - colorUpperCase = color.ToUpper(); } + // will not choose this one as it contains ref and out parameters public ClassWithInAndOutAndRefParameters(in string color, int length, out string colorUpperCase, ref int alternativeLength) { Color = color; From 4d04a4bed011628b69b11b9579fb171d7daeae59 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Tue, 5 Apr 2022 07:42:05 +0100 Subject: [PATCH 11/41] PR feedback: test for readonly struct --- .../tests/ConfigurationBinderTests.cs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 09c43dbf684070..7212c708eee879 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -165,6 +165,18 @@ public StructTypeOptions(string color, int length) public int Length { get; } } + public struct ReadonlyStructTypeOptions + { + public ReadonlyStructTypeOptions(string color, int length) + { + Color = color; + Length = length; + } + + public string Color { get; } + public int Length { get; } + } + public class ContainerWithNestedImmutableObject { public string ContainerName { get; set; } @@ -1245,6 +1257,23 @@ public void CanBindStructOptions() Assert.Equal("Green", options.Color); } + [Fact] + public void CanBindReadonlyStructOptions() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("Green", options.Color); + } + [Fact] public void CanBindRecordStructOptions() { From a9e5eb7f7ccffedb00fee97260a6225bd348232e Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Tue, 5 Apr 2022 07:44:13 +0100 Subject: [PATCH 12/41] PR feedback: tests for sem-immutable class with init property --- .../tests/ConfigurationBinderTests.cs | 38 +++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 7212c708eee879..828e4238e1517e 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -229,9 +229,22 @@ public ImmutableClassWithConstructorOverloads(string string1) public int Int2 { get; } } - public class SemiImmutableType + public class SemiImmutableClass { - public SemiImmutableType(string color, int length) + public SemiImmutableClass(string color, int length) + { + Color = color; + Length = length; + } + + public string Color { get; } + public int Length { get; } + public decimal Thickness { get; set; } + } + + public class SemiImmutableClassWithInit + { + public SemiImmutableClassWithInit(string color, int length) { Color = color; Length = length; @@ -1217,7 +1230,26 @@ public void CanBindSemiImmutableClass() configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - var options = config.Get(); + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("Green", options.Color); + Assert.Equal(1.23m, options.Thickness); + } + + [Fact] + public void CanBindSemiImmutableClassWithInit() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + {"Thickness", "1.23"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); Assert.Equal(42, options.Length); Assert.Equal("Green", options.Color); Assert.Equal(1.23m, options.Thickness); From 98be745494a0b95fa81b1f7c7781bf74c590b634 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Tue, 5 Apr 2022 07:49:17 +0100 Subject: [PATCH 13/41] PR feedback: added tests for `record struct` and `readonly record struct` --- .../tests/ConfigurationBinderTests.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 828e4238e1517e..a62a8562bb1858 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -152,6 +152,7 @@ public ClassWithInAndOutAndRefParameters(in string color, int length, out string public record RecordTypeOptions(string Color, int Length); public record struct RecordStructTypeOptions(string Color, int Length); + public readonly record struct ReadonlyRecordStructTypeOptions(string Color, int Length); public struct StructTypeOptions { @@ -1323,6 +1324,23 @@ public void CanBindRecordStructOptions() Assert.Equal("Green", options.Color); } + [Fact] + public void CanBindReadonlyRecordStructOptions() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("Green", options.Color); + } + [Fact] public void CanBindByteArray() { From d690b952cc48769e25b6d670c2ea9d9fa39d19d9 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Tue, 5 Apr 2022 07:56:41 +0100 Subject: [PATCH 14/41] PR feedback --- .../tests/ConfigurationBinderTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index a62a8562bb1858..272eafca89233f 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -166,7 +166,7 @@ public StructTypeOptions(string color, int length) public int Length { get; } } - public struct ReadonlyStructTypeOptions + public readonly struct ReadonlyStructTypeOptions { public ReadonlyStructTypeOptions(string color, int length) { From eaa0ccbdf2bdfd042f96ea8e3a8a5c62002a3c86 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Fri, 8 Apr 2022 06:35:46 +0100 Subject: [PATCH 15/41] PR feedback --- .../src/ConfigurationBinder.cs | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index a9b4e1aaa22789..4dfcc4e25211c4 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -423,19 +423,13 @@ private static object CreateInstance( throw new InvalidOperationException(SR.Format(SR.Error_CannotActivateAbstractOrInterface, type)); } - ConstructorInfo[] constructors = type.GetConstructors(DeclaredOnlyLookup); + ConstructorInfo[] constructors = type.GetConstructors(BindingFlags.Public | BindingFlags.Instance); bool hasParameterlessPublicConstructor = - constructors.Any(ctor => ctor.IsPublic && ctor.GetParameters().Length == 0); + constructors.Any(ctor => ctor.GetParameters().Length == 0); if (constructors.Length > 0 && !hasParameterlessPublicConstructor) { - // if the only constructor is private, then throw - if (constructors.Length == 1 && !constructors[0].IsPublic) - { - throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); - } - // find the biggest constructor so that we can bind to the most parameters ParameterInfo[]? chosenParameters = null; ConstructorInfo? chosenConstructor = null; @@ -463,7 +457,7 @@ private static object CreateInstance( for (int index = 0; index < chosenParameters.Length; index++) { - parameterValues[index] = GetParameterValue(chosenParameters[index], config, options); + parameterValues[index] = BindParameter(chosenParameters[index], config, options); } return chosenConstructor.Invoke(parameterValues); @@ -745,7 +739,7 @@ private static List GetAllProperties( } [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] - private static object? GetParameterValue(ParameterInfo property, IConfiguration config, BinderOptions options) + private static object? BindParameter(ParameterInfo property, IConfiguration config, BinderOptions options) { string parameterName = property.Name!; @@ -773,15 +767,13 @@ private static string GetParameterName(ParameterInfo parameter) private static string GetPropertyName(MemberInfo property) { - var customAttributesData = property.GetCustomAttributesData(); - - return TryGetNameFromAttributes(customAttributesData) ?? property.Name; + return TryGetNameFromAttributes(property.GetCustomAttributesData()) ?? property.Name; } private static string? TryGetNameFromAttributes(IList customAttributesData) { // Check for a custom property name used for configuration key binding - foreach (var attributeData in customAttributesData) + foreach (CustomAttributeData attributeData in customAttributesData) { if (attributeData.AttributeType != typeof(ConfigurationKeyNameAttribute)) { From 2fb15ee5bf93230e2649d03321f019b3c286ccea Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Sat, 9 Apr 2022 07:24:46 +0100 Subject: [PATCH 16/41] Add tests for current behaviour discussed in the PR --- .../src/ConfigurationBinder.cs | 6 ++ .../tests/ConfigurationBinderTests.cs | 96 ++++++++++++++++++- 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 4dfcc4e25211c4..8102cf298eafcb 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -428,6 +428,12 @@ private static object CreateInstance( bool hasParameterlessPublicConstructor = constructors.Any(ctor => ctor.GetParameters().Length == 0); + // if it's not a value type, and there are no constructors, nor a default constructor + if (!type.IsValueType && constructors.Length == 0 && !hasParameterlessPublicConstructor) + { + throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); + } + if (constructors.Length > 0 && !hasParameterlessPublicConstructor) { // find the biggest constructor so that we can bind to the most parameters diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 272eafca89233f..f0205148baa753 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -230,6 +230,56 @@ public ImmutableClassWithConstructorOverloads(string string1) public int Int2 { get; } } + public class ImmutableClassWAmbiguousConstructorOverloads + { + public ImmutableClassWAmbiguousConstructorOverloads(string string1, int int1, string string2, int int2) + { + String1 = "created by 1st constructor"; + Int1 = int1; + String2 = string2; + Int2 = int2; + } + + public ImmutableClassWAmbiguousConstructorOverloads(int int1, string string1, int int2, string string2) + { + Int1 = int1; + String1 = "created by 2st constructor"; + Int2 = int2; + String2 = string2; + } + + public string String1 { get; } + public string String2 { get; } + public int Int1 { get; } + public int Int2 { get; } + } + + public class ImmutableClassWMoreConstructorParamsThatProperties + { + public ImmutableClassWMoreConstructorParamsThatProperties(string string1, int int1, string string2, int int2) + { + String1 = string1; + Int1 = int1; + String2 = string2; + Int2 = int2; + } + + public ImmutableClassWMoreConstructorParamsThatProperties(int int1, string string1, int int2, string string2, int int3, string string3) + { + Int1 = int1; + String1 = string1; + Int2 = int2; + String2 = string2; + + String1 = "set from the largest constructor"; + } + + public string String1 { get; } + public string String2 { get; } + public int Int1 { get; } + public int Int2 { get; } + } + public class SemiImmutableClass { public SemiImmutableClass(string color, int length) @@ -1194,7 +1244,7 @@ public void CanBindMutableClassWitNestedImmutableObject() // If the immutable type has multiple constructors, // then pick the one with the most parameters // and try to bind as many as possible. - // The type used in example below has multiple constructors in + // The type used in the example below has multiple constructors in // an arbitrary order, but/ with the biggest (chosen) one // deliberately not first or last. [Fact] @@ -1218,6 +1268,50 @@ public void CanBindImmutableClass_PicksBiggestNonParameterlessConstructor() Assert.Equal(2, options.Int2); } + [Fact] + public void CanBindImmutableClass_PicksFirstOfAnyAmbiguousConstructors() + { + var dic = new Dictionary + { + {"String1", "s1"}, + {"Int1", "1"}, + {"String2", "s2"}, + {"Int2", "2"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal("created by 1st constructor", options.String1); + Assert.Equal("s2", options.String2); + Assert.Equal(1, options.Int1); + Assert.Equal(2, options.Int2); + } + + [Fact] + public void CanBindImmutableClass_PicksLargestConstructorEvenIfItHasExtraneousParameters() + { + var dic = new Dictionary + { + {"String1", "s1"}, + {"Int1", "1"}, + {"String2", "s2"}, + {"Int2", "2"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + + // the biggest constructor sets this field to something else + Assert.Equal("set from the largest constructor", options.String1); + Assert.Equal("s2", options.String2); + Assert.Equal(1, options.Int1); + Assert.Equal(2, options.Int2); + } + [Fact] public void CanBindSemiImmutableClass() { From b948152ff13c13635471948dc33a0c38a5971dc3 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Thu, 21 Apr 2022 22:31:36 +0100 Subject: [PATCH 17/41] PR feedback. Now behaves more like System.Text.Json --- .../src/ConfigurationBinder.cs | 55 ++--- .../src/Resources/Strings.resx | 6 + .../tests/ConfigurationBinderTests.cs | 207 +++++++++--------- 3 files changed, 135 insertions(+), 133 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 8102cf298eafcb..5b1788e9198958 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -434,40 +434,29 @@ private static object CreateInstance( throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); } - if (constructors.Length > 0 && !hasParameterlessPublicConstructor) + if (constructors.Length > 1 && !hasParameterlessPublicConstructor) { - // find the biggest constructor so that we can bind to the most parameters - ParameterInfo[]? chosenParameters = null; - ConstructorInfo? chosenConstructor = null; - - for (int index = 0; index < constructors.Length; index++) - { - ConstructorInfo constructor = constructors[index]; - ParameterInfo[] constructorParameters = constructor.GetParameters(); - - if (!CanBindToTheseConstructorParameters(constructorParameters)) - { - continue; - } + throw new InvalidOperationException(SR.Format(SR.Error_MultipleParameterisedConstructors, type)); + } - if (chosenParameters is null || constructorParameters.Length > chosenParameters.Length) - { - chosenParameters = constructorParameters; - chosenConstructor = constructor; - } - } + if (constructors.Length == 1 && !hasParameterlessPublicConstructor) + { + ConstructorInfo constructor = constructors[0]; + ParameterInfo[] parameters = constructor.GetParameters(); - if (chosenParameters != null && chosenConstructor != null) + if (!CanBindToTheseConstructorParameters(parameters, out string invalidPropertyName)) { - object?[] parameterValues = new object?[chosenParameters.Length]; + throw new InvalidOperationException(SR.Format(SR.Error_CannotBindToConstructorParameter, type, invalidPropertyName)); + } - for (int index = 0; index < chosenParameters.Length; index++) - { - parameterValues[index] = BindParameter(chosenParameters[index], config, options); - } + object?[] parameterValues = new object?[parameters.Length]; - return chosenConstructor.Invoke(parameterValues); + for (int index = 0; index < parameters.Length; index++) + { + parameterValues[index] = BindParameter(parameters[index], config, options); } + + return constructor.Invoke(parameterValues); } object? instance; @@ -483,14 +472,16 @@ private static object CreateInstance( return instance ?? throw new InvalidOperationException(SR.Format(SR.Error_FailedToActivate, type)); } - private static bool CanBindToTheseConstructorParameters(ParameterInfo[] constructorParameters) + private static bool CanBindToTheseConstructorParameters(ParameterInfo[] constructorParameters, out string reason) { + reason = string.Empty; foreach (ParameterInfo p in constructorParameters) { - if (p.IsOut) return false; - - // `in` is OK, but filter out `ref` - if (!p.IsIn && p.ParameterType.IsByRef) return false; + if (p.IsOut || p.IsIn || p.ParameterType.IsByRef) + { + reason = p.ParameterType.ToString(); + return false; + } } return true; diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx index dca7cc1bf353b4..ea89e0d4086e1c 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx @@ -120,6 +120,9 @@ Cannot create instance of type '{0}' because it is either abstract or an interface. + + Cannot create instance of type '{0}' because parameter '{1}' cannot be bound to. + Failed to convert configuration value at '{0}' to type '{1}'. @@ -132,6 +135,9 @@ Cannot create instance of type '{0}' because it is missing a public parameterless constructor. + + Cannot create instance of type '{0}' because it has multiple public parameterised constructors. + Cannot create instance of type '{0}' because multidimensional arrays are not supported. diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index f0205148baa753..986e0c3c9595ae 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -112,43 +112,6 @@ public class DerivedOptionsWithIConfigurationSection : DerivedOptions public IConfigurationSection DerivedSection { get; set; } } - public class ClassWithInAndOutAndRefParameters - { - // will not choose because ref parameters are not considered - public ClassWithInAndOutAndRefParameters(in string color, ref int length) - { - Color = color; - Length = length; - } - - // will not choose this one because out parameters are not considered - public ClassWithInAndOutAndRefParameters(in string color, int length, out string colorUpperCase) - { - Color = color; - Length = length; - colorUpperCase = color.ToUpper(); - } - - // WILL choose this one; in parameters are considered - public ClassWithInAndOutAndRefParameters(in string color, int length) - { - Color = color; - Length = length; - } - - // will not choose this one as it contains ref and out parameters - public ClassWithInAndOutAndRefParameters(in string color, int length, out string colorUpperCase, ref int alternativeLength) - { - Color = color; - Length = length < 100 ? length : alternativeLength; - colorUpperCase = color.ToUpper(); - } - - - public string Color { get; } - public int Length { get; } - } - public record RecordTypeOptions(string Color, int Length); public record struct RecordStructTypeOptions(string Color, int Length); @@ -196,22 +159,25 @@ public ImmutableLengthAndColorClass(string color, int length) public int Length { get; } } - public class ImmutableClassWithConstructorOverloads + public class ImmutableClassWithOneParameterisedConstructor { - public ImmutableClassWithConstructorOverloads(string string1, int int1) - { - String1 = string1; - Int1 = int1; - } - - public ImmutableClassWithConstructorOverloads(string string1, int int1, string string2) + public ImmutableClassWithOneParameterisedConstructor(string string1, int int1, string string2, int int2) { String1 = string1; Int1 = int1; String2 = string2; + Int2 = int2; } - public ImmutableClassWithConstructorOverloads(string string1, int int1, string string2, int int2) + public string String1 { get; } + public string String2 { get; } + public int Int1 { get; } + public int Int2 { get; } + } + + public class ImmutableClassWithOneParameterisedConstructorButWithInParameter + { + public ImmutableClassWithOneParameterisedConstructorButWithInParameter(in string string1, int int1, string string2, int int2) { String1 = string1; Int1 = int1; @@ -219,33 +185,37 @@ public ImmutableClassWithConstructorOverloads(string string1, int int1, string s Int2 = int2; } - public ImmutableClassWithConstructorOverloads(string string1) - { - String1 = string1; - } - public string String1 { get; } public string String2 { get; } public int Int1 { get; } public int Int2 { get; } } - public class ImmutableClassWAmbiguousConstructorOverloads + public class ImmutableClassWithOneParameterisedConstructorButWithRefParameter { - public ImmutableClassWAmbiguousConstructorOverloads(string string1, int int1, string string2, int int2) + public ImmutableClassWithOneParameterisedConstructorButWithRefParameter(string string1, ref int int1, string string2, int int2) { - String1 = "created by 1st constructor"; + String1 = string1; Int1 = int1; String2 = string2; Int2 = int2; } - public ImmutableClassWAmbiguousConstructorOverloads(int int1, string string1, int int2, string string2) + public string String1 { get; } + public string String2 { get; } + public int Int1 { get; } + public int Int2 { get; } + } + + public class ImmutableClassWithOneParameterisedConstructorButWithOutParameter + { + public ImmutableClassWithOneParameterisedConstructorButWithOutParameter(string string1, int int1, + string string2, out decimal int2) { + String1 = string1; Int1 = int1; - String1 = "created by 2st constructor"; - Int2 = int2; String2 = string2; + int2 = 0; } public string String1 { get; } @@ -254,24 +224,32 @@ public ImmutableClassWAmbiguousConstructorOverloads(int int1, string string1, in public int Int2 { get; } } - public class ImmutableClassWMoreConstructorParamsThatProperties + public class ImmutableClassWithMultipleParameterisedConstructors { - public ImmutableClassWMoreConstructorParamsThatProperties(string string1, int int1, string string2, int int2) + public ImmutableClassWithMultipleParameterisedConstructors(string string1, int int1) { String1 = string1; Int1 = int1; - String2 = string2; - Int2 = int2; } - public ImmutableClassWMoreConstructorParamsThatProperties(int int1, string string1, int int2, string string2, int int3, string string3) + public ImmutableClassWithMultipleParameterisedConstructors(string string1, int int1, string string2) { + String1 = string1; Int1 = int1; + String2 = string2; + } + + public ImmutableClassWithMultipleParameterisedConstructors(string string1, int int1, string string2, int int2) + { String1 = string1; - Int2 = int2; + Int1 = int1; String2 = string2; + Int2 = int2; + } - String1 = "set from the largest constructor"; + public ImmutableClassWithMultipleParameterisedConstructors(string string1) + { + String1 = string1; } public string String1 { get; } @@ -1206,49 +1184,51 @@ public void CanBindImmutableClass() } [Fact] - public void CanBindImmutableClassAndSkipsConstructorsWithOutAndRefParameters() + public void CanBindMutableClassWitNestedImmutableObject() { var dic = new Dictionary { - {"Length", "42"}, - {"Color", "Green"}, + {"ContainerName", "Container123"}, + {"LengthAndColor:Length", "42"}, + {"LengthAndColor:Color", "Green"}, }; var configurationBuilder = new ConfigurationBuilder(); configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - var options = config.Get(); - Assert.Equal(42, options.Length); - Assert.Equal("Green", options.Color); + var options = config.Get(); + Assert.Equal("Container123", options.ContainerName); + Assert.Equal(42, options.LengthAndColor.Length); + Assert.Equal("Green", options.LengthAndColor.Color); } + // If the immutable type has multiple public parameterised constructors, then throw + // an exception. [Fact] - public void CanBindMutableClassWitNestedImmutableObject() + public void CanBindImmutableClass_ThrowsOnMultipleParameterisedConstructors() { var dic = new Dictionary { - {"ContainerName", "Container123"}, - {"LengthAndColor:Length", "42"}, - {"LengthAndColor:Color", "Green"}, + {"String1", "s1"}, + {"Int1", "1"}, + {"String2", "s2"}, + {"Int2", "2"}, }; var configurationBuilder = new ConfigurationBuilder(); configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - var options = config.Get(); - Assert.Equal("Container123", options.ContainerName); - Assert.Equal(42, options.LengthAndColor.Length); - Assert.Equal("Green", options.LengthAndColor.Color); + string expectedMessage = SR.Format(SR.Error_MultipleParameterisedConstructors, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithMultipleParameterisedConstructors"); + + var ex = Assert.Throws(() => config.Get()); + + Assert.Equal(expectedMessage, ex.Message); } - // If the immutable type has multiple constructors, - // then pick the one with the most parameters - // and try to bind as many as possible. - // The type used in the example below has multiple constructors in - // an arbitrary order, but/ with the biggest (chosen) one - // deliberately not first or last. + // If the immutable type has a parameterised constructor, then throw + // that constructor has an 'in' parameter [Fact] - public void CanBindImmutableClass_PicksBiggestNonParameterlessConstructor() + public void CanBindImmutableClass_ThrowsOnParameterisedConstructorWithAnInParameter() { var dic = new Dictionary { @@ -1261,15 +1241,17 @@ public void CanBindImmutableClass_PicksBiggestNonParameterlessConstructor() configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - var options = config.Get(); - Assert.Equal("s1", options.String1); - Assert.Equal("s2", options.String2); - Assert.Equal(1, options.Int1); - Assert.Equal(2, options.Int2); + string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterisedConstructorButWithInParameter", "System.String&"); + + var ex = Assert.Throws(() => config.Get()); + + Assert.Equal(expectedMessage, ex.Message); } + // If the immutable type has a parameterised constructors, then throw + // that constructor has a 'ref' parameter [Fact] - public void CanBindImmutableClass_PicksFirstOfAnyAmbiguousConstructors() + public void CanBindImmutableClass_ThrowsOnParameterisedConstructorWithARefParameter() { var dic = new Dictionary { @@ -1282,15 +1264,17 @@ public void CanBindImmutableClass_PicksFirstOfAnyAmbiguousConstructors() configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - var options = config.Get(); - Assert.Equal("created by 1st constructor", options.String1); - Assert.Equal("s2", options.String2); - Assert.Equal(1, options.Int1); - Assert.Equal(2, options.Int2); + string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterisedConstructorButWithRefParameter", "System.Int32&"); + + var ex = Assert.Throws(() => config.Get()); + + Assert.Equal(expectedMessage, ex.Message); } + // If the immutable type has a parameterised constructors, then throw + // that constructor has a 'ref' parameter [Fact] - public void CanBindImmutableClass_PicksLargestConstructorEvenIfItHasExtraneousParameters() + public void CanBindImmutableClass_ThrowsOnParameterisedConstructorWithAnOutParameter() { var dic = new Dictionary { @@ -1303,10 +1287,31 @@ public void CanBindImmutableClass_PicksLargestConstructorEvenIfItHasExtraneousPa configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - var options = config.Get(); + string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterisedConstructorButWithOutParameter", "System.Decimal&"); + + var ex = Assert.Throws(() => config.Get()); + + Assert.Equal(expectedMessage, ex.Message); + } + + // If the immutable type has a public parameterised constructor, + // then pick it. + [Fact] + public void CanBindImmutableClass_PicksParameterisedConstructorIfNoParameterlessConstructorExists() + { + var dic = new Dictionary + { + {"String1", "s1"}, + {"Int1", "1"}, + {"String2", "s2"}, + {"Int2", "2"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); - // the biggest constructor sets this field to something else - Assert.Equal("set from the largest constructor", options.String1); + var options = config.Get(); + Assert.Equal("s1", options.String1); Assert.Equal("s2", options.String2); Assert.Equal(1, options.Int1); Assert.Equal(2, options.Int2); @@ -1332,7 +1337,7 @@ public void CanBindSemiImmutableClass() } [Fact] - public void CanBindSemiImmutableClassWithInit() + public void CanBindSemiImmutableClass_WithInitProperties() { var dic = new Dictionary { From 4dcc89fe714681eecc59c8799d39045e45b2289f Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Thu, 28 Apr 2022 21:42:22 +0100 Subject: [PATCH 18/41] Slightly simplify the constructor picker logic --- .../src/ConfigurationBinder.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 5b1788e9198958..f538ead0ef10d4 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -425,21 +425,20 @@ private static object CreateInstance( ConstructorInfo[] constructors = type.GetConstructors(BindingFlags.Public | BindingFlags.Instance); - bool hasParameterlessPublicConstructor = + bool hasParameterlessConstructor = constructors.Any(ctor => ctor.GetParameters().Length == 0); - // if it's not a value type, and there are no constructors, nor a default constructor - if (!type.IsValueType && constructors.Length == 0 && !hasParameterlessPublicConstructor) + if (!type.IsValueType && constructors.Length == 0) { throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); } - if (constructors.Length > 1 && !hasParameterlessPublicConstructor) + if (constructors.Length > 1 && !hasParameterlessConstructor) { throw new InvalidOperationException(SR.Format(SR.Error_MultipleParameterisedConstructors, type)); } - if (constructors.Length == 1 && !hasParameterlessPublicConstructor) + if (constructors.Length == 1 && !hasParameterlessConstructor) { ConstructorInfo constructor = constructors[0]; ParameterInfo[] parameters = constructor.GetParameters(); From 3f10fccdd4acc5c152913923d57d5d73abad9a6a Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Thu, 5 May 2022 20:39:16 +0100 Subject: [PATCH 19/41] Update src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs Co-authored-by: Stephen Halter --- .../src/ConfigurationBinder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index f538ead0ef10d4..ba8c235e8e4b8a 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -735,7 +735,7 @@ private static List GetAllProperties( } [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] - private static object? BindParameter(ParameterInfo property, IConfiguration config, BinderOptions options) + private static object? BindParameter(ParameterInfo parameter, IConfiguration config, BinderOptions options) { string parameterName = property.Name!; From 7b95565d7f8d22857d04e75bcb9beb1c553efd86 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Sat, 7 May 2022 05:14:35 +0100 Subject: [PATCH 20/41] Fix build --- .../src/ConfigurationBinder.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index ba8c235e8e4b8a..1deeb1c73fe014 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -737,12 +737,12 @@ private static List GetAllProperties( [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] private static object? BindParameter(ParameterInfo parameter, IConfiguration config, BinderOptions options) { - string parameterName = property.Name!; + string parameterName = parameter.Name!; var propertyBindingPoint = new BindingPoint(initialValue: config.GetSection(parameterName).Value, isReadOnly: false); BindInstance( - property.ParameterType, + parameter.ParameterType, propertyBindingPoint, config.GetSection(parameterName), options); From 88985ca4bc1f1e2b3961edaa6bf9bf81e58557ed Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Sat, 21 May 2022 06:40:59 +0100 Subject: [PATCH 21/41] Update src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx Co-authored-by: Eric Erhardt --- .../src/Resources/Strings.resx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx index ea89e0d4086e1c..35673f168d583a 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx @@ -136,7 +136,7 @@ Cannot create instance of type '{0}' because it is missing a public parameterless constructor. - Cannot create instance of type '{0}' because it has multiple public parameterised constructors. + Cannot create instance of type '{0}' because it has multiple public parameterized constructors. Cannot create instance of type '{0}' because multidimensional arrays are not supported. From 41a71196078e8a1cc84ff946731acb14ea41f516 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Sat, 21 May 2022 06:47:15 +0100 Subject: [PATCH 22/41] Corrected 'parameterized' (was 'parameterised' - sorry, muscle memory!) --- .../src/ConfigurationBinder.cs | 2 +- .../src/Resources/Strings.resx | 2 +- .../tests/ConfigurationBinderTests.cs | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 1deeb1c73fe014..2dc2e3278d0062 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -435,7 +435,7 @@ private static object CreateInstance( if (constructors.Length > 1 && !hasParameterlessConstructor) { - throw new InvalidOperationException(SR.Format(SR.Error_MultipleParameterisedConstructors, type)); + throw new InvalidOperationException(SR.Format(SR.Error_MultipleParameterizedConstructors, type)); } if (constructors.Length == 1 && !hasParameterlessConstructor) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx index 35673f168d583a..0904f0b9153c56 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx @@ -135,7 +135,7 @@ Cannot create instance of type '{0}' because it is missing a public parameterless constructor. - + Cannot create instance of type '{0}' because it has multiple public parameterized constructors. diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 986e0c3c9595ae..39ca90a6c613eb 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -159,9 +159,9 @@ public ImmutableLengthAndColorClass(string color, int length) public int Length { get; } } - public class ImmutableClassWithOneParameterisedConstructor + public class ImmutableClassWithOneParameterizedConstructor { - public ImmutableClassWithOneParameterisedConstructor(string string1, int int1, string string2, int int2) + public ImmutableClassWithOneParameterizedConstructor(string string1, int int1, string string2, int int2) { String1 = string1; Int1 = int1; @@ -1218,7 +1218,7 @@ public void CanBindImmutableClass_ThrowsOnMultipleParameterisedConstructors() configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - string expectedMessage = SR.Format(SR.Error_MultipleParameterisedConstructors, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithMultipleParameterisedConstructors"); + string expectedMessage = SR.Format(SR.Error_MultipleParameterizedConstructors, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithMultipleParameterisedConstructors"); var ex = Assert.Throws(() => config.Get()); @@ -1310,7 +1310,7 @@ public void CanBindImmutableClass_PicksParameterisedConstructorIfNoParameterless configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - var options = config.Get(); + var options = config.Get(); Assert.Equal("s1", options.String1); Assert.Equal("s2", options.String2); Assert.Equal(1, options.Int1); From 6d54f2789e2cd653c10abd7302302200f5295f5a Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Sat, 21 May 2022 06:49:27 +0100 Subject: [PATCH 23/41] Corrected test type --- .../tests/ConfigurationBinderTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 39ca90a6c613eb..6d1108ac13301e 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -281,7 +281,7 @@ public SemiImmutableClassWithInit(string color, int length) public string Color { get; } public int Length { get; } - public decimal Thickness { get; set; } + public decimal Thickness { get; init; } } public struct ValueTypeOptions From fa8c4b5ef92c281b7e938f396cd1d0f90dd5988a Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Sat, 21 May 2022 06:51:21 +0100 Subject: [PATCH 24/41] Changed comment to be more accurate --- .../tests/ConfigurationBinderTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 6d1108ac13301e..68aeca3fcb690b 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -1272,7 +1272,7 @@ public void CanBindImmutableClass_ThrowsOnParameterisedConstructorWithARefParame } // If the immutable type has a parameterised constructors, then throw - // that constructor has a 'ref' parameter + // if the constructor has an 'out' parameter [Fact] public void CanBindImmutableClass_ThrowsOnParameterisedConstructorWithAnOutParameter() { From 11c780855217ed828e169b3a9cab02d31862e1fc Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Sat, 21 May 2022 06:54:20 +0100 Subject: [PATCH 25/41] Changed more instances of 'parameterised' to 'parameterized' --- .../tests/ConfigurationBinderTests.cs | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 68aeca3fcb690b..7c54cab60df13e 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -175,9 +175,9 @@ public ImmutableClassWithOneParameterizedConstructor(string string1, int int1, s public int Int2 { get; } } - public class ImmutableClassWithOneParameterisedConstructorButWithInParameter + public class ImmutableClassWithOneParameterizedConstructorButWithInParameter { - public ImmutableClassWithOneParameterisedConstructorButWithInParameter(in string string1, int int1, string string2, int int2) + public ImmutableClassWithOneParameterizedConstructorButWithInParameter(in string string1, int int1, string string2, int int2) { String1 = string1; Int1 = int1; @@ -191,9 +191,9 @@ public ImmutableClassWithOneParameterisedConstructorButWithInParameter(in string public int Int2 { get; } } - public class ImmutableClassWithOneParameterisedConstructorButWithRefParameter + public class ImmutableClassWithOneParameterizedConstructorButWithRefParameter { - public ImmutableClassWithOneParameterisedConstructorButWithRefParameter(string string1, ref int int1, string string2, int int2) + public ImmutableClassWithOneParameterizedConstructorButWithRefParameter(string string1, ref int int1, string string2, int int2) { String1 = string1; Int1 = int1; @@ -207,9 +207,9 @@ public ImmutableClassWithOneParameterisedConstructorButWithRefParameter(string s public int Int2 { get; } } - public class ImmutableClassWithOneParameterisedConstructorButWithOutParameter + public class ImmutableClassWithOneParameterizedConstructorButWithOutParameter { - public ImmutableClassWithOneParameterisedConstructorButWithOutParameter(string string1, int int1, + public ImmutableClassWithOneParameterizedConstructorButWithOutParameter(string string1, int int1, string string2, out decimal int2) { String1 = string1; @@ -224,22 +224,22 @@ public ImmutableClassWithOneParameterisedConstructorButWithOutParameter(string s public int Int2 { get; } } - public class ImmutableClassWithMultipleParameterisedConstructors + public class ImmutableClassWithMultipleParameterizedConstructors { - public ImmutableClassWithMultipleParameterisedConstructors(string string1, int int1) + public ImmutableClassWithMultipleParameterizedConstructors(string string1, int int1) { String1 = string1; Int1 = int1; } - public ImmutableClassWithMultipleParameterisedConstructors(string string1, int int1, string string2) + public ImmutableClassWithMultipleParameterizedConstructors(string string1, int int1, string string2) { String1 = string1; Int1 = int1; String2 = string2; } - public ImmutableClassWithMultipleParameterisedConstructors(string string1, int int1, string string2, int int2) + public ImmutableClassWithMultipleParameterizedConstructors(string string1, int int1, string string2, int int2) { String1 = string1; Int1 = int1; @@ -247,7 +247,7 @@ public ImmutableClassWithMultipleParameterisedConstructors(string string1, int i Int2 = int2; } - public ImmutableClassWithMultipleParameterisedConstructors(string string1) + public ImmutableClassWithMultipleParameterizedConstructors(string string1) { String1 = string1; } @@ -1202,10 +1202,10 @@ public void CanBindMutableClassWitNestedImmutableObject() Assert.Equal("Green", options.LengthAndColor.Color); } - // If the immutable type has multiple public parameterised constructors, then throw + // If the immutable type has multiple public parameterized constructors, then throw // an exception. [Fact] - public void CanBindImmutableClass_ThrowsOnMultipleParameterisedConstructors() + public void CanBindImmutableClass_ThrowsOnMultipleParameterizedConstructors() { var dic = new Dictionary { @@ -1218,17 +1218,17 @@ public void CanBindImmutableClass_ThrowsOnMultipleParameterisedConstructors() configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - string expectedMessage = SR.Format(SR.Error_MultipleParameterizedConstructors, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithMultipleParameterisedConstructors"); + string expectedMessage = SR.Format(SR.Error_MultipleParameterizedConstructors, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithMultipleParameterizedConstructors"); - var ex = Assert.Throws(() => config.Get()); + var ex = Assert.Throws(() => config.Get()); Assert.Equal(expectedMessage, ex.Message); } - // If the immutable type has a parameterised constructor, then throw + // If the immutable type has a parameterized constructor, then throw // that constructor has an 'in' parameter [Fact] - public void CanBindImmutableClass_ThrowsOnParameterisedConstructorWithAnInParameter() + public void CanBindImmutableClass_ThrowsOnParameterizedConstructorWithAnInParameter() { var dic = new Dictionary { @@ -1241,17 +1241,17 @@ public void CanBindImmutableClass_ThrowsOnParameterisedConstructorWithAnInParame configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterisedConstructorButWithInParameter", "System.String&"); + string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterizedConstructorButWithInParameter", "System.String&"); - var ex = Assert.Throws(() => config.Get()); + var ex = Assert.Throws(() => config.Get()); Assert.Equal(expectedMessage, ex.Message); } - // If the immutable type has a parameterised constructors, then throw + // If the immutable type has a parameterized constructors, then throw // that constructor has a 'ref' parameter [Fact] - public void CanBindImmutableClass_ThrowsOnParameterisedConstructorWithARefParameter() + public void CanBindImmutableClass_ThrowsOnParameterizedConstructorWithARefParameter() { var dic = new Dictionary { @@ -1264,17 +1264,17 @@ public void CanBindImmutableClass_ThrowsOnParameterisedConstructorWithARefParame configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterisedConstructorButWithRefParameter", "System.Int32&"); + string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterizedConstructorButWithRefParameter", "System.Int32&"); - var ex = Assert.Throws(() => config.Get()); + var ex = Assert.Throws(() => config.Get()); Assert.Equal(expectedMessage, ex.Message); } - // If the immutable type has a parameterised constructors, then throw + // If the immutable type has a parameterized constructors, then throw // if the constructor has an 'out' parameter [Fact] - public void CanBindImmutableClass_ThrowsOnParameterisedConstructorWithAnOutParameter() + public void CanBindImmutableClass_ThrowsOnParameterizedConstructorWithAnOutParameter() { var dic = new Dictionary { @@ -1287,17 +1287,17 @@ public void CanBindImmutableClass_ThrowsOnParameterisedConstructorWithAnOutParam configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterisedConstructorButWithOutParameter", "System.Decimal&"); + string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterizedConstructorButWithOutParameter", "System.Decimal&"); - var ex = Assert.Throws(() => config.Get()); + var ex = Assert.Throws(() => config.Get()); Assert.Equal(expectedMessage, ex.Message); } - // If the immutable type has a public parameterised constructor, + // If the immutable type has a public parameterized constructor, // then pick it. [Fact] - public void CanBindImmutableClass_PicksParameterisedConstructorIfNoParameterlessConstructorExists() + public void CanBindImmutableClass_PicksParameterizedConstructorIfNoParameterlessConstructorExists() { var dic = new Dictionary { From f2db596ede603d4dc7531a6938a216fa46abbb74 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Sat, 21 May 2022 07:15:29 +0100 Subject: [PATCH 26/41] Treat structs as always having a parameterless constructor: https://github.com/dotnet/runtime/pull/67258/files#r878637015 --- .../src/ConfigurationBinder.cs | 2 +- .../tests/ConfigurationBinderTests.cs | 81 ++++++------------- 2 files changed, 27 insertions(+), 56 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 2dc2e3278d0062..72b43c4282fc16 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -426,7 +426,7 @@ private static object CreateInstance( ConstructorInfo[] constructors = type.GetConstructors(BindingFlags.Public | BindingFlags.Instance); bool hasParameterlessConstructor = - constructors.Any(ctor => ctor.GetParameters().Length == 0); + constructors.Any(ctor => ctor.GetParameters().Length == 0 || type.IsValueType); if (!type.IsValueType && constructors.Length == 0) { diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 7c54cab60df13e..217b02e6e55be2 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -117,34 +117,22 @@ public record RecordTypeOptions(string Color, int Length); public record struct RecordStructTypeOptions(string Color, int Length); public readonly record struct ReadonlyRecordStructTypeOptions(string Color, int Length); - public struct StructTypeOptions + public class ContainerWithNestedImmutableObject { - public StructTypeOptions(string color, int length) - { - Color = color; - Length = length; - } - - public string Color { get; } - public int Length { get; } + public string ContainerName { get; set; } + public ImmutableLengthAndColorClass LengthAndColor { get; set; } } - public readonly struct ReadonlyStructTypeOptions + public struct MutableStructWithConstructor { - public ReadonlyStructTypeOptions(string color, int length) + public MutableStructWithConstructor(string randomParameter) { - Color = color; - Length = length; + Color = randomParameter; + Length = randomParameter.Length; } - public string Color { get; } - public int Length { get; } - } - - public class ContainerWithNestedImmutableObject - { - public string ContainerName { get; set; } - public ImmutableLengthAndColorClass LengthAndColor { get; set; } + public string Color { get; set; } + public int Length { get; set; } } public class ImmutableLengthAndColorClass @@ -1294,6 +1282,23 @@ public void CanBindImmutableClass_ThrowsOnParameterizedConstructorWithAnOutParam Assert.Equal(expectedMessage, ex.Message); } + [Fact] + public void CanBindMutableStruct_UnmatchedConstructorsAreIgnored() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("Green", options.Color); + } + // If the immutable type has a public parameterized constructor, // then pick it. [Fact] @@ -1372,40 +1377,6 @@ public void CanBindRecordOptions() Assert.Equal("Green", options.Color); } - [Fact] - public void CanBindStructOptions() - { - var dic = new Dictionary - { - {"Length", "42"}, - {"Color", "Green"}, - }; - var configurationBuilder = new ConfigurationBuilder(); - configurationBuilder.AddInMemoryCollection(dic); - var config = configurationBuilder.Build(); - - var options = config.Get(); - Assert.Equal(42, options.Length); - Assert.Equal("Green", options.Color); - } - - [Fact] - public void CanBindReadonlyStructOptions() - { - var dic = new Dictionary - { - {"Length", "42"}, - {"Color", "Green"}, - }; - var configurationBuilder = new ConfigurationBuilder(); - configurationBuilder.AddInMemoryCollection(dic); - var config = configurationBuilder.Build(); - - var options = config.Get(); - Assert.Equal(42, options.Length); - Assert.Equal("Green", options.Color); - } - [Fact] public void CanBindRecordStructOptions() { From 2d578913af4d6c8c4a02874a505c5f5959f18918 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Sat, 21 May 2022 07:44:23 +0100 Subject: [PATCH 27/41] Invalid parameters (ref/out/in) are reported by name rather than type --- .../src/ConfigurationBinder.cs | 10 +++++----- .../tests/ConfigurationBinderTests.cs | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 72b43c4282fc16..b2049fa2ca3fd0 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -443,9 +443,9 @@ private static object CreateInstance( ConstructorInfo constructor = constructors[0]; ParameterInfo[] parameters = constructor.GetParameters(); - if (!CanBindToTheseConstructorParameters(parameters, out string invalidPropertyName)) + if (!CanBindToTheseConstructorParameters(parameters, out string nameOfInvalidParameter)) { - throw new InvalidOperationException(SR.Format(SR.Error_CannotBindToConstructorParameter, type, invalidPropertyName)); + throw new InvalidOperationException(SR.Format(SR.Error_CannotBindToConstructorParameter, type, nameOfInvalidParameter)); } object?[] parameterValues = new object?[parameters.Length]; @@ -471,14 +471,14 @@ private static object CreateInstance( return instance ?? throw new InvalidOperationException(SR.Format(SR.Error_FailedToActivate, type)); } - private static bool CanBindToTheseConstructorParameters(ParameterInfo[] constructorParameters, out string reason) + private static bool CanBindToTheseConstructorParameters(ParameterInfo[] constructorParameters, out string nameOfInvalidParameter) { - reason = string.Empty; + nameOfInvalidParameter = string.Empty; foreach (ParameterInfo p in constructorParameters) { if (p.IsOut || p.IsIn || p.ParameterType.IsByRef) { - reason = p.ParameterType.ToString(); + nameOfInvalidParameter = p.Name!; // never null as we're not passed return value parameters: https://docs.microsoft.com/en-us/dotnet/api/system.reflection.parameterinfo.name?view=net-6.0#remarks return false; } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 217b02e6e55be2..8061361ad21e02 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -1229,7 +1229,7 @@ public void CanBindImmutableClass_ThrowsOnParameterizedConstructorWithAnInParame configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterizedConstructorButWithInParameter", "System.String&"); + string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterizedConstructorButWithInParameter", "string1"); var ex = Assert.Throws(() => config.Get()); @@ -1252,7 +1252,7 @@ public void CanBindImmutableClass_ThrowsOnParameterizedConstructorWithARefParame configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterizedConstructorButWithRefParameter", "System.Int32&"); + string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterizedConstructorButWithRefParameter", "int1"); var ex = Assert.Throws(() => config.Get()); @@ -1275,7 +1275,7 @@ public void CanBindImmutableClass_ThrowsOnParameterizedConstructorWithAnOutParam configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); - string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterizedConstructorButWithOutParameter", "System.Decimal&"); + string expectedMessage = SR.Format(SR.Error_CannotBindToConstructorParameter, "Microsoft.Extensions.Configuration.Binder.Test.ConfigurationBinderTests+ImmutableClassWithOneParameterizedConstructorButWithOutParameter", "int2"); var ex = Assert.Throws(() => config.Get()); From b41c60362c10ac738afe734a248e47eeaf500045 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Sat, 21 May 2022 07:59:33 +0100 Subject: [PATCH 28/41] Revert refactored method as it is no longer used in multiple places --- .../src/ConfigurationBinder.cs | 22 ++++--------------- 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index b2049fa2ca3fd0..6e6e35db04e2f6 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -750,26 +750,12 @@ private static List GetAllProperties( return propertyBindingPoint.Value; } - - // todo: steve - we might not need this; currently, these attributes are only applicable to properties and - // not parameters. We might be able to get rid of this method once confirm whether it's needed or not. - - private static string GetParameterName(ParameterInfo parameter) - { - var customAttributesData = parameter.GetCustomAttributesData(); - - return TryGetNameFromAttributes(customAttributesData) ?? parameter.Name!; - } - private static string GetPropertyName(MemberInfo property) { - return TryGetNameFromAttributes(property.GetCustomAttributesData()) ?? property.Name; - } + ThrowHelper.ThrowIfNull(property); - private static string? TryGetNameFromAttributes(IList customAttributesData) - { // Check for a custom property name used for configuration key binding - foreach (CustomAttributeData attributeData in customAttributesData) + foreach (var attributeData in property.GetCustomAttributesData()) { if (attributeData.AttributeType != typeof(ConfigurationKeyNameAttribute)) { @@ -788,10 +774,10 @@ private static string GetPropertyName(MemberInfo property) .Value? .ToString(); - return !string.IsNullOrWhiteSpace(name) ? name : null; + return !string.IsNullOrWhiteSpace(name) ? name : property.Name; } - return null; + return property.Name; } } } From 99b3329bfcdbe25d745a19464aa3df1cfa71bfcb Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Sat, 21 May 2022 08:31:23 +0100 Subject: [PATCH 29/41] Added tests for behaviour of setting parameters followed by properties --- .../tests/ConfigurationBinderTests.cs | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 8061361ad21e02..05690eed842a55 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -115,6 +115,27 @@ public class DerivedOptionsWithIConfigurationSection : DerivedOptions public record RecordTypeOptions(string Color, int Length); public record struct RecordStructTypeOptions(string Color, int Length); + + + public class ClassWithMatchingParametersAndProperties + { + private readonly string _color; + + public ClassWithMatchingParametersAndProperties(string Color, int Length) + { + _color = Color; + this.Length = Length; + } + + public int Length { get; set; } + + public string Color + { + get => _color; + init => _color = "the color is " + value; + } + } + public readonly record struct ReadonlyRecordStructTypeOptions(string Color, int Length); public class ContainerWithNestedImmutableObject @@ -1394,6 +1415,23 @@ public void CanBindRecordStructOptions() Assert.Equal("Green", options.Color); } + [Fact] + public void CanBindOnParametersAndProperties_PropertiesAreSetAfterTheConstructor() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("the color is Green", options.Color); + } + [Fact] public void CanBindReadonlyRecordStructOptions() { From 4e5f912fe3c93cd243dfee0e370eac762ebac2b5 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Tue, 24 May 2022 06:48:35 +0100 Subject: [PATCH 30/41] Reword and rename exception resource text --- .../src/ConfigurationBinder.cs | 2 +- .../src/Resources/Strings.resx | 4 ++-- .../tests/ConfigurationBinderTests.cs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 6e6e35db04e2f6..d273794f0f42f6 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -430,7 +430,7 @@ private static object CreateInstance( if (!type.IsValueType && constructors.Length == 0) { - throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); + throw new InvalidOperationException(SR.Format(SR.Error_MissingPublicInstanceConstructor, type)); } if (constructors.Length > 1 && !hasParameterlessConstructor) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx index 0904f0b9153c56..e574cd13a3f054 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx @@ -132,8 +132,8 @@ '{0}' was set on the provided {1}, but the following properties were not found on the instance of {2}: {3} - - Cannot create instance of type '{0}' because it is missing a public parameterless constructor. + + Cannot create instance of type '{0}' because it is missing a public instance constructor. Cannot create instance of type '{0}' because it has multiple public parameterized constructors. diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 05690eed842a55..749f68692ce12d 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -1115,7 +1115,7 @@ public void ExceptionWhenTryingToBindClassWithoutParameterlessConstructor() var exception = Assert.Throws( () => config.Bind(new TestOptions())); Assert.Equal( - SR.Format(SR.Error_MissingParameterlessConstructor, typeof(ClassWithoutPublicConstructor)), + SR.Format(SR.Error_MissingPublicInstanceConstructor, typeof(ClassWithoutPublicConstructor)), exception.Message); } From 2918063fb366007353f250fd9dbe8bdb21e1562a Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Thu, 26 May 2022 22:16:35 +0100 Subject: [PATCH 31/41] Disallow constructor parameters that do not match properties or do not have corresponding config items. --- .../src/ConfigurationBinder.cs | 37 ++++- .../src/Resources/Strings.resx | 8 +- .../tests/ConfigurationBinderTests.cs | 142 +++++++++++++++++- 3 files changed, 182 insertions(+), 5 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index d273794f0f42f6..6d8f8d50deb84f 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -448,11 +448,19 @@ private static object CreateInstance( throw new InvalidOperationException(SR.Format(SR.Error_CannotBindToConstructorParameter, type, nameOfInvalidParameter)); } + + var properties = GetAllProperties(type); + + if (!DoAllParametersHaveEquivalentProperties(parameters, properties, out string nameOfInvalidParameters)) + { + throw new InvalidOperationException(SR.Format(SR.Error_ConstructorParametersDoNotMatchProperties, type, nameOfInvalidParameters)); + } + object?[] parameterValues = new object?[parameters.Length]; for (int index = 0; index < parameters.Length; index++) { - parameterValues[index] = BindParameter(parameters[index], config, options); + parameterValues[index] = BindParameter(parameters[index], type, config, options); } return constructor.Invoke(parameterValues); @@ -471,6 +479,18 @@ private static object CreateInstance( return instance ?? throw new InvalidOperationException(SR.Format(SR.Error_FailedToActivate, type)); } + private static bool DoAllParametersHaveEquivalentProperties(ParameterInfo[] parameters, List properties, out string s) + { + var parameterNames = parameters.Select(param => param.Name).ToList(); + var propertyNames = properties.Select(prop => prop.Name).ToList(); + + var missingProperties = parameterNames.Where(pn => !propertyNames.Contains(pn!, StringComparer.OrdinalIgnoreCase)); + + s = string.Join(",", missingProperties); + + return s.Length == 0; + } + private static bool CanBindToTheseConstructorParameters(ParameterInfo[] constructorParameters, out string nameOfInvalidParameter) { nameOfInvalidParameter = string.Empty; @@ -735,12 +755,25 @@ private static List GetAllProperties( } [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] - private static object? BindParameter(ParameterInfo parameter, IConfiguration config, BinderOptions options) + private static object? BindParameter(ParameterInfo parameter, Type type, IConfiguration config, + BinderOptions options) { string parameterName = parameter.Name!; var propertyBindingPoint = new BindingPoint(initialValue: config.GetSection(parameterName).Value, isReadOnly: false); + if (propertyBindingPoint.Value is null) + { + if (parameter.HasDefaultValue) + { + propertyBindingPoint.SetValue(parameter.DefaultValue); + } + else + { + throw new InvalidOperationException(SR.Format(SR.Error_ParameterHasNoMatchingConfig, type, parameterName)); + } + } + BindInstance( parameter.ParameterType, propertyBindingPoint, diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx index e574cd13a3f054..906eb0a5c734aa 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx @@ -121,7 +121,10 @@ Cannot create instance of type '{0}' because it is either abstract or an interface. - Cannot create instance of type '{0}' because parameter '{1}' cannot be bound to. + Cannot create instance of type '{0}' because one or more parameters cannot be bound to. Constructor parameters cannot be declared as in, out, or ref. Invalid parameters are: '{1}' + + + Cannot create instance of type '{0}' because one or more parameters cannot be bound to. Constructor parameters must have corresponding properties. Missing properties are: '{1}' Failed to convert configuration value at '{0}' to type '{1}'. @@ -138,6 +141,9 @@ Cannot create instance of type '{0}' because it has multiple public parameterized constructors. + + Cannot create instance of type '{0}' because parameter '{1}' has no matching config. Each parameter in the constructor that does not have a default value must have a corresponding config entry. + Cannot create instance of type '{0}' because multidimensional arrays are not supported. diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 749f68692ce12d..aa13ce735f8747 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -112,10 +112,40 @@ public class DerivedOptionsWithIConfigurationSection : DerivedOptions public IConfigurationSection DerivedSection { get; set; } } - public record RecordTypeOptions(string Color, int Length); - public record struct RecordStructTypeOptions(string Color, int Length); + public class ClassWhereParametersDoNotMatchProperties + { + public string Name { get; } + public string Address { get; } + + public ClassWhereParametersDoNotMatchProperties(string name, string address, int age) + { + Name = name; + Address = address; + } + } + + public record RecordWhereParametersHaveDefaultValue(string Name, string Address, int Age = 42); + + public record ClassWhereParametersHaveDefaultValue + { + public string? Name { get; } + public string Address { get; } + public int Age { get; } + + public ClassWhereParametersHaveDefaultValue(string? name, string address, int age = 42) + { + Name = name; + Address = address; + Age = age; + } + } + + + public record RecordTypeOptions(string Color, int Length); + + public record Line(string Color, int Length, int Thickness); public class ClassWithMatchingParametersAndProperties { @@ -1119,6 +1149,110 @@ public void ExceptionWhenTryingToBindClassWithoutParameterlessConstructor() exception.Message); } + [Fact] + public void ExceptionWhenTryingToBindClassWherePropertiesDoMatchConstructorParameters() + { + var input = new Dictionary + { + {"ClassWhereParametersDoNotMatchPropertiesProperty:Name", "John"}, + {"ClassWhereParametersDoNotMatchPropertiesProperty:Address", "123, Abc St."}, + {"ClassWhereParametersDoNotMatchPropertiesProperty:Age", "42"} + }; + + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(input); + var config = configurationBuilder.Build(); + + var exception = Assert.Throws( + () => config.Bind(new TestOptions())); + Assert.Equal( + SR.Format(SR.Error_ConstructorParametersDoNotMatchProperties, typeof(ClassWhereParametersDoNotMatchProperties), "age"), + exception.Message); + } + + [Fact] + public void ExceptionWhenTryingToBindToConstructorWithMissingConfig() + { + var input = new Dictionary + { + {"LineProperty:Color", "Red"}, + {"LineProperty:Length", "22"} + }; + + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(input); + var config = configurationBuilder.Build(); + + var exception = Assert.Throws( + () => config.Bind(new TestOptions())); + Assert.Equal( + SR.Format(SR.Error_ParameterHasNoMatchingConfig, typeof(Line), nameof(Line.Thickness)), + exception.Message); + } + + [Fact] + public void ExceptionWhenTryingToBindConfigToClassWhereNoMatchingParameterIsFoundInConstructor() + { + var input = new Dictionary + { + {"ClassWhereParametersDoNotMatchPropertiesProperty:Name", "John"}, + {"ClassWhereParametersDoNotMatchPropertiesProperty:Address", "123, Abc St."}, + {"ClassWhereParametersDoNotMatchPropertiesProperty:Age", "42"} + }; + + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(input); + var config = configurationBuilder.Build(); + + var exception = Assert.Throws( + () => config.Bind(new TestOptions())); + Assert.Equal( + SR.Format(SR.Error_ConstructorParametersDoNotMatchProperties, typeof(ClassWhereParametersDoNotMatchProperties), "age"), + exception.Message); + } + + [Fact] + public void BindsToClassConstructorParametersWithDefaultValues() + { + var input = new Dictionary + { + {"ClassWhereParametersHaveDefaultValueProperty:Name", "John"}, + {"ClassWhereParametersHaveDefaultValueProperty:Address", "123, Abc St."} + }; + + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(input); + var config = configurationBuilder.Build(); + + TestOptions testOptions = new TestOptions(); + + config.Bind(testOptions); + Assert.Equal("John", testOptions.ClassWhereParametersHaveDefaultValueProperty.Name); + Assert.Equal("123, Abc St.", testOptions.ClassWhereParametersHaveDefaultValueProperty.Address); + Assert.Equal(42, testOptions.ClassWhereParametersHaveDefaultValueProperty.Age); + } + + [Fact] + public void BindsToRecordPrimaryConstructorParametersWithDefaultValues() + { + var input = new Dictionary + { + {"RecordWhereParametersHaveDefaultValueProperty:Name", "John"}, + {"RecordWhereParametersHaveDefaultValueProperty:Address", "123, Abc St."} + }; + + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(input); + var config = configurationBuilder.Build(); + + TestOptions testOptions = new TestOptions(); + + config.Bind(testOptions); + Assert.Equal("John", testOptions.RecordWhereParametersHaveDefaultValueProperty.Name); + Assert.Equal("123, Abc St.", testOptions.RecordWhereParametersHaveDefaultValueProperty.Address); + Assert.Equal(42, testOptions.RecordWhereParametersHaveDefaultValueProperty.Age); + } + [Fact] public void ExceptionWhenTryingToBindToTypeThrowsWhenActivated() { @@ -1608,6 +1742,10 @@ private class TestOptions public ISomeInterface ISomeInterfaceProperty { get; set; } public ClassWithoutPublicConstructor ClassWithoutPublicConstructorProperty { get; set; } + public ClassWhereParametersDoNotMatchProperties ClassWhereParametersDoNotMatchPropertiesProperty { get; set; } + public Line LineProperty { get; set; } + public ClassWhereParametersHaveDefaultValue ClassWhereParametersHaveDefaultValueProperty { get; set; } + public RecordWhereParametersHaveDefaultValue RecordWhereParametersHaveDefaultValueProperty { get; set; } public int IntProperty { get; set; } From d7754c2ddd0007f631414894389eb47d2f0ec49e Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Thu, 26 May 2022 22:22:04 +0100 Subject: [PATCH 32/41] PR feedback --- .../src/ConfigurationBinder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 6d8f8d50deb84f..ef5be6b9665d82 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -426,7 +426,7 @@ private static object CreateInstance( ConstructorInfo[] constructors = type.GetConstructors(BindingFlags.Public | BindingFlags.Instance); bool hasParameterlessConstructor = - constructors.Any(ctor => ctor.GetParameters().Length == 0 || type.IsValueType); + type.IsValueType || constructors.Any(ctor => ctor.GetParameters().Length == 0); if (!type.IsValueType && constructors.Length == 0) { From 59ed6d3ba507eae5bc3c721aafd4fce7b4731fc1 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Fri, 27 May 2022 05:42:04 +0100 Subject: [PATCH 33/41] Check for null parameter name --- .../src/ConfigurationBinder.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index ef5be6b9665d82..dd4425b9e52e21 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -758,7 +758,12 @@ private static List GetAllProperties( private static object? BindParameter(ParameterInfo parameter, Type type, IConfiguration config, BinderOptions options) { - string parameterName = parameter.Name!; + string? parameterName = parameter.Name; + + if (parameterName is null) + { + throw new InvalidOperationException(SR.Format(SR.Error_ParameterBeingBoundToHasNullName, type)); + } var propertyBindingPoint = new BindingPoint(initialValue: config.GetSection(parameterName).Value, isReadOnly: false); From ede16aaf403f1610465cd5ef7102549ccb38f2f6 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Fri, 27 May 2022 05:45:16 +0100 Subject: [PATCH 34/41] Check for null parameter name --- .../src/ConfigurationBinder.cs | 2 +- .../src/Resources/Strings.resx | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index dd4425b9e52e21..fc402d952fb44d 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -762,7 +762,7 @@ private static List GetAllProperties( if (parameterName is null) { - throw new InvalidOperationException(SR.Format(SR.Error_ParameterBeingBoundToHasNullName, type)); + throw new InvalidOperationException(SR.Format(SR.Error_ParameterBeingBoundToIsUnnamed, type)); } var propertyBindingPoint = new BindingPoint(initialValue: config.GetSection(parameterName).Value, isReadOnly: false); diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx index 906eb0a5c734aa..5cffa8e5eea6ab 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx @@ -141,6 +141,9 @@ Cannot create instance of type '{0}' because it has multiple public parameterized constructors. + + Cannot create instance of type '{0}' because one or more parameters are unnamed. + Cannot create instance of type '{0}' because parameter '{1}' has no matching config. Each parameter in the constructor that does not have a default value must have a corresponding config entry. From 0c558bc1bec68882d5522b4804ab562dfe9d8742 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Sat, 28 May 2022 07:44:37 +0100 Subject: [PATCH 35/41] Also check for fields when binding parameters --- .../src/ConfigurationBinder.cs | 31 +++++++++--- .../src/Resources/Strings.resx | 4 +- .../tests/ConfigurationBinderTests.cs | 48 ++++++++++++++++++- 3 files changed, 73 insertions(+), 10 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index fc402d952fb44d..dee240dfbf0669 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -449,11 +449,12 @@ private static object CreateInstance( } - var properties = GetAllProperties(type); + List properties = GetAllProperties(type); + List fields = GetAllFields(type); - if (!DoAllParametersHaveEquivalentProperties(parameters, properties, out string nameOfInvalidParameters)) + if (!DoAllParametersHaveEquivalentPropertiesOrFields(parameters, properties, fields, out string nameOfInvalidParameters)) { - throw new InvalidOperationException(SR.Format(SR.Error_ConstructorParametersDoNotMatchProperties, type, nameOfInvalidParameters)); + throw new InvalidOperationException(SR.Format(SR.Error_ConstructorParametersDoNotMatchPropertiesOrFields, type, nameOfInvalidParameters)); } object?[] parameterValues = new object?[parameters.Length]; @@ -479,12 +480,13 @@ private static object CreateInstance( return instance ?? throw new InvalidOperationException(SR.Format(SR.Error_FailedToActivate, type)); } - private static bool DoAllParametersHaveEquivalentProperties(ParameterInfo[] parameters, List properties, out string s) + private static bool DoAllParametersHaveEquivalentPropertiesOrFields(ParameterInfo[] parameters, + List properties, List fieldInfos, out string s) { var parameterNames = parameters.Select(param => param.Name).ToList(); - var propertyNames = properties.Select(prop => prop.Name).ToList(); + var propertyAndFieldNames = properties.Select(prop => prop.Name).Concat(fieldInfos.Select(fi => fi.Name)).ToList(); - var missingProperties = parameterNames.Where(pn => !propertyNames.Contains(pn!, StringComparer.OrdinalIgnoreCase)); + var missingProperties = parameterNames.Where(pn => !propertyAndFieldNames.Contains(pn!, StringComparer.OrdinalIgnoreCase)); s = string.Join(",", missingProperties); @@ -754,6 +756,23 @@ private static List GetAllProperties( return allProperties; } + private static List GetAllFields( + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] + Type type) + { + var allFields = new List(); + + Type? baseType = type; + do + { + allFields.AddRange(baseType!.GetFields(DeclaredOnlyLookup)); + baseType = baseType.BaseType; + } + while (baseType != typeof(object)); + + return allFields; + } + [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] private static object? BindParameter(ParameterInfo parameter, Type type, IConfiguration config, BinderOptions options) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx index 5cffa8e5eea6ab..dc18fb27e48026 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx @@ -123,8 +123,8 @@ Cannot create instance of type '{0}' because one or more parameters cannot be bound to. Constructor parameters cannot be declared as in, out, or ref. Invalid parameters are: '{1}' - - Cannot create instance of type '{0}' because one or more parameters cannot be bound to. Constructor parameters must have corresponding properties. Missing properties are: '{1}' + + Cannot create instance of type '{0}' because one or more parameters cannot be bound to. Constructor parameters must have corresponding properties or fields. Missing properties are: '{1}' Failed to convert configuration value at '{0}' to type '{1}'. diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index aa13ce735f8747..c35bd43314e3f2 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -114,6 +114,8 @@ public class DerivedOptionsWithIConfigurationSection : DerivedOptions public record struct RecordStructTypeOptions(string Color, int Length); + // Here, the constructor has three parameters, but not all of those match + // match to a property or field public class ClassWhereParametersDoNotMatchProperties { public string Name { get; } @@ -126,6 +128,25 @@ public ClassWhereParametersDoNotMatchProperties(string name, string address, int } } + // Here, the constructor has three parameters, and two of them match properties + // and one of them match a field. + public class ClassWhereParametersMatchPropertiesAndFields + { + private int Age; + + public string Name { get; } + public string Address { get; } + + public ClassWhereParametersMatchPropertiesAndFields(string name, string address, int age) + { + Name = name; + Address = address; + Age = age; + } + + public int GetAge() => Age; + } + public record RecordWhereParametersHaveDefaultValue(string Name, string Address, int Age = 42); public record ClassWhereParametersHaveDefaultValue @@ -1166,7 +1187,7 @@ public void ExceptionWhenTryingToBindClassWherePropertiesDoMatchConstructorParam var exception = Assert.Throws( () => config.Bind(new TestOptions())); Assert.Equal( - SR.Format(SR.Error_ConstructorParametersDoNotMatchProperties, typeof(ClassWhereParametersDoNotMatchProperties), "age"), + SR.Format(SR.Error_ConstructorParametersDoNotMatchPropertiesOrFields, typeof(ClassWhereParametersDoNotMatchProperties), "age"), exception.Message); } @@ -1207,7 +1228,7 @@ public void ExceptionWhenTryingToBindConfigToClassWhereNoMatchingParameterIsFoun var exception = Assert.Throws( () => config.Bind(new TestOptions())); Assert.Equal( - SR.Format(SR.Error_ConstructorParametersDoNotMatchProperties, typeof(ClassWhereParametersDoNotMatchProperties), "age"), + SR.Format(SR.Error_ConstructorParametersDoNotMatchPropertiesOrFields, typeof(ClassWhereParametersDoNotMatchProperties), "age"), exception.Message); } @@ -1232,6 +1253,28 @@ public void BindsToClassConstructorParametersWithDefaultValues() Assert.Equal(42, testOptions.ClassWhereParametersHaveDefaultValueProperty.Age); } + [Fact] + public void BindsToClassConstructorParametersWhereTheyMatchPropertiesAndFields() + { + var input = new Dictionary + { + {"ClassWhereParametersMatchPropertiesAndFieldsProperty:Name", "John"}, + {"ClassWhereParametersMatchPropertiesAndFieldsProperty:Address", "123, Abc St."}, + {"ClassWhereParametersMatchPropertiesAndFieldsProperty:Age", "42"} + }; + + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(input); + var config = configurationBuilder.Build(); + + TestOptions testOptions = new TestOptions(); + + config.Bind(testOptions); + Assert.Equal("John", testOptions.ClassWhereParametersMatchPropertiesAndFieldsProperty.Name); + Assert.Equal("123, Abc St.", testOptions.ClassWhereParametersMatchPropertiesAndFieldsProperty.Address); + Assert.Equal(42, testOptions.ClassWhereParametersMatchPropertiesAndFieldsProperty.GetAge()); + } + [Fact] public void BindsToRecordPrimaryConstructorParametersWithDefaultValues() { @@ -1745,6 +1788,7 @@ private class TestOptions public ClassWhereParametersDoNotMatchProperties ClassWhereParametersDoNotMatchPropertiesProperty { get; set; } public Line LineProperty { get; set; } public ClassWhereParametersHaveDefaultValue ClassWhereParametersHaveDefaultValueProperty { get; set; } + public ClassWhereParametersMatchPropertiesAndFields ClassWhereParametersMatchPropertiesAndFieldsProperty { get; set; } public RecordWhereParametersHaveDefaultValue RecordWhereParametersHaveDefaultValueProperty { get; set; } public int IntProperty { get; set; } From 5029862bd647d6167d65201063b940f4ea148bf3 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Wed, 1 Jun 2022 05:53:34 +0100 Subject: [PATCH 36/41] Remove check that constructor binding checks presense of fields. --- .../src/ConfigurationBinder.cs | 12 ++++++------ .../src/Resources/Strings.resx | 4 ++-- .../tests/ConfigurationBinderTests.cs | 16 ++++++++-------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index dee240dfbf0669..afb68034b1b6c7 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -452,9 +452,9 @@ private static object CreateInstance( List properties = GetAllProperties(type); List fields = GetAllFields(type); - if (!DoAllParametersHaveEquivalentPropertiesOrFields(parameters, properties, fields, out string nameOfInvalidParameters)) + if (!DoAllParametersHaveEquivalentProperties(parameters, properties, out string nameOfInvalidParameters)) { - throw new InvalidOperationException(SR.Format(SR.Error_ConstructorParametersDoNotMatchPropertiesOrFields, type, nameOfInvalidParameters)); + throw new InvalidOperationException(SR.Format(SR.Error_ConstructorParametersDoNotMatchProperties, type, nameOfInvalidParameters)); } object?[] parameterValues = new object?[parameters.Length]; @@ -480,13 +480,13 @@ private static object CreateInstance( return instance ?? throw new InvalidOperationException(SR.Format(SR.Error_FailedToActivate, type)); } - private static bool DoAllParametersHaveEquivalentPropertiesOrFields(ParameterInfo[] parameters, - List properties, List fieldInfos, out string s) + private static bool DoAllParametersHaveEquivalentProperties(ParameterInfo[] parameters, + List properties, out string s) { var parameterNames = parameters.Select(param => param.Name).ToList(); - var propertyAndFieldNames = properties.Select(prop => prop.Name).Concat(fieldInfos.Select(fi => fi.Name)).ToList(); + var propertyNames = properties.Select(prop => prop.Name).ToList(); - var missingProperties = parameterNames.Where(pn => !propertyAndFieldNames.Contains(pn!, StringComparer.OrdinalIgnoreCase)); + var missingProperties = parameterNames.Where(pn => !propertyNames.Contains(pn!, StringComparer.OrdinalIgnoreCase)); s = string.Join(",", missingProperties); diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx index dc18fb27e48026..18ebe4b0aa7e52 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx @@ -123,8 +123,8 @@ Cannot create instance of type '{0}' because one or more parameters cannot be bound to. Constructor parameters cannot be declared as in, out, or ref. Invalid parameters are: '{1}' - - Cannot create instance of type '{0}' because one or more parameters cannot be bound to. Constructor parameters must have corresponding properties or fields. Missing properties are: '{1}' + + Cannot create instance of type '{0}' because one or more parameters cannot be bound to. Constructor parameters must have corresponding properties. Fields are not supported. Missing properties are: '{1}' Failed to convert configuration value at '{0}' to type '{1}'. diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index c35bd43314e3f2..932b6111fb25a3 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -1187,7 +1187,7 @@ public void ExceptionWhenTryingToBindClassWherePropertiesDoMatchConstructorParam var exception = Assert.Throws( () => config.Bind(new TestOptions())); Assert.Equal( - SR.Format(SR.Error_ConstructorParametersDoNotMatchPropertiesOrFields, typeof(ClassWhereParametersDoNotMatchProperties), "age"), + SR.Format(SR.Error_ConstructorParametersDoNotMatchProperties, typeof(ClassWhereParametersDoNotMatchProperties), "age"), exception.Message); } @@ -1228,7 +1228,7 @@ public void ExceptionWhenTryingToBindConfigToClassWhereNoMatchingParameterIsFoun var exception = Assert.Throws( () => config.Bind(new TestOptions())); Assert.Equal( - SR.Format(SR.Error_ConstructorParametersDoNotMatchPropertiesOrFields, typeof(ClassWhereParametersDoNotMatchProperties), "age"), + SR.Format(SR.Error_ConstructorParametersDoNotMatchProperties, typeof(ClassWhereParametersDoNotMatchProperties), "age"), exception.Message); } @@ -1254,7 +1254,7 @@ public void BindsToClassConstructorParametersWithDefaultValues() } [Fact] - public void BindsToClassConstructorParametersWhereTheyMatchPropertiesAndFields() + public void FieldsNotSupported_ExceptionBindingToConstructorWithParameterMatchingAField() { var input = new Dictionary { @@ -1267,12 +1267,12 @@ public void BindsToClassConstructorParametersWhereTheyMatchPropertiesAndFields() configurationBuilder.AddInMemoryCollection(input); var config = configurationBuilder.Build(); - TestOptions testOptions = new TestOptions(); + var exception = Assert.Throws( + () => config.Bind(new TestOptions())); - config.Bind(testOptions); - Assert.Equal("John", testOptions.ClassWhereParametersMatchPropertiesAndFieldsProperty.Name); - Assert.Equal("123, Abc St.", testOptions.ClassWhereParametersMatchPropertiesAndFieldsProperty.Address); - Assert.Equal(42, testOptions.ClassWhereParametersMatchPropertiesAndFieldsProperty.GetAge()); + Assert.Equal( + SR.Format(SR.Error_ConstructorParametersDoNotMatchProperties, typeof(ClassWhereParametersMatchPropertiesAndFields), "age"), + exception.Message); } [Fact] From 6aa72add9ba57b5c6067c3be653ef27e48924284 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Thu, 2 Jun 2022 16:17:27 +0100 Subject: [PATCH 37/41] Use TryGetDefaultValue from common --- .../src/ConfigurationBinder.cs | 5 +++-- .../src/Microsoft.Extensions.Configuration.Binder.csproj | 9 +++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index afb68034b1b6c7..7c1639ae245404 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -9,6 +9,7 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection; +using Microsoft.Extensions.Internal; namespace Microsoft.Extensions.Configuration { @@ -788,9 +789,9 @@ private static List GetAllFields( if (propertyBindingPoint.Value is null) { - if (parameter.HasDefaultValue) + if (ParameterDefaultValue.TryGetDefaultValue(parameter, out object? defaultValue)) { - propertyBindingPoint.SetValue(parameter.DefaultValue); + propertyBindingPoint.SetValue(defaultValue); } else { diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj index 747e97a01882fc..d74d502d6b573e 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj @@ -9,14 +9,19 @@ - + + + + + + + From dee80fd882385c011e0a05df8940ef2c37419883 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Thu, 2 Jun 2022 16:18:48 +0100 Subject: [PATCH 38/41] Remove unused local and method for fields --- .../src/ConfigurationBinder.cs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 7c1639ae245404..84297d65dcbdc2 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -451,7 +451,6 @@ private static object CreateInstance( List properties = GetAllProperties(type); - List fields = GetAllFields(type); if (!DoAllParametersHaveEquivalentProperties(parameters, properties, out string nameOfInvalidParameters)) { @@ -757,23 +756,6 @@ private static List GetAllProperties( return allProperties; } - private static List GetAllFields( - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] - Type type) - { - var allFields = new List(); - - Type? baseType = type; - do - { - allFields.AddRange(baseType!.GetFields(DeclaredOnlyLookup)); - baseType = baseType.BaseType; - } - while (baseType != typeof(object)); - - return allFields; - } - [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] private static object? BindParameter(ParameterInfo parameter, Type type, IConfiguration config, BinderOptions options) From 2a59c9711f3c18aa709d28e0395426a9a98630fc Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Thu, 2 Jun 2022 16:34:24 +0100 Subject: [PATCH 39/41] Remove linq --- .../src/ConfigurationBinder.cs | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 84297d65dcbdc2..faa242c5f22882 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -481,16 +481,28 @@ private static object CreateInstance( } private static bool DoAllParametersHaveEquivalentProperties(ParameterInfo[] parameters, - List properties, out string s) + List properties, out string missing) { - var parameterNames = parameters.Select(param => param.Name).ToList(); - var propertyNames = properties.Select(prop => prop.Name).ToList(); + HashSet propertyNames = new(StringComparer.OrdinalIgnoreCase); + foreach (PropertyInfo prop in properties) + { + propertyNames.Add(prop.Name); + } - var missingProperties = parameterNames.Where(pn => !propertyNames.Contains(pn!, StringComparer.OrdinalIgnoreCase)); + List missingParameters = new(); + + foreach (ParameterInfo parameter in parameters) + { + string name = parameter.Name!; + if (!propertyNames.Contains(name)) + { + missingParameters.Add(name); + } + } - s = string.Join(",", missingProperties); + missing = string.Join(",", missingParameters); - return s.Length == 0; + return missing.Length == 0; } private static bool CanBindToTheseConstructorParameters(ParameterInfo[] constructorParameters, out string nameOfInvalidParameter) From 888672cd012308426038a9d9eb1f5a4f5ddf6020 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Wed, 8 Jun 2022 09:16:23 -0500 Subject: [PATCH 40/41] Revert unnecessary change. --- .../src/ConfigurationBinder.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index faa242c5f22882..2d2f5658188a44 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -352,6 +352,7 @@ private static void BindInstance( // Leaf nodes are always reinitialized bindingPoint.TrySetValue(convertedValue); + return; } if (config != null && config.GetChildren().Any()) From 24cd5a42d76e16e44ed87a4e0a4dc5e77bf2e9ef Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 8 Jun 2022 13:19:12 -0400 Subject: [PATCH 41/41] Update src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx --- .../src/Resources/Strings.resx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx index 18ebe4b0aa7e52..197b325252ef91 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx @@ -124,7 +124,7 @@ Cannot create instance of type '{0}' because one or more parameters cannot be bound to. Constructor parameters cannot be declared as in, out, or ref. Invalid parameters are: '{1}' - Cannot create instance of type '{0}' because one or more parameters cannot be bound to. Constructor parameters must have corresponding properties. Fields are not supported. Missing properties are: '{1}' + Cannot create instance of type '{0}' because one or more parameters cannot be bound to. Constructor parameters must have corresponding properties. Fields are not supported. Missing properties are: '{1}' Failed to convert configuration value at '{0}' to type '{1}'.