dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.91k stars 4.63k forks source link

Configuration Binding Generator does not correctly handle required fields in child objects. #101984

Closed mrudat closed 4 weeks ago

mrudat commented 4 months ago

Description

When using the Configuration Binding Generator with a configuration object with a child object with a required field, it attempts to construct the object without populating any required field causing a compiler error (CS9035).

Reproduction Steps

I put together https://github.com/mrudat/ConfigurationBindingGeneratorRequiredFailure to demonstrate the error.

If you restore the required attribute here, you will get a CS9035.

#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. - Workaround for bug in Configuration Binding Generator.
    public /*required*/ string RequiredField { get; set; }
#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable.

If you then comment out the line here to disable the Configuration Binding Generator, you no longer get CS9034, but you then get warnings IL2026 and IL3050 because you're once more using reflection.

    <EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator>

Expected behavior

The generated code will correctly handle child objects with required fields in a similar way that it already handles top-level required fields.

Actual behavior

CS9035 in the generated code due to using the parameterless constructor to (attempt) to instantiate the child object.

Regression?

No response

Known Workarounds

Removing the required from the field but keeping the Required attribute seems to work as desired:

    [Required]
#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. - Workaround for bug in Configuration Binding Generator.
    public /*required*/ string RequiredField { get; set; }
#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable.

Adding or removing a value for the required option correctly causes options validation to fail or ensures the option is present during Host startup:

                //["config:ChildOptionsWithRequiredField:RequiredField"] = "value"

Configuration

.NET 8, on Windows 11 using Visual Studio 2022 on x86

I do not believe that this is specific to this configuration.

Other information

The handling of child objects must differ from that of the parent object, as required top-level fields work exactly as expected.

dotnet-policy-service[bot] commented 4 months ago

Tagging subscribers to this area: @dotnet/area-extensions-configuration See info in area-owners.md if you want to be subscribed.

tarekgh commented 4 months ago

Thanks @mrudat for the report.

To simplify/summarize the issue:

Create appsettings.json file:

{
  "ChildOptionsWithRequiredField": {
    "RequiredField": "RequiredValue"
  }
}

Have the code:

    public sealed record ChildOptionsWithRequiredField
    {
        [Required]
        public required string RequiredField { get; set; }
    }

Then do the binding:

            var builder = new ConfigurationBuilder();
            builder.AddJsonFile("appsettings.json");
            var configuration = builder.Build();
            var options = configuration.GetSection("ChildOptionsWithRequiredField").Get<ChildOptionsWithRequiredField>(o => o.ErrorOnUnknownConfiguration = false);

Compile with enabling the configuration source gen <EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator>, will produce the following warning:

C:\NetCoreApp\obj\Debug\net9.0\generated\Microsoft.Extensions.Configuration.Binder.SourceGeneration\Microsoft.Extensions.Configuration.Binder.SourceGeneration.ConfigurationBindingGenerator\BindingExtensions.g.cs(58,36): error CS9035: Required member 'ChildOptionsWithRequiredField.RequiredField' must be set in the object initializer or attribute constructor.

The problem is the generator produce the following code which cause the warning:

            if (type == typeof(ChildOptionsWithRequiredField))
            {
                var instance = new ChildOptionsWithRequiredField(); // <<<<< This line produce the warning
                BindCore(configuration, ref instance, defaultValueIfNotFound: true, binderOptions);
                return instance;
            }

Disabling the source gen will make the code compile and run fine but apps enabling AOT will not be able use the reflection-based configuration and will need to enable the configuration source gen.

ericstj commented 4 weeks ago

This is a duplicate of https://github.com/dotnet/runtime/issues/95006 - we don't yet have support for required.