Runtime: Fail null deserialization on non-nullable reference types

[bwanner]

When deserializing "null" for a non-nullable value type System.Text.Json throws an Exception. However, for reference types this is not the case even if the project has the "Nullable" feature enabled.

Expectation: When nullable is enabled for the project deserializing null into a non-nullable type should fail for both value and reference types.

Question: This could be worked around by writing a custom Converter that honors nullable during deserialization. Are there plans to expose a setting to control the deserialization behavior for non-nullable reference types?

Repro: Deserializing DateTime as null fails (by design per this issue 40922). Deserializing List as null succeeds.

    class Program
    {
        public class MyDataObject
        {
            public DateTime Dt { get; set; }
            public List<int> MyNonNullableList { get; set; } = new List<int>();
        }

        static void Main(string[] args)
        {
            string input1 = "{\"Dt\":null, \"MyNonNullableList\":[42,32]}";
            string invalidInput = "{\"Dt\":\"0001-01-01T00:00:00\",\"MyNonNullableList\":null}";
            // Throws System.Text.Json.JsonException: 'The JSON value could not be converted to System.DateTime. Path: $.Dt | LineNumber: 0 | BytePositionInLine: 10.'
            // var validConverted = JsonSerializer.Deserialize<MyDataObject>(input1);

            // Does not throw Exception, but MyNonNullableList is null.
            var invalidConverted = JsonSerializer.Deserialize<MyDataObject>(invalidInput);
            Console.ReadKey();
        }
    }

[markm77]

I fully agree. Also for consistency shouldn't the absence of a non-nullable also cause a fail?

(If we don't have something like this the whole advantage of nullable gets significantly negated as runtime types can deviate from compile-time checks. We're then back to manual fixes (run-time checks, custom converters or even attributes - all subject to human error and forgotten updates etc...).)

[ahsonkhan]

Expectation: When nullable is enabled for the project deserializing null into a non-nullable type should fail for both value and reference types.

I believe "nullable reference type" is a compile-time/static analysis feature. I don't think we can leverage it to affect runtime behavior of APIs like the JsonSerializer (or any other API in the framework). This is especially true because the nullable is opt-in, and the API runtime behavior should be the same regardless of whether the nullability analysis is enabled or not. Maybe I am mistaken, but I don't see how that's feasible.

The nullable annotation on the JsonSerializer.Deserialize method expresses this intent (return: MaybeNull), which is that it may return null even if the type is marked as non-nullable. That's because the JSON payload passed in could be the "null" literal and I don't think there is a runtime check we can leverage to detect that the user has a non-nullable reference type that they want to deserialize the unknown JSON string into.
https://github.com/dotnet/runtime/blob/3a844ad951fda36fe3b570cc3af10738d898a7b3/src/libraries/System.Text.Json/ref/System.Text.Json.cs#L439-L440

What enabling the nullability will do is the compiler will warn you if you try to deserialize into a non-nullable reference type like your class.

cc @safern, @stephentoub, @jcouv

Are there plans to expose a setting to control the deserialization behavior for non-nullable reference types?

Aside from implementing your own JsonConverter<T>, we could consider extending the behavior of [JsonIgnore] and the IgnoreNullValue option to apply at a property level, in which case, the null JSON value will be ignored and not override the initialized non-nullable reference type's value (in your case, list). We can consider this requirement as part of these two related feature: https://github.com/dotnet/runtime/issues/30687 / https://github.com/dotnet/runtime/issues/29861

cc @steveharter, @layomia

[safern]

Yes I also agree that the nullable annotation for Deserialize is correct as the payload may contain nulls no matter what the nullable reference type is when calling the deserialize method. Unfortunately even if IgnoreNullValue is honored to ignore null we don't have a way to express that via nullable reference types. We can't tell the compiler, if this property is true, the return value won't be null.

So that's why the compiler will warn you, this method my return null and you're using a non-nullable type. So what you got to do is declare your generic parameter and variable type as nullable.

MyDataObject? invalidConverted = JsonSerializer.Deserialize<MyDataObject?>(invalidInput);

[Suchiman]

@ahsonkhan EF Core is actually doing this, when you have NRTs enabled, they'll make columns NULL / NOT NULL based on how your model class is annotated, see https://docs.microsoft.com/en-us/ef/core/miscellaneous/nullable-reference-types

[markm77]

So @ahsonkhan if I understand correctly you are suggesting a property attribute could be used to ensure NULL is ignored when de-serialising a non-nullable reference type. But don't we want a way to trigger an exception to be equivalent to the non-nullable value type case?

And if we have an attribute or way to trigger an exception with NULL would that not also solve the compiler warning issue mentioned by @safern in that the compiler flow analysis can see that an incoming NULL will lead to an exception and hence the conversion to non-nullable is safe?

I think it would be really great to have a solution that is compatible with the compile-time checker if possible. 😀

[markm77]

A couple of additional thoughts meant fully in the interests of making C# and .NET Core the best development environment possible.

Unfortunately even if IgnoreNullValue is honored to ignore null we don't have a way to express that via nullable reference types. We can't tell the compiler, if this property is true, the return value won't be null.

If the NRT complier flow analysis is not detecting things correctly, is it worth raising this with the compiler team? For example if the complier flow analysis is not smart about attributes shouldn't this be fixed?

So what you got to do is declare your generic parameter and variable type as nullable.

I realise it may take work from multiple teams but shouldn't we aim for a better solution than disabling the complier flow analysis (System.Diagnostics.CodeAnalysis.MaybeNull) and effectively not supporting NRT?

[Trolldemorted]

Any chance a fix for this is coming with .net 5?