[S.T.Json] EnumConverterFactory IL3050 when NativeAOT publish with `JsonSerializerIsReflectionEnabledByDefault`=true

[jkurdek]

Description:

When publishing a .NET application using System.Text.Json with NativeAOT, setting JsonSerializerIsReflectionEnabledByDefault to true triggers the following warning during compilation:

/_/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverterFactory.cs(33): 
AOT analysis warning IL3050: System.Text.Json.Serialization.Converters.EnumConverterFactory.CreateConverter(Type,JsonSerializerOptions): 
Using member 'System.Text.Json.Serialization.Converters.EnumConverterFactory.Create(Type,EnumConverterOptions,JsonNamingPolicy,JsonSerializerOptions)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. 
JSON serialization and deserialization might require types that cannot be statically analyzed and might need runtime code generation. 
Use System.Text.Json source generation for native AOT applications.

Repro:

using System.Text.Json;

class Program
{
    static void Main()
    {
        var something = JsonSerializer.Deserialize<String>("{}"); // Any call to JsonSerializer.Deserialize will do
    }
}

Enable AOT and JsonSerializerIsReflectionEnabledByDefault

<PublishAot>true</PublishAot> 
<JsonSerializerIsReflectionEnabledByDefault>true</JsonSerializerIsReflectionEnabledByDefault>

dotnet publish -c Release -p:TrimmerSingleWarn=false

Possible cause:

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverterFactory.cs

Lines 28 to 34 in 9ecef8c

	[SuppressMessage("AOT", "IL3050:Calling members annotated with 'RequiresDynamicCodeAttribute' may break functionality when AOT compiling.",
	Justification = "The constructor has been annotated with RequiredDynamicCodeAttribute.")]
	public override JsonConverter CreateConverter(Type type, JsonSerializerOptions options)
	{
	Debug.Assert(CanConvert(type));
	return Create(type, EnumConverterOptions.AllowNumbers, namingPolicy: null, options);
	}

should be an UnconditionalSuppressMessage ?

Context:

The issue was found in MAUI - dotnet/maui#25077

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

[eiriktsarpalis]

should be an UnconditionalSuppressMessage ?

Yes! Could you submit a PR please?

[vitek-karas]

Note that this is a regression introduced in 9.0 - so we should consider fixing this in servicing as well. (The risk of the fix is effectively 0 if it's just that attribute change - so should be relatively easy to push through).

/fyi @sbomer - this was introduced in #105032. This is a typical example of the "blame the constructor" approach (which we suggested for this) - the virtual method implementation needs RDC, but we can't add it on the base, so we "blame the constructor". Before #105032 this was done by adding RDC on the class itself - which works nicely, but works well only on classes without statics. #105032 added a static and so it removed RDC on the class and moved it onto the .ctor directly. But that means manually suppressing warnings, and that's where the bug was introduced.
The ideal solution would be to support RDC/RUC on class with a "instance only" flag - we've discussed this several times in the past, since it's the best solution we have so far for the problem of virtual method override requiring RDC/RUC.
This bug is basically another example of why we should add such capability as it's really easy to "blame the constructor" in a wrong way - and this specific case is even harder to detect, as it effectively requires having a sample app published as NativeAOT to see it (analyzers won't see it).

Additional idea - maybe we should add into trim analyzer detection of SuppressMessage over IL* warnings - as it's almost always the wrong thing to do - and there's a better way to suppress analyzer only warnings via pragmas anyway. Not sure it's possible though (I know Roslyn does weird things with SuppressMessage).