dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.45k stars 4.76k forks source link

ConfigurationBinder Source Generator is suppressing IL2026 and IL3050 diagnostics even when it doesn't actually intercept the call #96643

Open eerhardt opened 10 months ago

eerhardt commented 10 months ago

There are certain cases where the ConfigurationBinder Source Generator won't intercept a call to Bind, for example if the Type is an unsupported type. When this is the case, the Source Generator is still suppressing the IL2026 and IL3050 diagnostics even though it didn't intercept the call. This can lead to the developer not getting notified that their code isn't trim/AOT compatible.

Repro steps

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

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <IsAotCompatible>true</IsAotCompatible>
    <EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="8.0.0" />
  </ItemGroup>

</Project>
using Microsoft.Extensions.Configuration;
using System.Collections;

namespace ClassLibrary7;

public class Class1
{
    public static void Bind(IConfiguration config)
    {
        var supported = new SupportedType();
        config.Bind(supported);

        var unsupported = new UnsupportedType();
        config.Bind(unsupported);
    }
}

public class SupportedType
{
    public string? Name { get; set; }
}

public class UnsupportedType : IEnumerable<KeyValuePair<string, string>>
{
    public IEnumerator<KeyValuePair<string, string>> GetEnumerator()
    {
        yield break;
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        throw new NotImplementedException();
    }
}

Expected results

Should still get AOT warnings for the call to Bind(unsupported), since this call wasn't intercepted by the Source Generator. At runtime it will follow the reflection based implementation and not use a source generated version.

Actual results

No warnings are emitted for IL2026 and IL3050.

You do get SYSLIB1100 diagnostic to tell you that UnsupportedType is unsupported. But this warning is often disabled/NoWarn'd because a lot of times there is just a single property not supported on a Type, and you just want the property to be ignored. Getting this diagnostic isn't enough to turn off the AOT compatibility warning that should be emitted here.

ghost commented 10 months ago

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

Issue Details
There are certain cases where the ConfigurationBinder Source Generator won't intercept a call to `Bind`, for example if the Type is an unsupported type. When this is the case, the Source Generator is still suppressing the IL2026 and IL3050 diagnostics even though it didn't intercept the call. This can lead to the developer not getting notified that their code isn't trim/AOT compatible. ### Repro steps ```xml net8.0 enable enable true true ``` ```C# using Microsoft.Extensions.Configuration; using System.Collections; namespace ClassLibrary7; public class Class1 { public static void Bind(IConfiguration config) { var supported = new SupportedType(); config.Bind(supported); var unsupported = new UnsupportedType(); config.Bind(unsupported); } } public class SupportedType { public string? Name { get; set; } } public class UnsupportedType : IEnumerable> { public IEnumerator> GetEnumerator() { yield break; } IEnumerator IEnumerable.GetEnumerator() { throw new NotImplementedException(); } } ``` ### Expected results Should still get AOT warnings for the call to `Bind(unsupported)`, since this call wasn't intercepted by the Source Generator. At runtime it will follow the reflection based implementation and not use a source generated version. ### Actual results No warnings are emitted for IL2026 and IL3050. You do get `SYSLIB1100` diagnostic to tell you that `UnsupportedType` is unsupported. But this warning is often disabled/NoWarn'd because a lot of times there is just a single property not supported on a Type, and you just want the property to be ignored. Getting this diagnostic isn't enough to turn off the AOT compatibility warning that should be emitted here.
Author: eerhardt
Assignees: -
Labels: `area-Extensions-Configuration`
Milestone: -
ericstj commented 10 months ago

Bug is in the suppressor. It shouldn't be suppressing diagnostics if it didn't intercept the code.
https://github.com/dotnet/runtime/blob/6dab58f0213ec67157bdbd09630957e7cc3b1027/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Suppressor.cs#L61C38-L61C38