Setting RequiredMemberAttribute usage as inherited. (#89418)

* fix(86031): Processing "RequiredMemberAttribute" by whole type hierarchy.

* fix(86031): Optimization of calculation for RequiredMemberAttribute checking requirements.

* feat(86031): Adding additional tests for inheritance with required modifiers.

* fix(86031): Fix invalid number conversion for early existed tests.

* fix(86031): Fixes of formatting.

* fix(86031): Removing FluentAssertions dependency.

* Update src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/System.Text.Json.SourceGeneration.Tests.targets

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

---------

Co-authored-by: Maksim Golev <mgolev@htc-cs.ru>
Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs
index ea8d862..9a13d7a 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs
@@ -76,11 +76,9 @@ namespace System.Text.Json.Serialization.Metadata
             Debug.Assert(!typeInfo.IsReadOnly);
             Debug.Assert(typeInfo.Kind is JsonTypeInfoKind.Object);
 
-            // 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);
+            bool constructorHasSetsRequiredMembersAttribute =
+                typeInfo.Converter.ConstructorInfo?.HasSetsRequiredMembersAttribute() ?? false;
 
             JsonTypeInfo.PropertyHierarchyResolutionState state = new();
 
@@ -93,6 +91,10 @@ namespace System.Text.Json.Serialization.Metadata
                     break;
                 }
 
+                // Compiler adds RequiredMemberAttribute to type if any of the members are marked with 'required' keyword.
+                bool shouldCheckMembersForRequiredMemberAttribute =
+                    !constructorHasSetsRequiredMembersAttribute && currentType.HasRequiredMemberAttribute();
+
                 AddMembersDeclaredBySuperType(
                     typeInfo,
                     currentType,

diff --git a/src/libraries/System.Text.Json/tests/Common/NumberHandlingTests.cs b/src/libraries/System.Text.Json/tests/Common/NumberHandlingTests.cs
index 109123b..6b16a63 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Common/NumberHandlingTests.cs
+++ b/src/libraries/System.Text.Json/tests/Common/NumberHandlingTests.cs
@@ -104,7 +104,7 @@ namespace System.Text.Json.Serialization.Tests
                 double @double => @double.ToString(JsonTestHelper.DoubleFormatString, CultureInfo.InvariantCulture),
                 float @float => @float.ToString(JsonTestHelper.SingleFormatString, CultureInfo.InvariantCulture),
                 decimal @decimal => @decimal.ToString(CultureInfo.InvariantCulture),
-                _ => number.ToString()
+                _ => Convert.ToString(number, CultureInfo.InvariantCulture)
             };
         }
 

diff --git a/src/libraries/System.Text.Json/tests/Common/RequiredKeywordTests.cs b/src/libraries/System.Text.Json/tests/Common/RequiredKeywordTests.cs
index f1163ee..8741b76 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Common/RequiredKeywordTests.cs
+++ b/src/libraries/System.Text.Json/tests/Common/RequiredKeywordTests.cs
@@ -143,6 +143,86 @@ namespace System.Text.Json.Serialization.Tests
             Assert.Contains("Info2", exception.Message);
         }
 
+        [Fact]
+        public async Task InheritedPersonWithRequiredMembersWorksAsExpected()
+        {
+            var options = new JsonSerializerOptions(Serializer.DefaultOptions);
+            options.MakeReadOnly();
+
+            JsonTypeInfo typeInfo = options.GetTypeInfo(typeof(InheritedPersonWithRequiredMembers));
+            Assert.Equal(3, typeInfo.Properties.Count);
+
+            AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo<InheritedPersonWithRequiredMembers>(options),
+                nameof(InheritedPersonWithRequiredMembers.FirstName),
+                nameof(InheritedPersonWithRequiredMembers.LastName));
+
+            JsonException exception = await Assert.ThrowsAsync<JsonException>(() => Serializer.DeserializeWrapper("{}", typeInfo));
+            Assert.Contains("FirstName", exception.Message);
+            Assert.Contains("LastName", exception.Message);
+            Assert.DoesNotContain("MiddleName", exception.Message);
+        }
+
+        [Fact]
+        public async Task InheritedPersonWithRequiredMembersWithAdditionalRequiredMembersWorksAsExpected()
+        {
+            var options = new JsonSerializerOptions(Serializer.DefaultOptions);
+            options.MakeReadOnly();
+
+            JsonTypeInfo typeInfo = options.GetTypeInfo(typeof(InheritedPersonWithRequiredMembersWithAdditionalRequiredMembers));
+            Assert.Equal(4, typeInfo.Properties.Count);
+
+            AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo<InheritedPersonWithRequiredMembersWithAdditionalRequiredMembers>(options),
+                nameof(InheritedPersonWithRequiredMembersWithAdditionalRequiredMembers.FirstName),
+                nameof(InheritedPersonWithRequiredMembersWithAdditionalRequiredMembers.LastName),
+                nameof(InheritedPersonWithRequiredMembersWithAdditionalRequiredMembers.Post));
+
+            JsonException exception = await Assert.ThrowsAsync<JsonException>(() => Serializer.DeserializeWrapper("{}", typeInfo));
+            Assert.Contains("FirstName", exception.Message);
+            Assert.Contains("LastName", exception.Message);
+            Assert.Contains("Post", exception.Message);
+            Assert.DoesNotContain("MiddleName", exception.Message);
+        }
+
+        [Theory]
+        [MemberData(nameof(InheritedPersonWithRequiredMembersSetsRequiredMembersWorksAsExpectedSources))]
+        public async Task InheritedPersonWithRequiredMembersSetsRequiredMembersWorksAsExpected(string jsonValue,
+            InheritedPersonWithRequiredMembersSetsRequiredMembers expectedValue)
+        {
+            var options = new JsonSerializerOptions(Serializer.DefaultOptions);
+            options.MakeReadOnly();
+
+            JsonTypeInfo typeInfo = options.GetTypeInfo(typeof(InheritedPersonWithRequiredMembersSetsRequiredMembers));
+            Assert.Equal(3, typeInfo.Properties.Count);
+
+            AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo<InheritedPersonWithRequiredMembersSetsRequiredMembers>(options));
+
+            InheritedPersonWithRequiredMembersSetsRequiredMembers actualValue =
+                await Serializer.DeserializeWrapper<InheritedPersonWithRequiredMembersSetsRequiredMembers>(jsonValue, options);
+            Assert.Equal(expectedValue.FirstName, actualValue.FirstName);
+            Assert.Equal(expectedValue.LastName, actualValue.LastName);
+            Assert.Equal(expectedValue.MiddleName, actualValue.MiddleName);
+        }
+
+        public class InheritedPersonWithRequiredMembers : PersonWithRequiredMembers
+        {
+        }
+
+        public class InheritedPersonWithRequiredMembersWithAdditionalRequiredMembers : PersonWithRequiredMembers
+        {
+            public required string Post { get; set; }
+        }
+
+        public class InheritedPersonWithRequiredMembersSetsRequiredMembers : PersonWithRequiredMembers
+        {
+            [SetsRequiredMembers]
+            public InheritedPersonWithRequiredMembersSetsRequiredMembers()
+            {
+                FirstName = "FirstNameValueFromConstructor";
+                LastName = "LastNameValueFromConstructor";
+                MiddleName = "MiddleNameValueFromConstructor";
+            }
+        }
+
         public class PersonWithRequiredMembersAndSmallParametrizedCtor
         {
             public required string FirstName { get; set; }
@@ -584,6 +664,23 @@ namespace System.Text.Json.Serialization.Tests
             public required int PropertyWithInitOnlySetter { get; init; }
         }
 
+        public static IEnumerable<object[]> InheritedPersonWithRequiredMembersSetsRequiredMembersWorksAsExpectedSources()
+        {
+            yield return new object[]
+            {
+                "{}",
+                new InheritedPersonWithRequiredMembersSetsRequiredMembers()
+            };
+            yield return new object[]
+            {
+                """{"FirstName": "FirstNameFromJson"}""",
+                new InheritedPersonWithRequiredMembersSetsRequiredMembers
+                {
+                    FirstName = "FirstNameFromJson"
+                }
+            };
+        }
+
         private static JsonTypeInfo GetTypeInfo<T>(JsonSerializerOptions options)
         {
             options.TypeInfoResolver ??= JsonSerializerOptions.Default.TypeInfoResolver;

diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/RequiredKeywordTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/RequiredKeywordTests.cs
index 58cae74..91a51b7 100644 (file)
--- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/RequiredKeywordTests.cs
+++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/RequiredKeywordTests.cs
@@ -18,6 +18,9 @@ namespace System.Text.Json.SourceGeneration.Tests
         {
         }
 
+        [JsonSerializable(typeof(InheritedPersonWithRequiredMembers))]
+        [JsonSerializable(typeof(InheritedPersonWithRequiredMembersWithAdditionalRequiredMembers))]
+        [JsonSerializable(typeof(InheritedPersonWithRequiredMembersSetsRequiredMembers))]
         [JsonSerializable(typeof(PersonWithRequiredMembers))]
         [JsonSerializable(typeof(PersonWithRequiredMembersAndSmallParametrizedCtor))]
         [JsonSerializable(typeof(PersonWithRequiredMembersAndLargeParametrizedCtor))]