[release/9.0] Fix config source gen binding value types to null confi…

…guration values (#107670) * Fix config source gen binding value types to null configuration values * Address the feedback --------- Co-authored-by: Tarek Mahmoud Sayed <tarekms@microsoft.com>
dotnet · Sep 11, 2024 · 9bf6440 · 9bf6440
1 parent 3104ebb
commit 9bf6440
Show file tree

Hide file tree

Showing 47 changed files with 278 additions and 174 deletions.
diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs
@@ -1109,7 +1109,9 @@ private void EmitBindingLogic(
                 }
                 else
                 {
-                    EmitStartBlock($"if ({sectionValueExpr} is string {nonNull_StringValue_Identifier})");
+                    // In case of calling parsing methods, check the section value first for null or empty before calling parse.
+                    string extraCondition = typeKind == StringParsableTypeKind.AssignFromSectionValue ? "" : $" && !string.IsNullOrEmpty({nonNull_StringValue_Identifier})";
+                    EmitStartBlock($"if ({sectionValueExpr} is string {nonNull_StringValue_Identifier}{extraCondition})");
                     writeOnSuccess?.Invoke(parsedValueExpr);
                     EmitEndBlock();
                 }

diff --git a/...soft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs b/...soft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs
@@ -1111,5 +1111,25 @@ private IEnumerable<KeyValuePair<string, string>> enumerate()
             IEnumerator IEnumerable.GetEnumerator() => enumerate().GetEnumerator();
         }
 
+        public class ParsableValuesClass
+        {
+            public int? IntValue { get; set; }
+            public double? DoubleValue { get; set; }
+            public bool? BoolValue { get; set; }
+            public decimal? DecimalValue { get; set; }
+            public float? FloatValue { get; set; }
+            public long? LongValue { get; set; }
+            public short? ShortValue { get; set; }
+            public byte? ByteValue { get; set; }
+            public sbyte? SByteValue { get; set; }
+            public uint? UIntValue { get; set; }
+            public ushort? UShortValue { get; set; }
+            public ulong? ULongValue { get; set; }
+            public DateTime? DateTimeValue { get; set; }
+            public DateTimeOffset? DateTimeOffsetValue { get; set; }
+            public TimeSpan? TimeSpanValue { get; set; }
+            public Guid? GuidValue { get; set; }
+            public StringComparison? StringComparisonValue { get; set; }
+        }
     }
 }
diff --git a/...raries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs b/...raries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs
@@ -1544,6 +1544,88 @@ public void CanBindOnParametersAndProperties_RecordWithArrayConstructorParameter
             Assert.Equal(new string[] { "a", "b", "c" }, options.Array);
         }
 
+
+        public static IEnumerable<object[]> Configuration_TestData()
+        {
+            yield return new object[]
+            {
+                new ConfigurationBuilder()
+                    .AddJsonStream(new System.IO.MemoryStream(System.Text.Encoding.UTF8.GetBytes(@"{
+                            ""IntValue"" :              """",
+                            ""DoubleValue"" :           """",
+                            ""BoolValue"" :             """",
+                            ""DecimalValue"" :          """",
+                            ""FloatValue"" :            """",
+                            ""LongValue"" :             """",
+                            ""ShortValue"" :            """",
+                            ""ByteValue"" :             """",
+                            ""SByteValue"" :            """",
+                            ""UIntValue"" :             """",
+                            ""UShortValue"" :           """",
+                            ""ULongValue"" :            """",
+                            ""DateTimeValue"" :         """",
+                            ""DateTimeOffsetValue"" :   """",
+                            ""TimeSpanValue"" :         """",
+                            ""GuidValue"" :             """",
+                            ""StringComparisonValue"" : """"
+                        }"
+                    ))).Build()
+            };
+
+            yield return new object[]
+            {
+                new ConfigurationBuilder().AddInMemoryCollection(
+                    new Dictionary<string, string>
+                    {
+                        { "IntValue",               null },
+                        { "DoubleValue",            null },
+                        { "BoolValue",              null },
+                        { "DecimalValue",           null },
+                        { "FloatValue",             null },
+                        { "LongValue",              null },
+                        { "ShortValue",             null },
+                        { "ByteValue",              null },
+                        { "SByteValue",             null },
+                        { "UIntValue",              null },
+                        { "UShortValue",            null },
+                        { "ULongValue",             null },
+                        { "DateTimeValue",          null },
+                        { "DateTimeOffsetValue",    null },
+                        { "TimeSpanValue",          null },
+                        { "GuidValue",              null },
+                        { "StringComparisonValue",  null },
+                    }).Build()
+            };
+        }
+
+        /// <summary>
+        /// Test binding a value parsable types to null configuration values.
+        /// The test ensure the binding will succeed without exceptions and the value will be null (not set).
+        /// </summary>
+        [Theory]
+        [MemberData(nameof(Configuration_TestData))]
+        public void BindToKnownParsableTypesWithNullValueTest(IConfiguration configuration)
+        {
+            ParsableValuesClass instance = configuration.Get<ParsableValuesClass>();
+            Assert.Null(instance.IntValue);
+            Assert.Null(instance.DoubleValue);
+            Assert.Null(instance.BoolValue);
+            Assert.Null(instance.DecimalValue);
+            Assert.Null(instance.FloatValue);
+            Assert.Null(instance.LongValue);
+            Assert.Null(instance.ShortValue);
+            Assert.Null(instance.ByteValue);
+            Assert.Null(instance.SByteValue);
+            Assert.Null(instance.UIntValue);
+            Assert.Null(instance.UShortValue);
+            Assert.Null(instance.ULongValue);
+            Assert.Null(instance.DateTimeValue);
+            Assert.Null(instance.DateTimeOffsetValue);
+            Assert.Null(instance.TimeSpanValue);
+            Assert.Null(instance.GuidValue);
+            Assert.Null(instance.StringComparisonValue);
+        }
+
         /// <summary>
         /// Test binding to recursive types using Dictionary or Collections.
         /// This ensure no stack overflow will occur during the compilation through the source gen or at runtime.

diff --git a/...ts/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Bind.generated.txt b/...ts/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Bind.generated.txt
@@ -96,7 +96,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
         {
             foreach (IConfigurationSection section in configuration.GetChildren())
             {
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     instance.Add(ParseInt(value, section.Path));
                 }
@@ -143,7 +143,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                 }
             }
 
-            if (configuration["MyInt"] is string value1)
+            if (configuration["MyInt"] is string value1 && !string.IsNullOrEmpty(value1))
             {
                 instance.MyInt = ParseInt(value1, configuration.GetSection("MyInt").Path);
             }

diff --git a/...GenerationTests/Baselines/net462/ConfigurationBinder/Version1/Bind_Instance.generated.txt b/...GenerationTests/Baselines/net462/ConfigurationBinder/Version1/Bind_Instance.generated.txt
@@ -60,7 +60,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
         {
             foreach (IConfigurationSection section in configuration.GetChildren())
             {
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     instance.Add(ParseInt(value, section.Path));
                 }
@@ -107,7 +107,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                 }
             }
 
-            if (configuration["MyInt"] is string value1)
+            if (configuration["MyInt"] is string value1 && !string.IsNullOrEmpty(value1))
             {
                 instance.MyInt = ParseInt(value1, configuration.GetSection("MyInt").Path);
             }

diff --git a/...s/Baselines/net462/ConfigurationBinder/Version1/Bind_Instance_BinderOptions.generated.txt b/...s/Baselines/net462/ConfigurationBinder/Version1/Bind_Instance_BinderOptions.generated.txt
@@ -60,7 +60,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
         {
             foreach (IConfigurationSection section in configuration.GetChildren())
             {
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     instance.Add(ParseInt(value, section.Path));
                 }
@@ -107,7 +107,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                 }
             }
 
-            if (configuration["MyInt"] is string value1)
+            if (configuration["MyInt"] is string value1 && !string.IsNullOrEmpty(value1))
             {
                 instance.MyInt = ParseInt(value1, configuration.GetSection("MyInt").Path);
             }

diff --git a/...rationTests/Baselines/net462/ConfigurationBinder/Version1/Bind_Key_Instance.generated.txt b/...rationTests/Baselines/net462/ConfigurationBinder/Version1/Bind_Key_Instance.generated.txt
@@ -60,7 +60,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
         {
             foreach (IConfigurationSection section in configuration.GetChildren())
             {
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     instance.Add(ParseInt(value, section.Path));
                 }
@@ -107,7 +107,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                 }
             }
 
-            if (configuration["MyInt"] is string value1)
+            if (configuration["MyInt"] is string value1 && !string.IsNullOrEmpty(value1))
             {
                 instance.MyInt = ParseInt(value1, configuration.GetSection("MyInt").Path);
             }

diff --git a/...sts/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Get.generated.txt b/...sts/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Get.generated.txt
@@ -89,7 +89,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
         {
             foreach (IConfigurationSection section in configuration.GetChildren())
             {
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     instance.Add(ParseInt(value, section.Path));
                 }
@@ -102,7 +102,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
 
             foreach (IConfigurationSection section in configuration.GetChildren())
             {
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     temp2.Add(ParseInt(value, section.Path));
                 }
@@ -141,7 +141,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                 }
             }
 
-            if (configuration["MyInt"] is string value4)
+            if (configuration["MyInt"] is string value4 && !string.IsNullOrEmpty(value4))
             {
                 instance.MyInt = ParseInt(value4, configuration.GetSection("MyInt").Path);
             }
@@ -191,7 +191,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
         {
             ValidateConfigurationKeys(typeof(global::Program.MyClass2), s_configKeys_ProgramMyClass2, configuration, binderOptions);
 
-            if (configuration["MyInt"] is string value14)
+            if (configuration["MyInt"] is string value14 && !string.IsNullOrEmpty(value14))
             {
                 instance.MyInt = ParseInt(value14, configuration.GetSection("MyInt").Path);
             }

diff --git a/...ationTests/Baselines/net462/ConfigurationBinder/Version1/Get_PrimitivesOnly.generated.txt b/...ationTests/Baselines/net462/ConfigurationBinder/Version1/Get_PrimitivesOnly.generated.txt
@@ -71,7 +71,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                 {
                     throw new InvalidOperationException();
                 }
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     return ParseInt(value, section.Path);
                 }
@@ -90,7 +90,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                 {
                     throw new InvalidOperationException();
                 }
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     return ParseFloat(value, section.Path);
                 }
@@ -101,7 +101,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                 {
                     throw new InvalidOperationException();
                 }
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     return ParseDouble(value, section.Path);
                 }

diff --git a/...s/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Get_T.generated.txt b/...s/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Get_T.generated.txt
@@ -70,7 +70,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
         {
             foreach (IConfigurationSection section in configuration.GetChildren())
             {
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     instance.Add(ParseInt(value, section.Path));
                 }
@@ -83,7 +83,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
 
             foreach (IConfigurationSection section in configuration.GetChildren())
             {
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     temp1.Add(ParseInt(value, section.Path));
                 }
@@ -122,7 +122,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                 }
             }
 
-            if (configuration["MyInt"] is string value3)
+            if (configuration["MyInt"] is string value3 && !string.IsNullOrEmpty(value3))
             {
                 instance.MyInt = ParseInt(value3, configuration.GetSection("MyInt").Path);
             }

diff --git a/...tionTests/Baselines/net462/ConfigurationBinder/Version1/Get_T_BinderOptions.generated.txt b/...tionTests/Baselines/net462/ConfigurationBinder/Version1/Get_T_BinderOptions.generated.txt
@@ -70,7 +70,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
         {
             foreach (IConfigurationSection section in configuration.GetChildren())
             {
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     instance.Add(ParseInt(value, section.Path));
                 }
@@ -83,7 +83,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
 
             foreach (IConfigurationSection section in configuration.GetChildren())
             {
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     temp1.Add(ParseInt(value, section.Path));
                 }
@@ -122,7 +122,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                 }
             }
 
-            if (configuration["MyInt"] is string value3)
+            if (configuration["MyInt"] is string value3 && !string.IsNullOrEmpty(value3))
             {
                 instance.MyInt = ParseInt(value3, configuration.GetSection("MyInt").Path);
             }

diff --git a/...rceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Get_TypeOf.generated.txt b/...rceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Get_TypeOf.generated.txt
@@ -70,7 +70,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
         {
             ValidateConfigurationKeys(typeof(global::Program.MyClass2), s_configKeys_ProgramMyClass2, configuration, binderOptions);
 
-            if (configuration["MyInt"] is string value1)
+            if (configuration["MyInt"] is string value1 && !string.IsNullOrEmpty(value1))
             {
                 instance.MyInt = ParseInt(value1, configuration.GetSection("MyInt").Path);
             }

diff --git a/...ests/Baselines/net462/ConfigurationBinder/Version1/Get_TypeOf_BinderOptions.generated.txt b/...ests/Baselines/net462/ConfigurationBinder/Version1/Get_TypeOf_BinderOptions.generated.txt
@@ -70,7 +70,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
         {
             ValidateConfigurationKeys(typeof(global::Program.MyClass2), s_configKeys_ProgramMyClass2, configuration, binderOptions);
 
-            if (configuration["MyInt"] is string value1)
+            if (configuration["MyInt"] is string value1 && !string.IsNullOrEmpty(value1))
             {
                 instance.MyInt = ParseInt(value1, configuration.GetSection("MyInt").Path);
             }

diff --git a/...eGenerationTests/Baselines/net462/OptionsBuilder/Version1/BindConfiguration.generated.txt b/...eGenerationTests/Baselines/net462/OptionsBuilder/Version1/BindConfiguration.generated.txt
@@ -101,7 +101,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
         {
             foreach (IConfigurationSection section in configuration.GetChildren())
             {
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     instance.Add(ParseInt(value, section.Path));
                 }
@@ -125,7 +125,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                 }
             }
 
-            if (configuration["MyInt"] is string value2)
+            if (configuration["MyInt"] is string value2 && !string.IsNullOrEmpty(value2))
             {
                 instance.MyInt = ParseInt(value2, configuration.GetSection("MyInt").Path);
             }

diff --git a/...elines/net462/OptionsBuilder/Version1/BindConfigurationWithConfigureActions.generated.txt b/...elines/net462/OptionsBuilder/Version1/BindConfigurationWithConfigureActions.generated.txt
@@ -101,7 +101,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
         {
             foreach (IConfigurationSection section in configuration.GetChildren())
             {
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     instance.Add(ParseInt(value, section.Path));
                 }
@@ -125,7 +125,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                 }
             }
 
-            if (configuration["MyInt"] is string value2)
+            if (configuration["MyInt"] is string value2 && !string.IsNullOrEmpty(value2))
             {
                 instance.MyInt = ParseInt(value2, configuration.GetSection("MyInt").Path);
             }

diff --git a/...tests/SourceGenerationTests/Baselines/net462/OptionsBuilder/Version1/Bind_T.generated.txt b/...tests/SourceGenerationTests/Baselines/net462/OptionsBuilder/Version1/Bind_T.generated.txt
@@ -107,7 +107,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
         {
             foreach (IConfigurationSection section in configuration.GetChildren())
             {
-                if (section.Value is string value)
+                if (section.Value is string value && !string.IsNullOrEmpty(value))
                 {
                     instance.Add(ParseInt(value, section.Path));
                 }
@@ -131,7 +131,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                 }
             }
 
-            if (configuration["MyInt"] is string value2)
+            if (configuration["MyInt"] is string value2 && !string.IsNullOrEmpty(value2))
             {
                 instance.MyInt = ParseInt(value2, configuration.GetSection("MyInt").Path);
             }