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

Configuration binder source generator producing invalid code #92021

Closed
jamiewinder opened this issue Sep 13, 2023 · 7 comments
Closed

Configuration binder source generator producing invalid code #92021

jamiewinder opened this issue Sep 13, 2023 · 7 comments

Comments

@jamiewinder
Copy link

jamiewinder commented Sep 13, 2023

Description

In the new NET 8 RC1, even the most basic configuration binder source generator scenarios seem to produce invalid code.

Reproduction Steps

using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;

var configuration = new ConfigurationBuilder()
    .AddJsonFile("appsettings.json")
    .Build();

var services = new ServiceCollection();

services.Configure<A>(configuration.GetSection("MyOptions"));

Console.WriteLine("Hello, World!");

public class MyOptions 
{
    public string Text { get; set; } = string.Empty;
}

With the following csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <LangVersion>preview</LangVersion>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <PublishAot>true</PublishAot>
    <InvariantGlobalization>true</InvariantGlobalization>
    <EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator>
    <Features>InterceptorsPreview</Features>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Configuration" Version="8.0.0-rc.1.23419.4" />
    <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="8.0.0-rc.1.23419.4" />
    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0-rc.1.23419.4" />
    <PackageReference Include="Microsoft.Extensions.Options" Version="8.0.0-rc.1.23419.4" />
    <PackageReference Include="Microsoft.Extensions.Options.ConfigurationExtensions" Version="8.0.0-rc.1.23419.4" />
  </ItemGroup>

</Project>

Expected behavior

Should build.

Actual behavior

The source generator seems to generate invalid code, which produce the following errors:

  • The type or namespace name 'IOptionsChangeTokenSource<>' could not be found (are you missing a using directive or an assembly reference?) (line 55)
  • The type or namespace name 'ConfigurationChangeTokenSource<>' could not be found (are you missing a using directive or an assembly reference?) (line 55)
  • Syntax error, ',' expected (line 56)

Regression?

Worked in .NET 8 Preview 7

Known Workarounds

No response

Configuration

.NET 8.0.100-rc.1.23455.8

Other information

No response

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

ghost commented Sep 13, 2023

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

Issue Details

Description

In the new NET 8 RC1, even the most basic configuration binder source generator scenarios seem to produce invalid code.

Reproduction Steps

using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;

var configuration = new ConfigurationBuilder()
    .AddJsonFile("appsettings.json")
    .Build();

var services = new ServiceCollection();

services.Configure<A>(configuration.GetSection("MyOptions"));

Console.WriteLine("Hello, World!");

public class MyOptions 
{
    public string Text { get; set; } = string.Empty;
}

With the following csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <LangVersion>preview</LangVersion>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <PublishAot>true</PublishAot>
    <InvariantGlobalization>true</InvariantGlobalization>
	<EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator>
	<Features>InterceptorsPreview</Features>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Configuration" Version="8.0.0-rc.1.23419.4" />
    <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="8.0.0-rc.1.23419.4" />
    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0-rc.1.23419.4" />
    <PackageReference Include="Microsoft.Extensions.Options" Version="8.0.0-rc.1.23419.4" />
    <PackageReference Include="Microsoft.Extensions.Options.ConfigurationExtensions" Version="8.0.0-rc.1.23419.4" />
  </ItemGroup>

</Project>

Expected behavior

Should build.

Actual behavior

The source generator seems to generate invalid code, which produce the following errors:

  • The type or namespace name 'IOptionsChangeTokenSource<>' could not be found (are you missing a using directive or an assembly reference?) (line 55)
  • The type or namespace name 'ConfigurationChangeTokenSource<>' could not be found (are you missing a using directive or an assembly reference?) (line 55)
  • Syntax error, ',' expected (line 56)

Regression?

Worked in .NET 8 Preview 7

Known Workarounds

No response

Configuration

.NET 8.0.100-rc.1.23455.8

Other information

No response

Author: jamiewinder
Assignees: -
Labels:

area-Extensions-Configuration

Milestone: -

@jamiewinder
Copy link
Author

FYI, this is the source generated file:

// <auto-generated/>
#nullable enable
#pragma warning disable CS0612, CS0618 // Suppress warnings about [Obsolete] member usage in generated code.

namespace System.Runtime.CompilerServices
{
    using System;
    using System.CodeDom.Compiler;

    [GeneratedCode("Microsoft.Extensions.Configuration.Binder.SourceGeneration", "8.0.8.41904")]
    [AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
    file sealed class InterceptsLocationAttribute : Attribute
    {
        public InterceptsLocationAttribute(string filePath, int line, int column)
        {
        }
    }
}

namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
    using Microsoft.Extensions.Configuration;
    using Microsoft.Extensions.DependencyInjection;
    using System;
    using System.CodeDom.Compiler;
    using System.Collections.Generic;
    using System.Globalization;
    using System.Runtime.CompilerServices;

    [GeneratedCode("Microsoft.Extensions.Configuration.Binder.SourceGeneration", "8.0.8.41904")]
    file static class BindingExtensions
    {
        #region IServiceCollection extensions.
        /// <summary>Registers a configuration instance which TOptions will bind against.</summary>
        [InterceptsLocationAttribute(@"C:\Projects\platform\misc\DdbTest\ConsoleApp1\Program.cs", 10, 10)]
        public static IServiceCollection Configure<TOptions>(this IServiceCollection services, IConfiguration configuration) where TOptions : class
        {
            return Configure<TOptions>(services, string.Empty, configuration, configureOptions: null);
        }

        /// <summary>Registers a configuration instance which TOptions will bind against.</summary>
        public static IServiceCollection Configure<TOptions>(this IServiceCollection services, string? name, IConfiguration configuration, Action<BinderOptions>? configureOptions) where TOptions : class
        {
            if (services is null)
            {
                throw new ArgumentNullException(nameof(services));
            }

            if (configuration is null)
            {
                throw new ArgumentNullException(nameof(configuration));
            }

            OptionsServiceCollectionExtensions.AddOptions(services);
            services.AddSingleton<IOptionsChangeTokenSource<TOptions>>(new ConfigurationChangeTokenSource<TOptions>(name, configuration));
            return services.AddSingleton<Microsoft.Extensions.Options.IConfigureOptions<TOptions>>(new Microsoft.Extensions.Options.ConfigureNamedOptions<TOptions>(name, obj => BindCoreMain(configuration, obj, typeof(TOptions)configureOptions)));
        }
        #endregion IServiceCollection extensions.

        #region Core binding extensions.
        private readonly static Lazy<HashSet<string>> s_configKeys_MyOptions = new(() => new HashSet<string>(StringComparer.OrdinalIgnoreCase) { "Text" });

        public static void BindCoreMain(IConfiguration configuration, object obj, Type type, Action<BinderOptions>? configureOptions)
        {
            if (configuration is null)
            {
                throw new ArgumentNullException(nameof(configuration));
            }

            if (obj is null)
            {
                throw new ArgumentNullException(nameof(obj));
            }

            if (!HasValueOrChildren(configuration))
            {
                return;
            }

            BinderOptions? binderOptions = GetBinderOptions(configureOptions);

            if (type == typeof(MyOptions))
            {
                var temp = (MyOptions)obj;
                BindCore(configuration, ref temp, binderOptions);
                return;
            }

            throw new NotSupportedException($"Unable to bind to type '{type}': generator did not detect the type as input.");
        }

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

            obj.Text = configuration["Text"]!;
        }


        /// <summary>If required by the binder options, validates that there are no unknown keys in the input configuration object.</summary>
        public static void ValidateConfigurationKeys(Type type, Lazy<HashSet<string>> keys, IConfiguration configuration, BinderOptions? binderOptions)
        {
            if (binderOptions?.ErrorOnUnknownConfiguration is true)
            {
                List<string>? temp = null;
        
                foreach (IConfigurationSection section in configuration.GetChildren())
                {
                    if (!keys.Value.Contains(section.Key))
                    {
                        (temp ??= new List<string>()).Add($"'{section.Key}'");
                    }
                }
        
                if (temp is not null)
                {
                    throw new InvalidOperationException($"'ErrorOnUnknownConfiguration' was set on the provided BinderOptions, but the following properties were not found on the instance of {type}: {string.Join(", ", temp)}");
                }
            }
        }

        public static bool HasValueOrChildren(IConfiguration configuration)
        {
            if ((configuration as IConfigurationSection)?.Value is not null)
            {
                return true;
            }
            return AsConfigWithChildren(configuration) is not null;
        }

        public static IConfiguration? AsConfigWithChildren(IConfiguration configuration)
        {
            foreach (IConfigurationSection _ in configuration.GetChildren())
            {
                return configuration;
            }
            return null;
        }

        public static BinderOptions? GetBinderOptions(Action<BinderOptions>? configureOptions)
        {
            if (configureOptions is null)
            {
                return null;
            }
        
            BinderOptions binderOptions = new();
            configureOptions(binderOptions);
        
            if (binderOptions.BindNonPublicProperties)
            {
                throw new NotSupportedException($"The configuration binding source generator does not support 'BinderOptions.BindNonPublicProperties'.");
            }
        
            return binderOptions;
        }
        #endregion Core binding extensions.
    }
}

If I add this manually, I don't have the first two errors, just the missing comma error. This modification fixes it:

- return services.AddSingleton<Microsoft.Extensions.Options.IConfigureOptions<TOptions>>(new Microsoft.Extensions.Options.ConfigureNamedOptions<TOptions>(name, obj => BindCoreMain(configuration, obj, typeof(TOptions)configureOptions)));
+ return services.AddSingleton<Microsoft.Extensions.Options.IConfigureOptions<TOptions>>(new Microsoft.Extensions.Options.ConfigureNamedOptions<TOptions>(name, obj => BindCoreMain(configuration, obj, typeof(TOptions), configureOptions)));

@layomia
Copy link
Contributor

layomia commented Sep 13, 2023

This was a regression added in RC-2. Fixed for RC-2 in #91602. Please check if the latest RC-2 bits work for you - https://github.com/dotnet/installer#table.

@layomia layomia closed this as completed Sep 13, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Sep 13, 2023
@jamiewinder
Copy link
Author

jamiewinder commented Sep 14, 2023

@layomia I've tried in RC2 and the comma issue is sorted, but the other two errors remain.

The problem seems to be that IOptionsChangeTokenSource and ConfigurationChangeTokenSource aren't fully namespaced unlike the surrounding types, and the Microsoft.Extensions.Options using isn't declared.

services.AddSingleton<IOptionsChangeTokenSource<TOptions>>(new ConfigurationChangeTokenSource<TOptions>(name, configuration));
return services.AddSingleton<Microsoft.Extensions.Options.IConfigureOptions<TOptions>>(new Microsoft.Extensions.Options.ConfigureNamedOptions<TOptions>(name, obj => BindCoreMain(configuration, obj, typeof(TOptions), configureOptions)));

@layomia
Copy link
Contributor

layomia commented Sep 14, 2023

Which RC-2 version are you using?

@jamiewinder
Copy link
Author

@layomia This was 8.0.100-rc.2.23463.8. I'll try the new latest.

@jamiewinder
Copy link
Author

jamiewinder commented Sep 15, 2023

Seeing the same in 8.0.100-rc.2.23464.22, plus the comma issue seems to have re-materialised? (same example as above)

EDIT: Now resolved, though I had to add explicit references to the latest Microsoft.Extensions.Configuration.Json and Microsoft.Extensions.Options.ConfigurationExtensions?

@ghost ghost locked as resolved and limited conversation to collaborators Oct 15, 2023
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

2 participants