dotnet / runtime

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

Configuration binding generator produces incorrect type #95561

Closed mshwf closed 2 months ago

mshwf commented 10 months ago

Description

This was discovered while I was upgrading to .NET 8 from .NET 6. There were numerous errors in this code:

builder.Services.AddAuthentication().AddJwtBearer(JwtBearerDefaults.AuthenticationScheme, options =>
{
    // Nearly 300 errors caused by this line๐Ÿ‘‡, all say: "Property ... is not supported" or "Cannot create instance of type..."
    builder.Configuration.Bind("JwtSettings", options);
});

I created a minimal project to reproduce, and I found the configuration binder fails (or may be expected to fail in this case) to generate the correct type when there is explicit type casting from an inherited member.

Reproduction Steps

Create these models (these are similar to how JwtBearerOptions is structured which confuses the configuration binder):

public class SomeEvent { }
public class MyOptions : MyBaseOptions
{
    public new SomeEvent Events
    {
        get => (SomeEvent)base.Events!;
        set => base.Events = value;
    }
}
public class MyBaseOptions
{
    public object? Events { get; set; }
}

and try to bind MyOptions from a configuration section:

var myOptions = new MyOptions();
builder.Configuration.Bind("MyOptions", myOptions);

and add this to the CSProj file: <EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator> Try to build the project.

Expected behavior

I'd expect the same behavior as the .NET 6 configuration system, and no compilation errors.

Actual behavior

Compilation error due to incorrect source code generated by the configuration binder:

Cannot implicitly convert type 'string' to 'WebApplication8.SomeEvent' WebApplication8 ~\WebApplication8\WebApplication8\Microsoft.Extensions.Configuration.Binder.SourceGeneration\Microsoft.Extensions.Configuration.Binder.SourceGeneration.ConfigurationBindingGenerator\BindingExtensions.g.cs 137 Active

This is the generated method that produces the error:

public static void BindCore(IConfiguration configuration, ref MyOptions instance, bool defaultValueIfNotFound, BinderOptions? binderOptions)
{
    ValidateConfigurationKeys(typeof(MyOptions), s_configKeys_MyOptions, configuration, binderOptions);

    if (configuration["Events"] is string value10)
    {
        instance.Events = value10; // โŒ๐Ÿ‘ˆ Cannot implicitly convert type 'string' to 'WebApplication8.SomeEvent' 
    }
}

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

ghost commented 10 months ago

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

Issue Details
### Description This was discovered while I was upgrading to .NET 8 from .NET 6. There were numerous errors in this code: ``` builder.Services.AddAuthentication(opts => { }).AddJwtBearer(JwtBearerDefaults.AuthenticationScheme, options => { // Nearly 300 errors caused by this line๐Ÿ‘‡, all say: "Property ... is not supported" or "Cannot create instance of type..." builder.Configuration.Bind("JwtSettings", options); }) ``` I created a minimal project to reproduce, and I found the configuration binder fails (or may be expected to fail in this case) to generate the correct type when there is explicit type casting from an inherited member. Here are the steps to reproduce Create these models (these are similar to how `JwtBearerOptions` is structured which confuses the configuration binder): ``` public class SomeEvent { } public class MyOptions : MyBaseOptions { public new SomeEvent Events { get => (SomeEvent)base.Events!; set => base.Events = value; } } public class MyBaseOptions { public object? Events { get; set; } } ``` and try to bind `MyOptions` from a configuration section: ``` var myOptions = new MyOptions(); builder.Configuration.Bind("MyOptions", myOptions); ``` and add this to the CSProj file: `true ` try to build the project. ### Reproduction Steps Create these models (these are similar to how `JwtBearerOptions` is structured which confuses the configuration binder): ``` public class SomeEvent { } public class MyOptions : MyBaseOptions { public new SomeEvent Events { get => (SomeEvent)base.Events!; set => base.Events = value; } } public class MyBaseOptions { public object? Events { get; set; } } ``` and try to bind `MyOptions` from a configuration section: ``` var myOptions = new MyOptions(); builder.Configuration.Bind("MyOptions", myOptions); ``` and add this to the CSProj file: `true ` try to build the project. ``` public static void BindCore(IConfiguration configuration, ref MyOptions instance, bool defaultValueIfNotFound, BinderOptions? binderOptions) { ValidateConfigurationKeys(typeof(MyOptions), s_configKeys_MyOptions, configuration, binderOptions); if (configuration["Events"] is string value10) { instance.Events = value10; // โŒ๐Ÿ‘ˆ Cannot implicitly convert type 'string' to 'WebApplication8.SomeEvent' } } ``` ### Expected behavior I'd expect the same behavior as the .NET 6 configuration system, and no compilation errors. ### Actual behavior Compilation error due to incorrect source code generated by the configuration binder: > Cannot implicitly convert type 'string' to 'WebApplication8.SomeEvent' WebApplication8 ~\WebApplication8\WebApplication8\Microsoft.Extensions.Configuration.Binder.SourceGeneration\Microsoft.Extensions.Configuration.Binder.SourceGeneration.ConfigurationBindingGenerator\BindingExtensions.g.cs 137 Active This is the generated method that produces the error: ``` public static void BindCore(IConfiguration configuration, ref MyOptions instance, bool defaultValueIfNotFound, BinderOptions? binderOptions) { ValidateConfigurationKeys(typeof(MyOptions), s_configKeys_MyOptions, configuration, binderOptions); if (configuration["Events"] is string value10) { instance.Events = value10; // โŒ๐Ÿ‘ˆ Cannot implicitly convert type 'string' to 'WebApplication8.SomeEvent' } } ``` ### Regression? _No response_ ### Known Workarounds _No response_ ### Configuration _No response_ ### Other information _No response_
Author: mshwf
Assignees: -
Labels: `untriaged`, `area-Extensions-Configuration`
Milestone: -
tarekgh commented 10 months ago

https://github.com/dotnet/runtime/issues/94071

ericstj commented 2 months ago

This was fixed in https://github.com/dotnet/runtime/pull/94588

Can you make sure you update your package reference (if using the package) or update your SDK you aren't?

    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="8.0.2" />
mshwf commented 2 months ago

@ericstj I'm using the latest version of Visual Studio - v17.10.5 (so SDK is updated?), also added the mentioned package Microsoft.Extensions.Configuration.Binder (should I do something with it or it's for the SDK?), still have the same errors.

ericstj commented 2 months ago

It could be a namespace collision in your project with some other types you haven't shared? I'm not sure, I cannot reproduce the error you're seeing.

Here's an isolated repro that shows it working: testBindEvent.zip

You can see the generated code has

        public static void BindCore(IConfiguration configuration, ref global::MyOptions instance, bool defaultValueIfNotFound, BinderOptions? binderOptions)
        {
            ValidateConfigurationKeys(typeof(global::MyOptions), s_configKeys_MyOptions, configuration, binderOptions);

            if (AsConfigWithChildren(configuration.GetSection("Events")) is IConfigurationSection section0)
            {
                instance.Events ??= new global::SomeEvent();
            }
        }