diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs index 102d9a5213c414..75f78499b4fa7a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs @@ -146,6 +146,20 @@ public static Dictionary CreateDictionaryFromCollection + /// Adds a to the or returns the value. + /// + public static bool TryAdd(Dictionary dictionary, in TKey key, in TValue value, [MaybeNullWhen(true)] out TValue existing) where TKey : notnull + { + if (!dictionary.TryGetValue(key, out existing)) + { + dictionary[key] = value; + return true; + } + + return false; + } + public static bool IsFinite(double value) { #if BUILDING_INBOX_LIBRARY diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs index 2f7e7614430aba..9aa0b4d98ff953 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs @@ -198,6 +198,7 @@ internal JsonTypeInfo(Type type, JsonConverter converter, Type runtimeType, Json : StringComparer.Ordinal); Dictionary? ignoredMembers = null; + Dictionary? overrideProperties = null; // We start from the most derived type. for (Type? currentType = type; currentType != null; currentType = currentType.BaseType) @@ -220,7 +221,7 @@ internal JsonTypeInfo(Type type, JsonConverter converter, Type runtimeType, Json if (propertyInfo.GetMethod?.IsPublic == true || propertyInfo.SetMethod?.IsPublic == true) { - CacheMember(currentType, propertyInfo.PropertyType, propertyInfo, typeNumberHandling, cache, ref ignoredMembers); + CacheMember(currentType, propertyInfo.PropertyType, propertyInfo, typeNumberHandling, cache, ref ignoredMembers, ref overrideProperties); } else { @@ -235,18 +236,13 @@ internal JsonTypeInfo(Type type, JsonConverter converter, Type runtimeType, Json foreach (FieldInfo fieldInfo in currentType.GetFields(bindingFlags)) { - if (PropertyIsOverridenAndIgnored(fieldInfo, ignoredMembers)) - { - continue; - } - bool hasJsonInclude = JsonPropertyInfo.GetAttribute(fieldInfo) != null; if (fieldInfo.IsPublic) { if (hasJsonInclude || Options.IncludeFields) { - CacheMember(currentType, fieldInfo.FieldType, fieldInfo, typeNumberHandling, cache, ref ignoredMembers); + CacheMember(currentType, fieldInfo.FieldType, fieldInfo, typeNumberHandling, cache, ref ignoredMembers, ref overrideProperties); } } else @@ -328,18 +324,23 @@ private void CacheMember( MemberInfo memberInfo, JsonNumberHandling? typeNumberHandling, Dictionary cache, - ref Dictionary? ignoredMembers) + ref Dictionary? ignoredMembers, + ref Dictionary? overrideProperties) { JsonPropertyInfo jsonPropertyInfo = AddProperty(memberInfo, memberType, declaringType, typeNumberHandling, Options); Debug.Assert(jsonPropertyInfo.NameAsString != null); string memberName = memberInfo.Name; - - // The JsonPropertyNameAttribute or naming policy resulted in a collision. - if (!JsonHelpers.TryAdd(cache, jsonPropertyInfo.NameAsString, jsonPropertyInfo)) + PropertyInfo? property = memberInfo as PropertyInfo; + + // Virtual properties that are overrides are implicitly ignored regardless of a JsonPropertyNameAttribute. + if ((property == null || + overrideProperties == null || + !overrideProperties.TryGetValue(memberName, out PropertyInfo? overrideProperty) || + !IsOverrideOf(overrideProperty, property)) && + !JsonHelpers.TryAdd(cache, jsonPropertyInfo.NameAsString, jsonPropertyInfo, out JsonPropertyInfo? other)) { - JsonPropertyInfo other = cache[jsonPropertyInfo.NameAsString]; - + // The JsonPropertyNameAttribute or naming policy resulted in a collision. if (other.IsIgnored) { // Overwrite previously cached property since it has [JsonIgnore]. @@ -351,7 +352,7 @@ private void CacheMember( // Is the current property hidden by the previously cached property // (with `new` keyword, or by overriding)? other.MemberInfo!.Name != memberName && - // Was a property with the same CLR name was ignored? That property hid the current property, + // Was a property with the same CLR name ignored? That property hid the current property, // thus, if it was ignored, the current property should be ignored too. ignoredMembers?.ContainsKey(memberName) != true) { @@ -365,6 +366,34 @@ private void CacheMember( { (ignoredMembers ??= new Dictionary()).Add(memberName, memberInfo); } + else if (property != null && IsOverride(property)) + { + (overrideProperties ??= new Dictionary())[memberName] = property; + } + } + + private static bool IsOverrideOf(PropertyInfo property, PropertyInfo other) + { + MethodInfo? getMethod = property.GetMethod; + if (getMethod != null) + { + return getMethod.GetBaseDefinition() == other.GetMethod?.GetBaseDefinition(); + } + + MethodInfo? setMethod = property.SetMethod; + return setMethod != null && setMethod.GetBaseDefinition() == other.SetMethod?.GetBaseDefinition(); + } + + private static bool IsOverride(PropertyInfo property) + { + MethodInfo? getMethod = property.GetMethod; + if (getMethod != null) + { + return getMethod.GetBaseDefinition() != getMethod; + } + + MethodInfo? setMethod = property.SetMethod; + return setMethod != null && setMethod.GetBaseDefinition() == setMethod; } private sealed class ParameterLookupKey @@ -430,11 +459,10 @@ static Type GetMemberType(MemberInfo memberInfo) string propertyName = jsonProperty.MemberInfo!.Name; var key = new ParameterLookupKey(propertyName, GetMemberType(jsonProperty.MemberInfo)); var value= new ParameterLookupValue(jsonProperty); - if (!JsonHelpers.TryAdd(nameLookup, key, value)) + if (!JsonHelpers.TryAdd(nameLookup, key, value, out ParameterLookupValue? existing)) { // More than one property has the same case-insensitive name and Type. // Remember so we can throw a nice exception if this property is used as a parameter name. - ParameterLookupValue existing = nameLookup[key]; existing!.DuplicateName = propertyName; } } @@ -476,33 +504,16 @@ static Type GetMemberType(MemberInfo memberInfo) ParameterCache = parameterCache; ParameterCount = parameters.Length; } - private static bool PropertyIsOverridenAndIgnored(MemberInfo currentMember, Dictionary? ignoredMembers) + private static bool PropertyIsOverridenAndIgnored(PropertyInfo currentProperty, Dictionary? ignoredMembers) { - if (ignoredMembers == null || !ignoredMembers.TryGetValue(currentMember.Name, out MemberInfo? ignoredProperty)) + if (ignoredMembers == null || + !ignoredMembers.TryGetValue(currentProperty.Name, out MemberInfo? ignoredMember) || + ignoredMember is not PropertyInfo ignoredProperty) { return false; } - Debug.Assert(currentMember is PropertyInfo || currentMember is FieldInfo); - PropertyInfo? currentPropertyInfo = currentMember as PropertyInfo; - Type currentMemberType = currentPropertyInfo == null - ? Unsafe.As(currentMember).FieldType - : currentPropertyInfo.PropertyType; - - Debug.Assert(ignoredProperty is PropertyInfo || ignoredProperty is FieldInfo); - PropertyInfo? ignoredPropertyInfo = ignoredProperty as PropertyInfo; - Type ignoredPropertyType = ignoredPropertyInfo == null - ? Unsafe.As(ignoredProperty).FieldType - : ignoredPropertyInfo.PropertyType; - - return currentMemberType == ignoredPropertyType && - PropertyIsVirtual(currentPropertyInfo) && - PropertyIsVirtual(ignoredPropertyInfo); - } - - private static bool PropertyIsVirtual(PropertyInfo? propertyInfo) - { - return propertyInfo != null && (propertyInfo.GetMethod?.IsVirtual == true || propertyInfo.SetMethod?.IsVirtual == true); + return IsOverrideOf(currentProperty, ignoredProperty); } private bool DetermineExtensionDataProperty(Dictionary cache) diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/PropertyVisibilityTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/PropertyVisibilityTests.cs index f164592f4fb306..c43c801fee3afb 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/PropertyVisibilityTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/PropertyVisibilityTests.cs @@ -583,6 +583,9 @@ public static void HiddenPropertiesIgnored_WhenOverridesIgnored() string serialized = JsonSerializer.Serialize(new DerivedClass_With_IgnoredOverride()); Assert.Equal(@"{}", serialized); + serialized = JsonSerializer.Serialize(new DerivedClass_With_IgnoredNewSlotVirtual()); + Assert.Equal(@"{""MyProp"":false}", serialized); + serialized = JsonSerializer.Serialize(new DerivedClass_WithVisibleProperty_Of_DerivedClass_With_IgnoredOverride()); Assert.Equal(@"{""MyProp"":false}", serialized); @@ -627,6 +630,20 @@ public static void HiddenPropertiesIgnored_WhenOverridesIgnored() Assert.Equal(@"{""MyProp"":null}", serialized); } + [Fact] + public static void OverriddenPropertiesIgnored_WhenRenamed() + { + string serialized = JsonSerializer.Serialize(new DerivedClass_WithRenamedOverride()); + Assert.Equal(@"{""Renamed"":true}", serialized); + + serialized = JsonSerializer.Serialize(new FurtherDerivedClass_WithRenamedOverride()); + Assert.Equal(@"{""MyProp"":false}", serialized); + + // Don't ignore virtual properties with same name and type when they are in a new slot. + serialized = JsonSerializer.Serialize(new FurtherDerivedClass_WithRenamedNewSlotVirtual()); + Assert.Equal(@"{""MyProp"":false,""Renamed"":true}", serialized); + } + public class ClassWithInternalField { internal string MyString = "DefaultValue"; @@ -852,6 +869,12 @@ private class Class_With_VirtualProperty public virtual bool MyProp { get; set; } } + private class DerivedClass_With_IgnoredNewSlotVirtual : Class_With_VirtualProperty + { + [JsonIgnore] + public new virtual bool MyProp { get; set; } + } + private class DerivedClass_With_IgnoredOverride : Class_With_VirtualProperty { [JsonIgnore] @@ -953,6 +976,22 @@ private class FurtherDerivedClass_With_IgnoredOverride : DerivedClass_WithConfli public override bool MyProp { get; set; } } + private class DerivedClass_WithRenamedOverride : Class_With_VirtualProperty + { + [JsonPropertyName("Renamed")] + public override bool MyProp { get; set; } = true; + } + + private class FurtherDerivedClass_WithRenamedOverride : DerivedClass_WithRenamedOverride + { + public override bool MyProp { get; set; } + } + + private class FurtherDerivedClass_WithRenamedNewSlotVirtual : DerivedClass_WithRenamedOverride + { + public new virtual bool MyProp { get; set; } + } + [Fact] public static void IgnoreReadOnlyProperties() {