Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiler error when using configuration binder source generator with custom collections #91324

Closed
adamsitnik opened this issue Aug 30, 2023 · 7 comments

Comments

@adamsitnik
Copy link
Member

adamsitnik commented Aug 30, 2023

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:

<?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

cc @layomia @eerhardt

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 30, 2023
@ghost
Copy link

ghost commented Aug 30, 2023

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

Issue Details

I've tried to use the new configuration binder source generator to bind a type that contains a custom collection of complex 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:

<?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

cc @layomia @eerhardt

Author: adamsitnik
Assignees: -
Labels:

area-Extensions-Configuration

Milestone: -

@layomia
Copy link
Contributor

layomia commented Aug 30, 2023

Thanks, taking a look.

@layomia
Copy link
Contributor

layomia commented Aug 30, 2023

I was able to repro; it's related to #90974. Working on a fix.

@layomia layomia self-assigned this Aug 30, 2023
@layomia layomia added this to the 8.0.0 milestone Aug 30, 2023
@layomia layomia added blocking-release and removed untriaged New issue has not been triaged by the area owner labels Aug 30, 2023
@layomia
Copy link
Contributor

layomia commented Sep 1, 2023

Issues isn't really about custom collections. Will address in #90974 & test the scenario.

@ericstj
Copy link
Member

ericstj commented Sep 15, 2023

Reactivating since we're backporting this to release/8.0.

I confirmed that with the latest from main this issue is fixed. We'll no longer produce bad code when generating for members that have lists of abstract types.

We will, however, skip binding to this member since we cannot create the abstract types. That does seem consistent with what reflection the binder does, and appears to be what you expected based on the repro manually handling the endpoint values.

@adamsitnik
Copy link
Member Author

I confirmed that with the latest from main this issue is fixed.

Great!

That does seem consistent with what reflection the binder does, and appears to be what you expected based on the repro manually handling the endpoint values.

Do we have any plans to be able to hint the binders on how to deal with abstract types? Similarly to what serializers do?
Or perhaps this is an anti-pattern and we should discourage the users from doing that?

@ericstj
Copy link
Member

ericstj commented Sep 19, 2023

We have an issue tracking user-provided extensibility. #83599
In this case, since you don't own the type it would fall into the registration scenario. I'll add your scenario to that issue.
Closing this, since the fix was merged to RC2 with #92118

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants