Config generator wrongly emits init-related exception in bind operations

This code without the generator:

using Microsoft.Extensions.Configuration;

Environment.SetEnvironmentVariable("Value", "42");
IConfiguration b = new ConfigurationBuilder().AddEnvironmentVariables().Build();

Base d = new Derived();
b.Bind(d);
Console.WriteLine(d.Value);

internal abstract class Base
{
    public int Value { get; set; }
}

internal sealed class Derived : Base { }

prints 42.

With the config generator, that same code throws an exception, because the generator emits this:

public static void Bind_Base(this IConfiguration configuration, object? obj)
{
    throw new InvalidOperationException("Cannot create instance of type '<global namespace>.Base' because it is missing a public instance constructor.");
}

We have some options here, but handling polymorphism is a challenge in the source generator. I'm not sure if this should emit a diagnostic either. Assuming we don't do any polymorphic binding, what would you expect this to do:

using Microsoft.Extensions.Configuration;

Environment.SetEnvironmentVariable("Value", "42");
IConfiguration b = new ConfigurationBuilder().AddEnvironmentVariables().Build();

void DoBind(Base d) => b.Bind(d);
DoBind(new Derived());

Console.WriteLine(d.Value);

internal abstract class Base
{
    public int Value { get; set; }
}

internal sealed class Derived : Base { }

Assuming we don't do any polymorphic binding, what would you expect this to do

Either a) not emit any code for this and raise a diagnostic that says as much just as happens in other cases where it can't fully handle the situation, or b) emit code that handles setting everything on Base, since that's no different from what it would do if Base weren't abstract. Silently emitting code that will always fail at execution time seems like the worst outcome.

b) emit code that handles setting everything on Base, since that's no different from what it would do if Base weren't abstract. Silently emitting code that will always fail at execution time seems like the worst outcome.

This is the best we could do, but I thought this was a bad idea since it isn't what happens at runtime today and instead would silently ignoring all of the properties on your derived type. What's worse? Null properties or an exception that says no polymorphism is supported?

b) emit code that handles setting everything on Base, since that's no different from what it would do if Base weren't abstract. Silently emitting code that will always fail at execution time seems like the worst outcome.

This is the best we could do, but I thought this was a bad idea since it isn't what happens at runtime today and instead would silently ignoring all of the properties on your derived type. What's worse? Null properties or an exception that says no polymorphism is supported?

Why is the abstract case handled differently from this?

using Microsoft.Extensions.Configuration;

Environment.SetEnvironmentVariable("Value1", "42");
Environment.SetEnvironmentVariable("Value2", "84");
IConfiguration configuration = new ConfigurationBuilder().AddEnvironmentVariables().Build();

Derived d = new Derived();
Base b = d;
configuration.Bind(b);
Console.WriteLine(d.Value1);
Console.WriteLine(d.Value2);

internal class Base
{
    public int Value1 { get; set; }
}

internal sealed class Derived : Base
{
    public int Value2 { get; set; }
}

That's still polymorphic, the Value2 on Derived still won't be set with the generator, but there's no diagnostic and no exception.

I didn't realize that the abstract base class changed the behavior. That is inconsistent... I would have expected both to behavior similarly....

Taking a look, consistency is desired. We shouldn't throw here because we don't need to init the object. IConfiguration.Get should though.

@adamsitnik

Related issue of abstract/interface/polymorphic type handling shared by @adamsitnik in #91324.

I've tried to use the new configuration binder source generator to bind a type that contains a custom collection of abstract types.

I got following error:

C:\Users\adsitnik\source\repos\ConfigBindRepro\Microsoft.Extensions.Configuration.Binder.SourceGeneration\Microsoft.Extensions.Configuration.Binder.SourceGeneration.ConfigurationBindingGenerator\BindingExtensions.g.cs(61,29): error CS0
103: The name 'InitializeEndPoint' does not exist in the current context [C:\Users\adsitnik\source\repos\ConfigBindRepro\ConfigBindRepro.csproj]

I would expect it to not produce a compiler error and just try to map what it can, without throwing an exception at runtime (this is what the reflection based binder does in this case).

Repro:

Details

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <packageSources>
    <clear />
    <add key="dotnet8" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet8/nuget/v3/index.json" />
  </packageSources>
</configuration>

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator>
    <Features>InterceptorsPreview</Features>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="8.0.0-rc.2.23429.20" />
    <PackageReference Include="Microsoft.Extensions.Hosting" Version="8.0.0-rc.2.23429.20" />
  </ItemGroup>
</Project>

using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Hosting;
using System.Collections.ObjectModel;
using System.Net;

namespace ConfigBindRepro
{
    internal class Program
    {
        static void Main()
        {
            HostApplicationBuilder builder = Host.CreateEmptyApplicationBuilder(settings: null);
            builder.Configuration.AddInMemoryCollection(new KeyValuePair<string, string?>[]
            {
                 new KeyValuePair<string, string?>("ConfigBindRepro:EndPoints:0", "localhost"),
                 new KeyValuePair<string, string?>("ConfigBindRepro:Property", "true")
            });

            AClass instance = new();
#pragma warning disable SYSLIB1100
#pragma warning disable SYSLIB1101
            builder.Configuration.GetSection("ConfigBindRepro").Bind(instance);
#pragma warning restore

            BindToConfiguration(instance, builder.Configuration);
            if (instance.Property && instance.EndPoints.Any())
            {
                Console.WriteLine("OK");
            }
        }

        private static void BindToConfiguration(AClass settings, IConfiguration configuration)
        {
            var endpointsConfig = configuration.GetSection("ConfigBindRepro:EndPoints");
            foreach (var endpoint in endpointsConfig.GetChildren())
            {
                if (endpoint.Value is string)
                {
                    settings.EndPoints.Add(endpoint.Value);
                }
            }
        }
    }

    public class AClass
    {
        public EndPointCollection EndPoints { get; init; } = new EndPointCollection();

        public bool Property { get; set; } = false;
    }

    public sealed class EndPointCollection : Collection<EndPoint>, IEnumerable<EndPoint>
    {
        public EndPointCollection() { }

        public void Add(string hostAndPort)
        {
            EndPoint? endpoint;

            if (IPAddress.TryParse(hostAndPort, out IPAddress? address))
            {
                endpoint = new IPEndPoint(address, 0);
            }
            else
            {
                endpoint = new DnsEndPoint(hostAndPort, 0);
            }

            Add(endpoint);
        }
    }
}

.NET SDK:
 Version:   8.0.100-rc.2.23429.6
 Commit:    4bafa2271a

Needs servicing request for RC-2 backport.

I was trying out the fix here and noted a different scenario we might want to consider: #92137

Fixed in #92118

Navigation Menu

Repository navigation

Config generator wrongly emits init-related exception in bind operations #90974

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Config generator wrongly emits init-related exception in bind operations #90974

Description

Activity

davidfowl commented on Aug 23, 2023

stephentoub commented on Aug 23, 2023

davidfowl commented on Aug 23, 2023

stephentoub commented on Aug 23, 2023

davidfowl commented on Aug 23, 2023

layomia commented on Aug 23, 2023

layomia commented on Sep 1, 2023

layomia commented on Sep 12, 2023

ericstj commented on Sep 15, 2023

ericstj commented on Sep 20, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions