Config generator wrongly emits init-related exception in bind operations

[stephentoub]

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.");
}

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

Issue Details

---

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.");
}

Author:	stephentoub
Assignees:	-
Labels:	untriaged, area-Extensions-Configuration
Milestone:	-

[davidfowl]

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 { }

[stephentoub]

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.

[davidfowl]

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?

[stephentoub]

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.

[davidfowl]

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

[layomia]

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