Ensure that types of ignored or inaccessible properties are not inclu…

…ded by the source generator. (#87383)

src/libraries/System.Text.Json/gen/Helpers/KnownTypeSymbols.cs

@@ -197,9 +197,21 @@ public KnownTypeSymbols(Compilation compilation)
         public INamedTypeSymbol? JsonUnmappedMemberHandlingAttributeType => GetOrResolveType("System.Text.Json.Serialization.JsonUnmappedMemberHandlingAttribute", ref _JsonUnmappedMemberHandlingAttributeType);
         private Option<INamedTypeSymbol?> _JsonUnmappedMemberHandlingAttributeType;
 
+        public INamedTypeSymbol? JsonConstructorAttributeType => GetOrResolveType("System.Text.Json.Serialization.JsonConstructorAttribute", ref _JsonConstructorAttributeType);
+        private Option<INamedTypeSymbol?> _JsonConstructorAttributeType;
+
+        public INamedTypeSymbol? SetsRequiredMembersAttributeType => GetOrResolveType("System.Diagnostics.CodeAnalysis.SetsRequiredMembersAttribute", ref _SetsRequiredMembersAttributeType);
+        private Option<INamedTypeSymbol?> _SetsRequiredMembersAttributeType;
+
         public INamedTypeSymbol? JsonStringEnumConverterType => GetOrResolveType("System.Text.Json.Serialization.JsonStringEnumConverter", ref _JsonStringEnumConverterType);
         private Option<INamedTypeSymbol?> _JsonStringEnumConverterType;
 
+        public INamedTypeSymbol? IJsonOnSerializingType => GetOrResolveType(JsonConstants.IJsonOnSerializingFullName, ref _IJsonOnSerializingType);
+        private Option<INamedTypeSymbol?> _IJsonOnSerializingType;
+
+        public INamedTypeSymbol? IJsonOnSerializedType => GetOrResolveType(JsonConstants.IJsonOnSerializedFullName, ref _IJsonOnSerializedType);
+        private Option<INamedTypeSymbol?> _IJsonOnSerializedType;
+
         // Unsupported types
         public INamedTypeSymbol? DelegateType => _DelegateType ??= Compilation.GetSpecialType(SpecialType.System_Delegate);
         private INamedTypeSymbol? _DelegateType;

src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs

@@ -98,8 +98,8 @@ public static bool CanUseDefaultConstructorForDeserialization(this ITypeSymbol t
         public static IEnumerable<IMethodSymbol> GetExplicitlyDeclaredInstanceConstructors(this INamedTypeSymbol type)
             => type.Constructors.Where(ctor => !ctor.IsStatic && !(ctor.IsImplicitlyDeclared && type.IsValueType && ctor.Parameters.Length == 0));
 
-        public static bool ContainsAttribute(this ISymbol memberInfo, string attributeFullName)
-            => memberInfo.GetAttributes().Any(attr => attr.AttributeClass?.ToDisplayString() == attributeFullName);
+        public static bool ContainsAttribute(this ISymbol memberInfo, INamedTypeSymbol? attributeType)
+            => attributeType != null && memberInfo.GetAttributes().Any(attr => SymbolEqualityComparer.Default.Equals(attr.AttributeClass, attributeType));
 
         public static bool IsVirtual(this ISymbol symbol)
             => symbol.IsVirtual || symbol.IsOverride || symbol.IsAbstract;

src/libraries/System.Text.Json/gen/Model/PropertyGenerationSpec.cs

@@ -137,5 +137,47 @@ public sealed record PropertyGenerationSpec
         /// Design-time specified custom converter type.
         /// </summary>
         public required TypeRef? ConverterType { get; init; }
+
+        /// <summary>
+        /// Determines if the specified property should be included in the fast-path method body.
+        /// </summary>
+        public bool ShouldIncludePropertyForFastPath(ContextGenerationSpec contextSpec)
+        {
+            // Discard ignored properties
+            if (DefaultIgnoreCondition is JsonIgnoreCondition.Always)
+            {
+                return false;
+            }
+
+            // Discard properties without getters
+            if (!CanUseGetter)
+            {
+                return false;
+            }
+
+            // Discard fields when JsonInclude or IncludeFields aren't enabled.
+            if (!IsProperty && !HasJsonInclude && !contextSpec.IncludeFields)
+            {
+                return false;
+            }
+
+            // Ignore read-only properties/fields if enabled in configuration.
+            if (IsReadOnly)
+            {
+                if (IsProperty)
+                {
+                    if (contextSpec.IgnoreReadOnlyProperties)
+                    {
+                        return false;
+                    }
+                }
+                else if (contextSpec.IgnoreReadOnlyFields)
+                {
+                    return false;
+                }
+            }
+
+            return true;
+        }
     }
 }

src/libraries/System.Text.Json/gen/Model/TypeGenerationSpec.cs

@@ -3,6 +3,7 @@
 
 using System.Diagnostics;
 using System.Text.Json.Serialization;
+using Microsoft.CodeAnalysis;
 
 namespace System.Text.Json.SourceGeneration
 {
@@ -55,11 +56,11 @@ public sealed record TypeGenerationSpec
         public required JsonUnmappedMemberHandling? UnmappedMemberHandling { get; init; }
         public required JsonObjectCreationHandling? PreferredPropertyObjectCreationHandling { get; init; }
 
-        public required ImmutableEquatableArray<PropertyGenerationSpec>? PropertyGenSpecs { get; init; }
+        public required ImmutableEquatableArray<PropertyGenerationSpec> PropertyGenSpecs { get; init; }
 
-        public required ImmutableEquatableArray<ParameterGenerationSpec>? CtorParamGenSpecs { get; init; }
+        public required ImmutableEquatableArray<ParameterGenerationSpec> CtorParamGenSpecs { get; init; }
 
-        public required ImmutableEquatableArray<PropertyInitializerGenerationSpec>? PropertyInitializerSpecs { get; init; }
+        public required ImmutableEquatableArray<PropertyInitializerGenerationSpec> PropertyInitializerSpecs { get; init; }
 
         public required CollectionType CollectionType { get; init; }
 
@@ -79,10 +80,51 @@ public sealed record TypeGenerationSpec
         /// </summary>
         public required TypeRef? RuntimeTypeRef { get; init; }
 
-        public required TypeRef? ExtensionDataPropertyType { get; init; }
+        public required bool HasExtensionDataPropertyType { get; init; }
 
         public required TypeRef? ConverterType { get; init; }
 
         public required string? ImmutableCollectionFactoryMethod { get; init; }
+
+        public bool IsFastPathSupported()
+        {
+            if (IsPolymorphic)
+            {
+                return false;
+            }
+
+            switch (ClassType)
+            {
+                case ClassType.Object:
+                    if (HasExtensionDataPropertyType)
+                    {
+                        return false;
+                    }
+
+                    foreach (PropertyGenerationSpec property in PropertyGenSpecs)
+                    {
+                        if (property.PropertyType.SpecialType is SpecialType.System_Object ||
+                            property.NumberHandling is JsonNumberHandling.AllowNamedFloatingPointLiterals
+                                                    or JsonNumberHandling.WriteAsString ||
+                            property.ConverterType != null)
+                        {
+                            return false;
+                        }
+                    }
+
+                    return true;
+
+                case ClassType.Enumerable:
+                    return CollectionType != CollectionType.IAsyncEnumerableOfT &&
+                           CollectionValueType!.SpecialType is not SpecialType.System_Object;
+
+                case ClassType.Dictionary:
+                    return CollectionKeyType!.SpecialType is SpecialType.System_String &&
+                           CollectionValueType!.SpecialType is not SpecialType.System_Object;
+
+                default:
+                    return false;
+            }
+        }
     }
 }

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

@@ -123,15 +123,11 @@ private static void AddMembersDeclaredBySuperType(
                 BindingFlags.NonPublic |
                 BindingFlags.DeclaredOnly;
 
-            PropertyInfo[] properties = currentType.GetProperties(BindingFlags);
-
-            foreach (PropertyInfo propertyInfo in properties)
+            foreach (PropertyInfo propertyInfo in currentType.GetProperties(BindingFlags))
             {
-                string propertyName = propertyInfo.Name;
-
                 // Ignore indexers and virtual properties that have overrides that were [JsonIgnore]d.
                 if (propertyInfo.GetIndexParameters().Length > 0 ||
-                    PropertyIsOverriddenAndIgnored(propertyName, propertyInfo.PropertyType, propertyInfo.IsVirtual(), state.IgnoredProperties))
+                    PropertyIsOverriddenAndIgnored(propertyInfo, state.IgnoredProperties))
                 {
                     continue;
                 }
@@ -151,7 +147,7 @@ private static void AddMembersDeclaredBySuperType(
                 {
                     if (propertyInfo.GetCustomAttribute<JsonIncludeAttribute>(inherit: false) != null)
                     {
-                        ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(propertyName, currentType);
+                        ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(propertyInfo.Name, currentType);
                     }
 
                     // Non-public properties should not be included for (de)serialization.
@@ -160,13 +156,6 @@ private static void AddMembersDeclaredBySuperType(
 
             foreach (FieldInfo fieldInfo in currentType.GetFields(BindingFlags))
             {
-                string fieldName = fieldInfo.Name;
-
-                if (PropertyIsOverriddenAndIgnored(fieldName, fieldInfo.FieldType, currentMemberIsVirtual: false, state.IgnoredProperties))
-                {
-                    continue;
-                }
-
                 bool hasJsonInclude = fieldInfo.GetCustomAttribute<JsonIncludeAttribute>(inherit: false) != null;
 
                 if (fieldInfo.IsPublic)
@@ -185,7 +174,7 @@ private static void AddMembersDeclaredBySuperType(
                 {
                     if (hasJsonInclude)
                     {
-                        ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(fieldName, currentType);
+                        ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(fieldInfo.Name, currentType);
                     }
 
                     // Non-public fields should not be included for (de)serialization.
@@ -267,20 +256,12 @@ private static void AddMember(
             return numberHandlingAttribute?.UnmappedMemberHandling;
         }
 
-        private static bool PropertyIsOverriddenAndIgnored(
-            string currentMemberName,
-            Type currentMemberType,
-            bool currentMemberIsVirtual,
-            Dictionary<string, JsonPropertyInfo>? ignoredMembers)
+        private static bool PropertyIsOverriddenAndIgnored(PropertyInfo propertyInfo, Dictionary<string, JsonPropertyInfo>? ignoredMembers)
         {
-            if (ignoredMembers == null || !ignoredMembers.TryGetValue(currentMemberName, out JsonPropertyInfo? ignoredMember))
-            {
-                return false;
-            }
-
-            return currentMemberType == ignoredMember.PropertyType &&
-                currentMemberIsVirtual &&
-                ignoredMember.IsVirtual;
+            return propertyInfo.IsVirtual() &&
+                ignoredMembers?.TryGetValue(propertyInfo.Name, out JsonPropertyInfo? ignoredMember) == true &&
+                ignoredMember.IsVirtual &&
+                propertyInfo.PropertyType == ignoredMember.PropertyType;
         }
 
         private static void PopulateParameterInfoValues(JsonTypeInfo typeInfo)

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.Helpers.cs

@@ -135,8 +135,8 @@ internal static void PopulateProperties(JsonTypeInfo typeInfo, JsonTypeInfo.Json
             JsonSerializerContext? context = typeInfo.Options.TypeInfoResolver as JsonSerializerContext;
             JsonPropertyInfo[] properties = propInitFunc(context!);
 
-            // TODO update the source generator so that all property
-            // hierarchy resolution is happening at compile time.
+            // Regardless of the source generator we need to re-run the naming conflict resolution algorithm
+            // at run time since it is possible that the naming policy or other configs can be different then.
             JsonTypeInfo.PropertyHierarchyResolutionState state = new();
 
             foreach (JsonPropertyInfo jsonPropertyInfo in properties)

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs

@@ -1347,6 +1347,8 @@ public void AddPropertyWithConflictResolution(JsonPropertyInfo jsonPropertyInfo,
             {
                 Debug.Assert(!_jsonTypeInfo.IsConfigured);
                 Debug.Assert(jsonPropertyInfo.MemberName != null, "MemberName can be null in custom JsonPropertyInfo instances and should never be passed in this method");
+
+                // Algorithm should be kept in sync with the Roslyn equivalent in JsonSourceGenerator.Emitter.cs
                 string memberName = jsonPropertyInfo.MemberName;
 
                 if (state.AddedProperties.TryAdd(jsonPropertyInfo.Name, (jsonPropertyInfo, Count)))
@@ -1405,8 +1407,7 @@ public void AddPropertyWithConflictResolution(JsonPropertyInfo jsonPropertyInfo,
 
                 if (jsonPropertyInfo.IsIgnored)
                 {
-                    state.IgnoredProperties ??= new();
-                    state.IgnoredProperties[memberName] = jsonPropertyInfo;
+                    (state.IgnoredProperties ??= new())[memberName] = jsonPropertyInfo;
                 }
             }
         }

src/libraries/System.Text.Json/tests/Common/PropertyVisibilityTests.NonPublicAccessors.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections.Generic;
+using System.Text.Json.Serialization.Metadata;
 using System.Threading.Tasks;
 using Xunit;
 
@@ -395,5 +396,66 @@ public class ClassWithProtected_InitOnlyProperty_WithJsonIncludeProperty
             [JsonInclude]
             protected string MyString { get; init; }
         }
+
+        [Fact]
+        public async Task CanAlwaysRoundtripInternalJsonIncludeProperties()
+        {
+            // Internal JsonInclude properties should be honored
+            // by both reflection and the source generator.
+
+            var value = new ClassWithInternalJsonIncludeProperties { X = 1, Y = 2 };
+            string json = await Serializer.SerializeWrapper(value);
+            Assert.Equal("""{"X":1,"Y":2}""", json);
+
+            value = await Serializer.DeserializeWrapper<ClassWithInternalJsonIncludeProperties>(json);
+            Assert.Equal(1, value.X);
+            Assert.Equal(2, value.Y);
+        }
+
+        public class ClassWithInternalJsonIncludeProperties
+        {
+            [JsonInclude]
+            public int X { get; internal set; }
+            [JsonInclude]
+            public int Y { internal get; set; }
+        }
+
+        [Fact]
+        public virtual async Task ClassWithIgnoredAndPrivateMembers_DoesNotIncludeIgnoredMetadata()
+        {
+            JsonSerializerOptions options = Serializer.CreateOptions(includeFields: true);
+
+            JsonTypeInfo typeInfo = options.GetTypeInfo(typeof(ClassWithIgnoredAndPrivateMembers));
+
+            // The contract surfaces the ignored properties but not the private ones
+            Assert.Equal(2, typeInfo.Properties.Count);
+            Assert.Contains(typeInfo.Properties, prop => prop.Name == "PublicIgnoredField");
+            Assert.Contains(typeInfo.Properties, prop => prop.Name == "PublicIgnoredProperty");
+
+            // The ignored properties included in the contract do not specify any accessor delegates.
+            Assert.All(typeInfo.Properties, prop => Assert.True(prop.Get is null));
+            Assert.All(typeInfo.Properties, prop => Assert.True(prop.Set is null));
+
+            string json = await Serializer.SerializeWrapper(new ClassWithIgnoredAndPrivateMembers(), options);
+            Assert.Equal("{}", json);
+        }
+
+        public class ClassWithIgnoredAndPrivateMembers
+        {
+            [JsonIgnore]
+            public TypeThatShouldNotBeGenerated PublicIgnoredField = new();
+
+            private TypeThatShouldNotBeGenerated PrivateField = new();
+
+            [JsonIgnore]
+            public TypeThatShouldNotBeGenerated PublicIgnoredProperty { get; set; } = new();
+
+            private TypeThatShouldNotBeGenerated PrivateProperty { get; set; } = new();
+        }
+
+        public class TypeThatShouldNotBeGenerated
+        {
+            private protected object _thisLock = new object();
+        }
     }
 }

src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/PropertyVisibilityTests.cs

@@ -171,6 +171,17 @@ public override async Task TestDictionaryWithPrivateKeyAndValueType()
             await Assert.ThrowsAsync<NotSupportedException>(() => Serializer.DeserializeWrapper<DictionaryWithPrivateKeyAndValueType>(json));
         }
 
+        [Fact]
+        public override async Task ClassWithIgnoredAndPrivateMembers_DoesNotIncludeIgnoredMetadata()
+        {
+            // The type referenced by the ignored/private properties should
+            // not be included in the supported types by the generator.
+            JsonSerializerOptions options = Serializer.DefaultOptions;
+            Assert.Null(options.TypeInfoResolver.GetTypeInfo(typeof(TypeThatShouldNotBeGenerated), options));
+
+            await base.ClassWithIgnoredAndPrivateMembers_DoesNotIncludeIgnoredMetadata();
+        }
+
         [JsonSourceGenerationOptions(GenerationMode = JsonSourceGenerationMode.Metadata)]
         [JsonSerializable(typeof(ClassWithNewSlotField))]
         [JsonSerializable(typeof(int))]
@@ -320,7 +331,9 @@ public override async Task TestDictionaryWithPrivateKeyAndValueType()
         [JsonSerializable(typeof(IDiamondInterfaceHierarchyWithNamingConflict.IJoinInterface), TypeInfoPropertyName = "IDiamondInterfaceHierarchyWithNamingConflictIJoinInterface")]
         [JsonSerializable(typeof(IDiamondInterfaceHierarchyWithNamingConflictUsingAttribute.IJoinInterface), TypeInfoPropertyName = "IDiamondInterfaceHierarchyWithNamingConflictUsingAttributeIJoinInterface")]
         [JsonSerializable(typeof(CollectionWithPrivateElementType))]
-        [JsonSerializable(typeof(DictionaryWithPrivateKeyAndValueType))]
+        [JsonSerializable(typeof(DictionaryWithPrivateKeyAndValueType))][JsonSerializable(typeof(ClassWithIgnoredAndPrivateMembers))]
+        [JsonSerializable(typeof(ClassWithInternalJsonIncludeProperties))]
+        [JsonSerializable(typeof(ClassWithIgnoredAndPrivateMembers))]
         internal sealed partial class PropertyVisibilityTestsContext_Metadata : JsonSerializerContext
         {
         }
@@ -576,6 +589,8 @@ public void PublicContextAndJsonSerializerOptions()
         [JsonSerializable(typeof(IDiamondInterfaceHierarchyWithNamingConflictUsingAttribute.IJoinInterface), TypeInfoPropertyName = "IDiamondInterfaceHierarchyWithNamingConflictUsingAttributeIJoinInterface")]
         [JsonSerializable(typeof(CollectionWithPrivateElementType))]
         [JsonSerializable(typeof(DictionaryWithPrivateKeyAndValueType))]
+        [JsonSerializable(typeof(ClassWithInternalJsonIncludeProperties))]
+        [JsonSerializable(typeof(ClassWithIgnoredAndPrivateMembers))]
         internal sealed partial class PropertyVisibilityTestsContext_Default : JsonSerializerContext
         {
         }