System.Text.Json deserialisation does not respect "required" keyword when used in base class

[stedaho]

Description

When I use System.Text.Json to deserialise to a class which derives from a base class using required keyword in a property, the keyword has no effect.

Reproduction Steps

Code snippet showing the unexpected behavior (case 1) and also a working example (case 2):

using System;
using System.Text.Json;

public class Program
{
	public static void Main()
	{
		// Problematic test case
		Console.WriteLine("Case 1: Derived class");
		try
		{
			var obj = JsonSerializer.Deserialize("{}", typeof(DerivedTestClass));
			Console.WriteLine("obj: " + obj.ToString());
		}
		catch (JsonException e)
		{
			// Is not been thrown
			Console.WriteLine(e.Message);
		}

		// Second test case for reference
		Console.WriteLine("\nCase 2: Base class");
		try
		{
			var obj = JsonSerializer.Deserialize("{}", typeof(TestClass));
			Console.WriteLine("obj: " + obj.ToString());
		}
		catch (JsonException e)
		{
			// Is thrown as expected
			Console.WriteLine(e.Message);
		}
	}

	private class TestClass
	{
		public required string StringValue { get; set; }
		public required int IntegerValue { get; set; }
	}

	private class DerivedTestClass : TestClass
	{
		public override string ToString()
		{
			return $"StringValue: '{StringValue}', IntegerValue: '{IntegerValue}'";
		}
	}
}

Expected behavior

JsonSerializer.Deserialize should throw a JsonException for the required (and not set) properties.

Actual behavior

The deserialised class is instantiated with default values for the properties.

Regression?

Not applicable, new feature in .NET 7

Known Workarounds

The example works as expected when I used the [JsonRequired] attribute instead of the required keyword.

Configuration

.NET 7.0.5
Also on local machine (windows, x64) and in .NET Fiddle: https://dotnetfiddle.net/ffvMGn

Other information

No response

[ghost]

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

Issue Details

---

Description

When I use System.Text.Json to deserialise to a class which derives from a base class using required keyword in a property, the keyword has no effect.

Reproduction Steps

Code snippet showing the unexpected behavior (case 1) and also a working example (case 2):

using System;
using System.Text.Json;

public class Program
{
	public static void Main()
	{
		// Problematic test case
		Console.WriteLine("Case 1: Derived class");
		try
		{
			var obj = JsonSerializer.Deserialize("{}", typeof(DerivedTestClass));
			Console.WriteLine("obj: " + obj.ToString());
		}
		catch (JsonException e)
		{
			// Is not been thrown
			Console.WriteLine(e.Message);
		}

		// Second test case for reference
		Console.WriteLine("\nCase 2: Base class");
		try
		{
			var obj = JsonSerializer.Deserialize("{}", typeof(TestClass));
			Console.WriteLine("obj: " + obj.ToString());
		}
		catch (JsonException e)
		{
			// Is thrown as expected
			Console.WriteLine(e.Message);
		}
	}

	private class TestClass
	{
		public required string StringValue { get; set; }
		public required int IntegerValue { get; set; }
	}

	private class DerivedTestClass : TestClass
	{
		public override string ToString()
		{
			return $"StringValue: '{StringValue}', IntegerValue: '{IntegerValue}'";
		}
	}
}

Expected behavior

JsonSerializer.Deserialize should throw a JsonException for the required (and not set) properties.

Actual behavior

The deserialised class is instantiated with default values for the properties.

Regression?

Not applicable, new feature in .NET 7

Known Workarounds

The example works as expected when I used the [JsonRequired] attribute instead of the required keyword.

Configuration

.NET 7.0.5
Also on local machine (windows, x64) and in .NET Fiddle: https://dotnetfiddle.net/ffvMGn

Other information

No response

Author:	stedaho
Assignees:	-
Labels:	area-System.Text.Json, untriaged
Milestone:	-

[eiriktsarpalis]

cc @krwq

[eiriktsarpalis]

The issue appears to be caused by this logic:

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs

Lines 79 to 83 in 5e1608d

	// Compiler adds RequiredMemberAttribute to type if any of the members is marked with 'required' keyword.
	// SetsRequiredMembersAttribute means that all required members are assigned by constructor and therefore there is no enforcement
	bool shouldCheckMembersForRequiredMemberAttribute =
	typeInfo.Type.HasRequiredMemberAttribute()
	&& !(typeInfo.Converter.ConstructorInfo?.HasSetsRequiredMembersAttribute() ?? false);

The shouldCheckMembersForRequiredMemberAttribute is scoped to the declaring type, we should refine the check so that this boolean is evaluated for every type in the hierarchy.

[eiriktsarpalis]

I should add that the issue only impacts the reflection serializer, it works as expected if you try the same repro in the source generator.