search

dotnet / runtime

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

[System.Text.Json] Consider removing `RequiresUnreferenceCode`/`RequiresDynamicCode` annotations from certain `JsonSerializer` APIs #108781

Open eiriktsarpalis opened 2 hours ago

eiriktsarpalis commented 2 hours ago

Today all JsonSerializer methods accepting JsonSerializerOptions are marked as RequiresUnreferencedCode/RequiresDynamicCode on account of their behavior in a couple of cases:

  1. If no JsonSerializerOptions is specified, it will be defaulted to JsonSerializerOptions.Default.
  2. If a JsonSerializerOptions is specified that doesn't explicitly set a TypeInfoResolver, then that field will be silently populated using JsonSerializerOptions.Default.TypeInfoResolver.

The pervasiveness of the annotations in some of the most commonly used APIs in STJ is creating substantial usability issues for library and Native AOT application authors, who often have to come up with elaborate workarounds to evade warnings.

What appears to be true for both cases is that code paths accessing unreferenced/dynamic code are guarded by the JsonSerializer.IsRefectionEnabledByDefault feature switch:

  1. Here's the JsonSerializerOptions.Default constructor and
  2. here is the TypeInfoResolver populating behavior.

It seems to me that this protection should suffice to remove the annotation; the IsRefectionEnabledByDefault feature switch is turned off by default for every application setting PublishTrimmed to true. Granted, a trimmed/Native AOT application could explicitly turn on IsRefectionEnabledByDefault but this is usually a case of misconfiguration. In any case, this is the tactic that has been employed by ASP.NET core since .NET 8 and one that just got rolled out in Microsoft.Extensions.AI so it makes sense that this a tried approach that we could try moving down the stack.

eiriktsarpalis commented 2 hours ago

cc @eerhardt @stephentoub @sbomer

dotnet-policy-service[bot] commented 2 hours ago

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

eiriktsarpalis commented 1 hour ago

Here's a draft of the proposed change: https://github.com/dotnet/runtime/pull/108782

Adding just three suppressions removed the bulk of the RUC/RDC annotations from the STJ API surface.