Fix binding types with optional string parameters (#93563)

ericstj · web-flow · commit cf47d9ff6827 · 2023-11-06T08:26:00.000-08:00
* Add a baseline test for constructor parameters

* Fix binding types with optional string parameters

Ensure the source generator emits the declaration with default value for
all cases when we emit the bind logic.

* Add a test case that uses a Primary Constructor with default values

* Split baseline data for added test.

* Fix .NETFramework test baseline

* Update baselines after global namespace change
diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs
@@ -379,7 +379,6 @@ void EmitBindImplForMember(MemberSpec member)
                     TypeSpec memberType = _typeIndex.GetTypeSpec(member.TypeRef);
                     string parsedMemberDeclarationLhs = $"{memberType.TypeRef.FullyQualifiedName} {member.Name}";
                     string configKeyName = member.ConfigurationKeyName;
-                    string parsedMemberAssignmentLhsExpr;
 
                     switch (memberType)
                     {
@@ -392,31 +391,22 @@ void EmitBindImplForMember(MemberSpec member)
                                     _writer.WriteLine();
                                     return;
                                 }
-
-                                parsedMemberAssignmentLhsExpr = parsedMemberDeclarationLhs;
                             }
                             break;
                         case ConfigurationSectionSpec:
                             {
                                 _writer.WriteLine($"{parsedMemberDeclarationLhs} = {GetSectionFromConfigurationExpression(configKeyName)};");
                                 return;
                             }
-                        default:
-                            {
-                                string bangExpr = memberType.IsValueType ? string.Empty : "!";
-                                string parsedMemberIdentifierDeclaration = $"{parsedMemberDeclarationLhs} = {member.DefaultValueExpr}{bangExpr};";
-
-                                _writer.WriteLine(parsedMemberIdentifierDeclaration);
-                                _emitBlankLineBeforeNextStatement = false;
-
-                                parsedMemberAssignmentLhsExpr = member.Name;
-                            }
-                            break;
                     }
 
+                    string bangExpr = memberType.IsValueType ? string.Empty : "!";
+                    _writer.WriteLine($"{parsedMemberDeclarationLhs} = {member.DefaultValueExpr}{bangExpr};");
+                    _emitBlankLineBeforeNextStatement = false;
+
                     bool canBindToMember = this.EmitBindImplForMember(
                         member,
-                        parsedMemberAssignmentLhsExpr,
+                        member.Name,
                         sectionPathExpr: GetSectionPathFromConfigurationExpression(configKeyName),
                         canSet: true,
                         InitializationKind.None);
diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs
@@ -129,7 +129,7 @@ public class ClassWhereParametersHaveDefaultValue
             public StringComparison? NSC { get; }
             public char? NC { get; }
 
-            public ClassWhereParametersHaveDefaultValue(string? name, string address, 
+            public ClassWhereParametersHaveDefaultValue(string? name = "John Doe", string address = "1 Microsoft Way",
                 int age = 42, float f = 42.0f, double d = 3.14159, decimal m = 3.1415926535897932384626433M, StringComparison sc = StringComparison.Ordinal, char c = 'q',
                 int? nage = 42, float? nf = 42.0f, double? nd = 3.14159, decimal? nm = 3.1415926535897932384626433M, StringComparison? nsc = StringComparison.Ordinal, char? nc = 'q')
             {
@@ -150,13 +150,19 @@ public ClassWhereParametersHaveDefaultValue(string? name, string address,
             }
         }
 
-
         public class ClassWithPrimaryCtor(string color, int length)
         {
             public string Color { get; } = color;
             public int Length { get; } = length;
         }
 
+        public class ClassWithPrimaryCtorDefaultValues(string color = "blue", int length = 15, decimal height = 5.946238490567943927384M, EditorBrowsableState eb = EditorBrowsableState.Never)
+        {
+            public string Color { get; } = color;
+            public int Length { get; } = length;
+            public decimal Height { get; } = height;
+            public EditorBrowsableState EB { get;} = eb;
+        }
         public record RecordTypeOptions(string Color, int Length);
 
         public record Line(string Color, int Length, int Thickness);
diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs
@@ -1398,6 +1398,24 @@ public void CanBindClassWithPrimaryCtor()
             Assert.Equal("Green", options.Color);
         }
 
+        [Fact]
+        public void CanBindClassWithPrimaryCtorWithDefaultValues()
+        {
+            var dic = new Dictionary<string, string>
+            {
+                {"Length", "-1"}
+            };
+            var configurationBuilder = new ConfigurationBuilder();
+            configurationBuilder.AddInMemoryCollection(dic);
+            var config = configurationBuilder.Build();
+
+            var options = config.Get<ClassWithPrimaryCtorDefaultValues>();
+            Assert.Equal(-1, options.Length);
+            Assert.Equal("blue", options.Color);
+            Assert.Equal(5.946238490567943927384M, options.Height);
+            Assert.Equal(EditorBrowsableState.Never, options.EB);
+        }
+
         [Fact]
         public void CanBindRecordStructOptions()
         {
diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/net462/DefaultConstructorParameters.generated.txt b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/net462/DefaultConstructorParameters.generated.txt
@@ -0,0 +1,249 @@
+// <auto-generated/>
+
+#nullable enable annotations
+#nullable disable warnings
+
+// Suppress warnings about [Obsolete] member usage in generated code.
+#pragma warning disable CS0612, CS0618
+
+namespace System.Runtime.CompilerServices
+{
+    using System;
+    using System.CodeDom.Compiler;
+
+    [GeneratedCode("Microsoft.Extensions.Configuration.Binder.SourceGeneration", "42.42.42.42")]
+    [AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
+    file sealed class InterceptsLocationAttribute : Attribute
+    {
+        public InterceptsLocationAttribute(string filePath, int line, int column)
+        {
+        }
+    }
+}
+
+namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
+{
+    using Microsoft.Extensions.Configuration;
+    using System;
+    using System.CodeDom.Compiler;
+    using System.Collections.Generic;
+    using System.Globalization;
+    using System.Runtime.CompilerServices;
+
+    [GeneratedCode("Microsoft.Extensions.Configuration.Binder.SourceGeneration", "42.42.42.42")]
+    file static class BindingExtensions
+    {
+        #region IConfiguration extensions.
+        /// <summary>Attempts to bind the given object instance to configuration values by matching property names against configuration keys recursively.</summary>
+        [InterceptsLocation(@"src-0.cs", 13, 16)]
+        public static void Bind_ProgramClassWhereParametersHaveDefaultValue(this IConfiguration configuration, object? instance)
+        {
+            if (configuration is null)
+            {
+                throw new ArgumentNullException(nameof(configuration));
+            }
+
+            if (instance is null)
+            {
+                return;
+            }
+
+            var typedObj = (global::Program.ClassWhereParametersHaveDefaultValue)instance;
+            BindCore(configuration, ref typedObj, defaultValueIfNotFound: false, binderOptions: null);
+        }
+        #endregion IConfiguration extensions.
+
+        #region Core binding extensions.
+        private readonly static Lazy<HashSet<string>> s_configKeys_ProgramClassWhereParametersHaveDefaultValue = new(() => new HashSet<string>(StringComparer.OrdinalIgnoreCase) { "Name", "Address", "Age", "F", "D", "M", "SC", "C", "NAge", "NF", "ND", "NM", "NSC", "NC" });
+
+        public static void BindCore(IConfiguration configuration, ref global::Program.ClassWhereParametersHaveDefaultValue instance, bool defaultValueIfNotFound, BinderOptions? binderOptions)
+        {
+            ValidateConfigurationKeys(typeof(global::Program.ClassWhereParametersHaveDefaultValue), s_configKeys_ProgramClassWhereParametersHaveDefaultValue, configuration, binderOptions);
+        }
+
+        public static global::Program.ClassWhereParametersHaveDefaultValue InitializeProgramClassWhereParametersHaveDefaultValue(IConfiguration configuration, BinderOptions? binderOptions)
+        {
+            string name = "John Doe"!;
+            if (configuration["Name"] is string value0)
+            {
+                name = value0;
+            }
+
+            string address = "1 Microsoft Way"!;
+            if (configuration["Address"] is string value1)
+            {
+                address = value1;
+            }
+
+            int age = (int)(42);
+            if (configuration["Age"] is string value2)
+            {
+                age = ParseInt(value2, () => configuration.GetSection("Age").Path);
+            }
+
+            float f = 42F;
+            if (configuration["F"] is string value3)
+            {
+                f = ParseFloat(value3, () => configuration.GetSection("F").Path);
+            }
+
+            double d = 3.1415899999999999D;
+            if (configuration["D"] is string value4)
+            {
+                d = ParseDouble(value4, () => configuration.GetSection("D").Path);
+            }
+
+            decimal m = 3.1415926535897932384626433M;
+            if (configuration["M"] is string value5)
+            {
+                m = ParseDecimal(value5, () => configuration.GetSection("M").Path);
+            }
+
+            global::System.StringComparison sc = (global::System.StringComparison)(4);
+            if (configuration["SC"] is string value6)
+            {
+                sc = ParseEnum<global::System.StringComparison>(value6, () => configuration.GetSection("SC").Path);
+            }
+
+            char c = 'q';
+            if (configuration["C"] is string value7)
+            {
+                c = ParseChar(value7, () => configuration.GetSection("C").Path);
+            }
+
+            int? nage = (int?)(42);
+            if (configuration["NAge"] is string value8)
+            {
+                nage = ParseInt(value8, () => configuration.GetSection("NAge").Path);
+            }
+
+            float? nf = 42F;
+            if (configuration["NF"] is string value9)
+            {
+                nf = ParseFloat(value9, () => configuration.GetSection("NF").Path);
+            }
+
+            double? nd = 3.1415899999999999D;
+            if (configuration["ND"] is string value10)
+            {
+                nd = ParseDouble(value10, () => configuration.GetSection("ND").Path);
+            }
+
+            decimal? nm = 3.1415926535897932384626433M;
+            if (configuration["NM"] is string value11)
+            {
+                nm = ParseDecimal(value11, () => configuration.GetSection("NM").Path);
+            }
+
+            global::System.StringComparison? nsc = (global::System.StringComparison?)(4);
+            if (configuration["NSC"] is string value12)
+            {
+                nsc = ParseEnum<global::System.StringComparison>(value12, () => configuration.GetSection("NSC").Path);
+            }
+
+            char? nc = 'q';
+            if (configuration["NC"] is string value13)
+            {
+                nc = ParseChar(value13, () => configuration.GetSection("NC").Path);
+            }
+
+            return new global::Program.ClassWhereParametersHaveDefaultValue(name, address, age, f, d, m, sc, c, nage, nf, nd, nm, nsc, nc);
+        }
+
+
+        /// <summary>If required by the binder options, validates that there are no unknown keys in the input configuration object.</summary>
+        public static void ValidateConfigurationKeys(Type type, Lazy<HashSet<string>> keys, IConfiguration configuration, BinderOptions? binderOptions)
+        {
+            if (binderOptions?.ErrorOnUnknownConfiguration is true)
+            {
+                List<string>? temp = null;
+        
+                foreach (IConfigurationSection section in configuration.GetChildren())
+                {
+                    if (!keys.Value.Contains(section.Key))
+                    {
+                        (temp ??= new List<string>()).Add($"'{section.Key}'");
+                    }
+                }
+        
+                if (temp is not null)
+                {
+                    throw new InvalidOperationException($"'ErrorOnUnknownConfiguration' was set on the provided BinderOptions, but the following properties were not found on the instance of {type}: {string.Join(", ", temp)}");
+                }
+            }
+        }
+
+        public static T ParseEnum<T>(string value, Func<string?> getPath) where T : struct
+        {
+            try
+            {
+                return (T)Enum.Parse(typeof(T), value, ignoreCase: true);
+            }
+            catch (Exception exception)
+            {
+                throw new InvalidOperationException($"Failed to convert configuration value at '{getPath()}' to type '{typeof(T)}'.", exception);
+            }
+        }
+
+        public static int ParseInt(string value, Func<string?> getPath)
+        {
+            try
+            {
+                return int.Parse(value, NumberStyles.Integer, CultureInfo.InvariantCulture);
+            }
+            catch (Exception exception)
+            {
+                throw new InvalidOperationException($"Failed to convert configuration value at '{getPath()}' to type '{typeof(int)}'.", exception);
+            }
+        }
+
+        public static float ParseFloat(string value, Func<string?> getPath)
+        {
+            try
+            {
+                return float.Parse(value, NumberStyles.Float, CultureInfo.InvariantCulture);
+            }
+            catch (Exception exception)
+            {
+                throw new InvalidOperationException($"Failed to convert configuration value at '{getPath()}' to type '{typeof(float)}'.", exception);
+            }
+        }
+
+        public static double ParseDouble(string value, Func<string?> getPath)
+        {
+            try
+            {
+                return double.Parse(value, NumberStyles.Float, CultureInfo.InvariantCulture);
+            }
+            catch (Exception exception)
+            {
+                throw new InvalidOperationException($"Failed to convert configuration value at '{getPath()}' to type '{typeof(double)}'.", exception);
+            }
+        }
+
+        public static decimal ParseDecimal(string value, Func<string?> getPath)
+        {
+            try
+            {
+                return decimal.Parse(value, NumberStyles.Float, CultureInfo.InvariantCulture);
+            }
+            catch (Exception exception)
+            {
+                throw new InvalidOperationException($"Failed to convert configuration value at '{getPath()}' to type '{typeof(decimal)}'.", exception);
+            }
+        }
+
+        public static char ParseChar(string value, Func<string?> getPath)
+        {
+            try
+            {
+                return char.Parse(value);
+            }
+            catch (Exception exception)
+            {
+                throw new InvalidOperationException($"Failed to convert configuration value at '{getPath()}' to type '{typeof(char)}'.", exception);
+            }
+        }
+        #endregion Core binding extensions.
+    }
+}
diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/netcoreapp/DefaultConstructorParameters.generated.txt b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Baselines/netcoreapp/DefaultConstructorParameters.generated.txt
diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.Baselines.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.Baselines.cs

Original file line number	Diff line number	Diff line change
`@@ -129,7 +129,7 @@ public class ClassWhereParametersHaveDefaultValue`
`129`	`129`	`public StringComparison? NSC { get; }`
`130`	`130`	`public char? NC { get; }`
`131`	`131`
`132`		`- public ClassWhereParametersHaveDefaultValue(string? name, string address,`
	`132`	`+ public ClassWhereParametersHaveDefaultValue(string? name = "John Doe", string address = "1 Microsoft Way",`
`133`	`133`	`int age = 42, float f = 42.0f, double d = 3.14159, decimal m = 3.1415926535897932384626433M, StringComparison sc = StringComparison.Ordinal, char c = 'q',`
`134`	`134`	`int? nage = 42, float? nf = 42.0f, double? nd = 3.14159, decimal? nm = 3.1415926535897932384626433M, StringComparison? nsc = StringComparison.Ordinal, char? nc = 'q')`
`135`	`135`	`{`
`@@ -150,13 +150,19 @@ public ClassWhereParametersHaveDefaultValue(string? name, string address,`
`150`	`150`	`}`
`151`	`151`	`}`
`152`	`152`
`153`		`-`
`154`	`153`	`public class ClassWithPrimaryCtor(string color, int length)`
`155`	`154`	`{`
`156`	`155`	`public string Color { get; } = color;`
`157`	`156`	`public int Length { get; } = length;`
`158`	`157`	`}`
`159`	`158`
	`159`	`+ public class ClassWithPrimaryCtorDefaultValues(string color = "blue", int length = 15, decimal height = 5.946238490567943927384M, EditorBrowsableState eb = EditorBrowsableState.Never)`
	`160`	`+ {`
	`161`	`+ public string Color { get; } = color;`
	`162`	`+ public int Length { get; } = length;`
	`163`	`+ public decimal Height { get; } = height;`
	`164`	`+ public EditorBrowsableState EB { get;} = eb;`
	`165`	`+ }`
`160`	`166`	`public record RecordTypeOptions(string Color, int Length);`
`161`	`167`
`162`	`168`	`public record Line(string Color, int Length, int Thickness);`