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

ConfigurationBinder source generator generates code with the wrong nullable annotations #94105

Open
eerhardt opened this issue Oct 27, 2023 · 10 comments
Labels
area-Extensions-Configuration bug source-generator Indicates an issue with a source generator feature
Milestone

Comments

@eerhardt
Copy link
Member

Description

When attempting to use the ConfigurationBinder source generator in dotnet/extensions (see dotnet/extensions#4625), I'm getting a nullable warning that is causing the generated code to not compile (I would need to disable nullable validation for the whole project if I wanted to ignore the nullable warning, which isn't acceptable).

I'm not sure what the general pattern is here that causes this issue, so I logged the issue for the scenario specifically. I think it might need to include nested generics where one of the nested generic types is nullable.

Reproduction Steps

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

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator>
    <NoWarn>$(NoWarn);SYSLIB1100;SYSLIB1101</NoWarn>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="8.0.0-rtm.23511.16" />
    <PackageReference Include="Microsoft.Extensions.Http.Resilience" Version="8.0.0-rc.2.23510.2" />
  </ItemGroup>

</Project>
// needed to work around https://github.com/dotnet/runtime/issues/94065
global using Polly;
global using Polly.Hedging;
global using Polly.RateLimiting;
global using Polly.Timeout;

using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Http.Resilience;

var c = new ConfigurationBuilder().Build();
c.Get<HttpStandardHedgingResilienceOptions>();

Expected behavior

Should be without warnings

Actual behavior

Get a nullable warning from generated code:

CS8619	Nullability of reference types in value of type 'Func<HedgingActionGeneratorArguments<HttpResponseMessage>, Func<ValueTask<Outcome<HttpResponseMessage>>>?>' doesn't match target type 'Func<HedgingActionGeneratorArguments<HttpResponseMessage>, Func<ValueTask<Outcome<HttpResponseMessage>>>>'.	ConsoleApp101	C:\Users\eerhardt\source\repos\ConsoleApp101\ConsoleApp101\Microsoft.Extensions.Configuration.Binder.SourceGeneration\Microsoft.Extensions.Configuration.Binder.SourceGeneration.ConfigurationBindingGenerator\BindingExtensions.g.cs	1418		

The generated code looks like:

            if (AsConfigWithChildren(configuration.GetSection("ActionGenerator")) is IConfigurationSection section256)
            {
                Func<HedgingActionGeneratorArguments<HttpResponseMessage>, Func<ValueTask<Outcome<HttpResponseMessage>>>>? temp258 = instance.ActionGenerator;
                if (temp258 is not null)
                {

That middle line is the issue, it is of type:

Func<HedgingActionGeneratorArguments<HttpResponseMessage>, Func<ValueTask<Outcome<HttpResponseMessage>>>>
but the type of instance.ActionGenerator is:
Func<HedgingActionGeneratorArguments<TResult>, Func<ValueTask<Outcome<TResult>>>?>

Note the ? inside the last > brace.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

cc @tarekgh @ericstj

@ghost
Copy link

ghost commented Oct 27, 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

When attempting to use the ConfigurationBinder source generator in dotnet/extensions (see dotnet/extensions#4625), I'm getting a nullable warning that is causing the generated code to not compile (I would need to disable nullable validation for the whole project if I wanted to ignore the nullable warning, which isn't acceptable).

I'm not sure what the general pattern is here that causes this issue, so I logged the issue for the scenario specifically. I think it might need to include nested generics where one of the nested generic types is nullable.

Reproduction Steps

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

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator>
    <NoWarn>$(NoWarn);SYSLIB1100;SYSLIB1101</NoWarn>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="8.0.0-rtm.23511.16" />
    <PackageReference Include="Microsoft.Extensions.Http.Resilience" Version="8.0.0-rc.2.23510.2" />
  </ItemGroup>

</Project>
// needed to work around https://github.com/dotnet/runtime/issues/94065
global using Polly;
global using Polly.Hedging;
global using Polly.RateLimiting;
global using Polly.Timeout;

using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Http.Resilience;

var c = new ConfigurationBuilder().Build();
c.Get<HttpStandardHedgingResilienceOptions>();

Expected behavior

Should be without warnings

Actual behavior

Get a nullable warning from generated code:

CS8619	Nullability of reference types in value of type 'Func<HedgingActionGeneratorArguments<HttpResponseMessage>, Func<ValueTask<Outcome<HttpResponseMessage>>>?>' doesn't match target type 'Func<HedgingActionGeneratorArguments<HttpResponseMessage>, Func<ValueTask<Outcome<HttpResponseMessage>>>>'.	ConsoleApp101	C:\Users\eerhardt\source\repos\ConsoleApp101\ConsoleApp101\Microsoft.Extensions.Configuration.Binder.SourceGeneration\Microsoft.Extensions.Configuration.Binder.SourceGeneration.ConfigurationBindingGenerator\BindingExtensions.g.cs	1418		

The generated code looks like:

            if (AsConfigWithChildren(configuration.GetSection("ActionGenerator")) is IConfigurationSection section256)
            {
                Func<HedgingActionGeneratorArguments<HttpResponseMessage>, Func<ValueTask<Outcome<HttpResponseMessage>>>>? temp258 = instance.ActionGenerator;
                if (temp258 is not null)
                {

That middle line is the issue, it is of type:

Func<HedgingActionGeneratorArguments<HttpResponseMessage>, Func<ValueTask<Outcome<HttpResponseMessage>>>>
but the type of instance.ActionGenerator is:
Func<HedgingActionGeneratorArguments<TResult>, Func<ValueTask<Outcome<TResult>>>?>

Note the ? inside the last > brace.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

cc @tarekgh @ericstj

Author: eerhardt
Assignees: -
Labels:

area-Extensions-Configuration

Milestone: -

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 27, 2023
@eiriktsarpalis
Copy link
Member

That's surprising, doesn't the generated code suppress nullability warnings in source?

@eerhardt
Copy link
Member Author

That's surprising, doesn't the generated code suppress nullability warnings in source?

No, it explicitly enables it:

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

@eiriktsarpalis
Copy link
Member

That seems surprising given that this was picked up from the JSON generator:

writer.WriteLine("""
// <auto-generated/>
#nullable enable annotations
#nullable disable warnings
// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0612, CS0618
""");

I think we should consistently disable nullability warnings in all generated code (except perhaps in debug builds of the generator?)

@eiriktsarpalis eiriktsarpalis added this to the 8.0.x milestone Nov 1, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Nov 1, 2023
@eiriktsarpalis eiriktsarpalis added bug untriaged New issue has not been triaged by the area owner labels Nov 1, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Nov 1, 2023
@eiriktsarpalis eiriktsarpalis self-assigned this Nov 1, 2023
@eiriktsarpalis
Copy link
Member

@eerhardt I'm preparing a PR that disables nullability warnings in generated code. I'm not super familiar with this library though, do you know if there's a minimal repro I can use that doesn't require the dependency on Polly?

@eerhardt
Copy link
Member Author

eerhardt commented Nov 1, 2023

Unfortunately, no I don't. I haven't had time to pare it down.

I would try making a type of Func<HedgingActionGeneratorArguments<TResult>, Func<ValueTask<Outcome<TResult>>>?>

where HedgingActionGeneratorArguments and Outcome are generic types.

@eiriktsarpalis
Copy link
Member

Is that warning a side-effect of the generator erroneously generating binding code for delegate types?

@eerhardt
Copy link
Member Author

eerhardt commented Nov 1, 2023

Yes, but I believe the issue will also occur if you replace the Func with some other generic types.

@eiriktsarpalis
Copy link
Member

This was addressed in .NET 8 via #94267 by disabling nullability warnings altogether. Moving to Future for a fix that addresses the specific annotation issue.

@eiriktsarpalis
Copy link
Member

Moving to 10.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Extensions-Configuration bug source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

No branches or pull requests

3 participants