[API Proposal]: Expose the NullabilityInfoContext.IsSupported property

[eiriktsarpalis]

Background and motivation

NullabilityInfoContext.IsSupported is an internal property pointing to the System.Reflection.NullabilityInfoContext.IsSupported feature switch switch that additionally defines a corresponding ILLink substitution. If explicitly turned off, this causes the NullabilityInfoContext class to throw exceptions, which can create problems with third-party components that depend on NullabilityInfoContext. This is particularly problematic in SDKs that turn off this feature by default (such as Blazor wasm).

STJ attempted to address this problem by manually reading the feature switch, however this effort was impeded by the fact that feature switches cannot currently be read at run-time by Blazor wasm. Instead, Blazor wasm relies on ILLink substitutions for this configuration to be read. We should make this property public so that third-party applications can determine if they can use NullabilityInfoContext.

cc @javiercn

API Proposal

namespace System.Reflection;

public class NullabilityInfoContext
{
+    public static bool IsSupported { get; } = AppContext.TryGetSwitch("System.Reflection.NullabilityInfoContext.IsSupported", out bool isSupported) ? isSupported : true;
}

API Usage

NullabilityInfo? info = NullabilityInfoContext.IsSupported
    ? new NullabilityInfoContext().Create(propertyInfo)
    : null;

Alternative Designs

The implementation of NullabilityInfoContext could be updated so that exceptions are no longer being thrown.

Risks

No response

[dotnet-policy-service]

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

[eiriktsarpalis]

cc @Buyaa @steveharter

[MichalStrehovsky]

We don't document anything around this. The NullabilityInfoContextSupport property has no hits on learn.microsoft.com.

I think the whole existence of NullabilityInfoContextSupport should be reconsidered given this is now part of JSON serialization scenarios. This looked like a cheap optimization to remove things that nobody uses, but JSON serialization is a pretty core scenario.

Cc @eerhardt

[halter73]

Alternative Designs

The implementation of NullabilityInfoContext could be updated so that exceptions are no longer being thrown.

Would this always return NullablityState.Unknown for reference types in the cases it would have thrown previously? Is there any real downside to this? It seems fair to say we don't know the nullability similar to when a project was compiled without <Nullable>enable</Nullable>. If we continue to throw from NullabilityInfoContext.Create, I think we have no choice but to add this API.

I think the whole existence of NullabilityInfoContextSupport should be reconsidered given this is now part of JSON serialization scenarios. This looked like a cheap optimization to remove things that nobody uses, but JSON serialization is a pretty core scenario.

The S.T.J source generator still work even without NullabilityInfoContextSupport, right? I suspect the kind of apps that would set this to false outside of Blazor are also the kind likely to use the source generator to trim JSON properties better. There are at least a few open-source projects already doing this according to grep.app and that doesn't even catch our usage in BlazorWebAssembly or account for closed-source usage.

And out of curiosity, given that this is a trimming option that utilizes ILLink substitution, is there a realistic scenario where an application built against an older SDK that did support NullabilityInfoContextSupport property would use a new S.T.J that shipped after it was removed making this API helpful after we've removed support anyway?

[MichalStrehovsky]

I'll add a bit background. In #39000 we started to unconditionally trim all custom attributes that looked "unnecessary" on WASM. This included all the nullable attributes. This was an illegal optimization because it caused a behavioral difference that is only observable after trimming, without raising a publish-time warning (we don't annotate custom attribute APIs as trimming-unfriendly and make no effort to analyze what attribute types are reflected on). The justification at that time was that "few people will be broken".

Soon after that, NullabilityInfoContext APIs were added and they got broken by this. So a feature switch was introduced and removal of the nullable custom attribute subset was conditioned on this. A feature switch is a pair of two things: instructions to remove a thing during trimming + an AppContext switch we set and check for at runtime, both with and without trimming. This kind of works around some of the illegal optimization concerns since NullabilityInfoContext APIs will throw both with and without trimming if the feature switch is set. We still have a silent behavioral difference if one looks for these custom attributes using reflection APIs, but "few people will be broken".

Now System.Text.Json started using NullabilityInfoContext APIs. Nobody really realized that this is going to be problematic for trimming because NullabilityInfoContext APIs are not annotated as trim unsafe. They are not annotated as such because they throw with or without trimming if PublishAot or PublishTrimmed is set in the project. The System.Text.Json project obviously doesn't set that, it's a library. Because this interaction between the feature switch and S.T.Json was detected very late, it caused some stress after Preview 5 snap. I want to note that the stress after Preview 5 snap is a representative experience of feature switches in general - this is what we inflict on our customers every time we introduce a feature switch. It is exactly the kind of behavior we wanted to avoid in the new trimming design in .NET 5 that either has expected behaviors after trimming, or raises warnings.

I think we're at a fork where we can do one of these:

• Double down on the feature switch, add the IsSupported property. It doesn't really fix the trap that we introduced with the feature switch, but allows other components deal with it.
• Step back and reconsider. NullabilityInfoContext support is about a 0.5% size saving with PublishTrimmed on an app I tried. It's about a 0.1% saving with PublishAot. Do we care about that? If we do, this attribute stripping optimization can still be done transparently without any feature switches. The approach was sketched out in Attribute stripping #103934. It would not save the full 0.5%, but probably something close to that - the fix is rather simple: only drop the attributes on things that are not visible targets of reflection. That approach will never generate a behavioral difference unless there was a trimming warning and nobody (NullabilityInfoContext APIs, S.T.Json) needs to be aware that such optimization is even happening.

Cc @dotnet/illink-contrib

[eerhardt]

I'd be OK with removing the feature switch all together if the affected app model teams agree. I believe Blazor WASM is the only one using the switch.

In my measuring of a .NET 8 Blazor WASM app, it adds 0.76% of size to the brotli compressed app:

1. dotnet new blazorwasm
2. dotnet publish
   • With <NullabilityInfoContextSupport>false</NullabilityInfoContextSupport> (the default for Blazor WASM)
     • 2.56 MB (2,691,072 bytes)
   • With <NullabilityInfoContextSupport>true</NullabilityInfoContextSupport>
     • 2.58 MB (2,711,552 bytes)

cc @lewing @SteveSandersonMS @javiercn @marek-safar

[jkotas]

Another problem with the existing NullabilityInfoContextSupport. switch and proposed IsSupported property is that NullabilityInfoContext is designed to work for any System.Type, not just runtime System.Types. For example, it is meant to work
with types from System.Reflection.MetadataLoadContext that are not coupled to the runtime trimming policy and the IsSupported property does not make sense for them.

I agree that it would be best to delete the existing NullabilityInfoContextSupport. switch and look into better attribute stripping instead to mitigate the size growth.

[eiriktsarpalis]

Do we need API review to remove a feature switch? Seems we might as well just yank it.

[eiriktsarpalis]

Another question is how we should be handling older runtimes that include NullabilityInfoContext. STJ version 9.0.0 will also support net8.0 which has the feature switch. Is it going to be possible for Blazor wasm apps running .NET 8 to reference the v9 System.Text.Json NuGet package?

[jkotas]

Is it going to be possible for Blazor wasm apps running .NET 8 to reference the v9 System.Text.Json NuGet package?

It is possible. You can do try/catch around NullabilityInfoContext in the down-level packages and rethrow the exception with a better error message that says the new package System.Text.Json is not compatible with NullabilityInfoContextSupport disabled and tell people how to enable it.

[lewing]

I'd be OK with removing the feature switch all together if the affected app model teams agree. I believe Blazor WASM is the only one using the switch.

In my measuring of a .NET 8 Blazor WASM app, it adds 0.76% of size to the brotli compressed app:

1. dotnet new blazorwasm

2. dotnet publish

   • With <NullabilityInfoContextSupport>false</NullabilityInfoContextSupport> (the default for Blazor WASM)

     • 2.56 MB (2,691,072 bytes)

   • With <NullabilityInfoContextSupport>true</NullabilityInfoContextSupport>

     • 2.58 MB (2,711,552 bytes)

cc @lewing @SteveSandersonMS @javiercn @marek-safar

It is also used in mobile https://github.com/search?q=org%3Axamarin%20NullabilityInfoContextSupport&type=code but I don't think that changes the arguments for or against materially.

[lewing]

STJ attempted to address this problem by manually reading the feature switch, however this effort was impeded by the fact that feature switches cannot currently be read at run-time by Blazor wasm. Instead, Blazor wasm relies on ILLink substitutions for this configuration to be read. We should make this property public so that third-party applications can determine if they can use NullabilityInfoContext.

Manually reading the feature switch is actually supposed to work on wasm too, but we haven't had time to diagnose why the required build step isn't working in blazor at the moment and it wasn't noticed because the substitution was still working.

[terrajobst]

Video

• We think the original sin was to support attribute-level trimming for nullability info as the trimmer doesn't do that for any other attributes.
• We should remove the AppContext switch and not expose this property.

namespace System.Reflection;

public partial class NullabilityInfoContext
{
    public static bool IsSupported { get; } = AppContext.TryGetSwitch("System.Reflection.NullabilityInfoContext.IsSupported", out bool isSupported) ? isSupported : true;
}