The C# required modifier should trigger Required.Always behaviour

[tobymiller]

In C# 11 (dotnet 7) the required modifier was introduced for fields and properties. It forces a caller to provide a value in, for example, an object initialiser. The compiler enforces this, and knows not to complain about non-nullable types without a default value, as a value must always be provided.

System.Text.Json throws an exception if a required field is omitted in source json. However, Newtonsoft does not, and silently fills this field with a default value, such as null. The desired behaviour can be achieved in Newtonsoft using JsonProperty(Required = Required.Always), but this is verbose and should be redundant in these cases.

I think it would be sensible default behaviour (or at least configurable behaviour) to respect required, as intuitively, this is what is expected.

Source/destination types

public class Thing
{
    public required string Field { get; init; }
}

Source JSON

{}

Expected behavior

An error to be thrown, as Field is required.

Actual behavior

Newtonsoft deserialises happily, with null being the value of Field, even though this is invalid as the field is non-nullable.

Steps to reproduce

public class Thing
{
    public required string Field { get; init; }
}

JsonSerializer.Deserialize<Thing>("{}");

[dsherret]

I hope this project doesn't make the same design mistake that dotnet made. The required modifier is a language feature that "indicates that the field or property it's applied to must be initialized by an object initializer" and that should be it. In my opinion, ensuring that a property appears in an object initializer and ensuring that a property is required for JSON deserialization are separate matters. It's extremely unfortunate that System.Text.Json has piggybacked on this keyword to give it additional meaning.

For example, you could have your type defined like so for the purpose of ensuring its set in object initializers:

public class Thing
{
    public required string? Field { get; init; }
}

...but System.Text.Json will now throw an error when Field doesn't exist as a property in the JSON even though deserializing to null is just fine in 99.99% of cases.

The main problem described in this issue is that a nullable value has been assigned to a non-nullable one and I think it would be nice to have better solutions around that instead. Then for marking a property as required for JSON deserialization, I believe it should be kept separate from the required modifier and use the separate [JsonRequired] attribute to say a property is required:

public class Thing
{
    [JsonRequired]                               // required for JSON deserialization
    public required string? Field { get; init; } // required in object initializers
}

[tobymiller]

I disagree. Default values are, in my view, a disaster of a language concept. C# is stuck with them for backward compatibility, but for the first time with the required keyword, we have a way to express to the type system that something must always have a meaningful value. The benefit of being able to rely on this without exception is huge, and can eliminate a large class of bugs. However, if there are common circumstances (such as when using this library), where the apparent guarantee of the required keyword is broken, then it significantly undermines this.

I don't know what the language designers had in mind for this keyword, but it seems to me far more useful if libraries such as this take the approach that I'm suggesting. It can always be optional configuration so that those like me that never want to see another number come through as 0 when it was actually misspelt can have our wish.

On the null issue, I don't really see the problem, because things can be explicitly set to null in json just as they can in an object initialiser. If they're missing, that usually indicates a problem, not a legitimate null value.

[dsherret]

If they're missing, that usually indicates a problem, not a legitimate null value.

It's very common for null values to be omitted in serialized JSON in order to use less data. If the required modifier is piggybacked on then it means there now need to be a way to mark something as required in object initializers (the required modifier's intent) and not required in the JSON.

for the first time with the required keyword, we have a way to express to the type system that something must always have a meaningful value.

I disagree, this keyword was intended to be a way to mark a property as required in an object initializer. Now libraries (like System.Text.Json) are making it have additional meanings. Nullable types should be used to help enforce a non-nullable value and not the required keyword.

Default values are, in my view, a disaster of a language concept.

The ideal behaviour for a C# deserialization library would be for a string property to error if not present or null and for string? to not error if not present or null. This would be a really big breaking change though (I'm also not sure about the performance cost of checking for the NullableAttribute and the docs specifically say libraries should not use it so... 🤷‍♂️).

Another point to note is that making the required modifier have this new meaning would also be a breaking change.

[ronnieoverby]

I hope this project doesn't make the same design mistake that dotnet made. The required modifier is a language feature that "indicates that the field or property it's applied to must be initialized by an object initializer" and that should be it. In my opinion, ensuring that a property appears in an object initializer and ensuring that a property is required for JSON deserialization are separate matters. It's extremely unfortunate that System.Text.Json has piggybacked on this keyword to give it additional meaning.

I absolutely 100% agree that System.Text.Json should not have given runtime significance to what is a essentially a compile time feature. I have excluded serializing default values many times as a conscientious design choice.

I don't know what the language designers had in mind for this keyword

Just read this and this. I believe it's very clear what the spirit and extent of the feature would be.

Recently I made use of the new required modifier on my classes properties, having studied the documentation for that new keyword and understanding that it has only to do that when using property initializers in the source code. I started to see very unexpected exceptions with System.Text.Json deserialization because of the use of required modifier on my types. Despite coming to what I thought was a solid grasp of a language concept, I was forced to deal with the the whims of library designers that gave extra, inconsistent meaning to it, only discoverable at runtime.

On the null issue, I don't really see the problem, because things can be explicitly set to null in JSON just as they can in an object initializer. If they're missing, that usually indicates a problem, not a legitimate null value.

The problem is backwards compatibility with stored JSON or JSON coming from an external system that we may have absolutely no control whatsoever.

Default values are, in my view, a disaster of a language concept.

I understand, but the safest way to deal with distaste of default values is to use a language where that concept is not baked into its foundations.

[QuintinWillison]

To @dsherret's:

but System.Text.Json will now throw an error when Field doesn't exist as a property in the JSON even though deserializing to null is just fine in 99.99% of cases

I could not agree with you more! 🤓 After having got perhaps a bit too giddy with required attribution a while back, I now find myself going through constructor-less record type definitions and iterating to strip the required modifiers away in order to work around downstream JSON deserialization issues when fancy serialization options are used (we don't use Newtonsoft any more, having migrated to System.Text.Json some time ago).

"required but optional (nullable)" makes no sense in the JSON context, especially when using JsonIgnoreCondition.WhenWritingNull (the aforementioned fancy feature) with your writers/encoders (i.e. you care about encoded payload size! 🤔).

[dsherret]

@QuintinWillison I found a way to disable it for System.Text.Json here by overriding the TypeInfoResolver. It's worked so far for me.

Code from link

var resolver = = new DefaultJsonTypeInfoResolver
{
  Modifiers =
  {
    static typeInfo =>
    {
      foreach (var info in typeInfo.Properties)
      {
        if (info.IsRequired)
        {
          info.IsRequired = info.AttributeProvider?.IsDefined(
            typeof(JsonRequiredAttribute),
            inherit: false
          ) ?? false;
        }
      }
    }
  }
};

var result = JsonSerializer.Deserialize<MyRecord>(
  "{}",
  new JsonSerializerOptions
  {
    TypeInfoResolver = resolver,
  }
);

// or for asp.net core
builder.Services.AddControllers().AddJsonOptions(options =>
{
    options.JsonSerializerOptions.TypeInfoResolver = resolver
});

[ronnieoverby]

@QuintinWillison @dsherret
Here's how I disabled the behavior:

    public static JsonSerializerOptions IgnoreCSharpRequiredModifiers(this JsonSerializerOptions options)
    {
        options.TypeInfoResolver ??= new DefaultJsonTypeInfoResolver();

        options.TypeInfoResolver =
            options.TypeInfoResolver.WithAddedModifier(static typeInfo => typeInfo.IgnoreCSharpRequiredModifiers());

        return options;
    }

    private static void IgnoreCSharpRequiredModifiers(this JsonTypeInfo info)
    {
        foreach (var p in info.Properties)
            p.IsRequired &= p.AttributeProvider.HasJsonRequiredAttribute();
    }
	
    private static bool HasJsonRequiredAttribute(this ICustomAttributeProvider? attributes) =>
        attributes is not null && attributes.IsDefined(typeof(JsonRequiredAttribute), inherit: false);

[StingyJack]

I disagree. Default values are, in my view, a disaster of a language concept. C# is stuck with them for backward compatibility, but for the first time with the required keyword, we have a way to express to the type system that something must always have a meaningful value.

@tobymiller - Its a bad assumption that the value will be meaningful and not just some unexpected/garbage. null is only one of the many unexpected/garbage values that can break a function. The entire class of bugs will still exist thanks to all those other non-null values.

Is this the default value disaster of which you speak?

pubic class MyClass
{
   public Dictionary<string, string> HumanDictionary {get; } = new(StringComparer.OrdinalIgnoreCase);
}

This seems like a win to me. NullReferenceExceptions avoided when someone calls HumanDictionary.Add(... and bugs due to casing differences ("dog", "Dog") are avoided thanks to default values.