[release/8.0-rc1] Fix support for non-public default constructors using JsonIncludeAt...

* Fix support for non-public constructors using JsonIncludeAttribute

* Address feedback.

---------

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 7ec9db9adc77396a58bf93831491c710d2029ded..8ee9f3db0283b4f220453d29a469d52ca8a31aec 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
@@ -49,7 +49,7 @@ namespace System.Text.Json.Serialization.Metadata
             typeInfo.PopulatePolymorphismMetadata();
             typeInfo.MapInterfaceTypesToCallbacks();
 
-            Func<object>? createObject = MemberAccessor.CreateConstructor(typeInfo.Type);
+            Func<object>? createObject = DetermineCreateObjectDelegate(type, converter);
             typeInfo.SetCreateObjectIfCompatible(createObject);
             typeInfo.CreateObjectForExtensionDataProperty = createObject;
 
@@ -411,5 +411,24 @@ namespace System.Text.Json.Serialization.Metadata
                     break;
             }
         }
+
+        [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
+        [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
+        private static Func<object>? DetermineCreateObjectDelegate(Type type, JsonConverter converter)
+        {
+            ConstructorInfo? defaultCtor = null;
+
+            if (converter.ConstructorInfo != null && !converter.ConstructorIsParameterized)
+            {
+                // A parameterless constructor has been resolved by the converter
+                // (e.g. it might be a non-public ctor with JsonConverterAttribute).
+                defaultCtor = converter.ConstructorInfo;
+            }
+
+            // Fall back to resolving any public constructors on the type.
+            defaultCtor ??= type.GetConstructor(BindingFlags.Public | BindingFlags.Instance, binder: null, Type.EmptyTypes, modifiers: null);
+
+            return MemberAccessor.CreateParameterlessConstructor(type, defaultCtor);
+        }
     }
 }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/MemberAccessor.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/MemberAccessor.cs
index 326ab657b8899d61a255167c8816e4abdc63d421..ff6c442fa488cbd2b451c7ebdd0e40d8eaea5861 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/MemberAccessor.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/MemberAccessor.cs
@@ -9,8 +9,9 @@ namespace System.Text.Json.Serialization.Metadata
 {
     internal abstract class MemberAccessor
     {
-        public abstract Func<object>? CreateConstructor(
-            [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type classType);
+        public abstract Func<object>? CreateParameterlessConstructor(
+             [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type,
+             ConstructorInfo? constructorInfo);
 
         public abstract Func<object[], T> CreateParameterizedConstructor<T>(ConstructorInfo constructor);
 

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs
index a243f4be4d86a67c8f764f37da13db63b29b7014..e30f87d76da9f14e1e262d801876c6c9c280e43a 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs
@@ -21,12 +21,12 @@ namespace System.Text.Json.Serialization.Metadata
             => s_cache.GetOrAdd((nameof(CreateAddMethodDelegate), typeof(TCollection), null),
                 static (_) => s_sourceAccessor.CreateAddMethodDelegate<TCollection>());
 
-        public override Func<object>? CreateConstructor([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type classType)
-            => s_cache.GetOrAdd((nameof(CreateConstructor), classType, null),
+        public override Func<object>? CreateParameterlessConstructor([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type, ConstructorInfo? ctorInfo)
+            => s_cache.GetOrAdd((nameof(CreateParameterlessConstructor), type, ctorInfo),
                 [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2077:UnrecognizedReflectionPattern",
                     Justification = "Cannot apply DynamicallyAccessedMembersAttribute to tuple properties.")]
 #pragma warning disable IL2077 // The suppression doesn't work for the trim analyzer: https://github.com/dotnet/roslyn/issues/59746
-                static (key) => s_sourceAccessor.CreateConstructor(key.declaringType));
+                static (key) => s_sourceAccessor.CreateParameterlessConstructor(key.declaringType, (ConstructorInfo?)key.member));
 #pragma warning restore IL2077
 
         public override Func<object, TProperty> CreateFieldGetter<TProperty>(FieldInfo fieldInfo)

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitMemberAccessor.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitMemberAccessor.cs
index 6e05e5c8057ac7202067cf26f3e479eac7dcca5b..4b0b426bdaa9ef7999fc69b4d080bae9fb51a8e5 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitMemberAccessor.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitMemberAccessor.cs
@@ -13,18 +13,19 @@ namespace System.Text.Json.Serialization.Metadata
     [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
     internal sealed class ReflectionEmitMemberAccessor : MemberAccessor
     {
-        public override Func<object>? CreateConstructor(
-            [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type)
+        public override Func<object>? CreateParameterlessConstructor(
+            [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type,
+            ConstructorInfo? constructorInfo)
         {
             Debug.Assert(type != null);
-            ConstructorInfo? realMethod = type.GetConstructor(BindingFlags.Public | BindingFlags.Instance, binder: null, Type.EmptyTypes, modifiers: null);
+            Debug.Assert(constructorInfo is null || constructorInfo.GetParameters().Length == 0);
 
             if (type.IsAbstract)
             {
                 return null;
             }
 
-            if (realMethod == null && !type.IsValueType)
+            if (constructorInfo is null && !type.IsValueType)
             {
                 return null;
             }
@@ -38,8 +39,10 @@ namespace System.Text.Json.Serialization.Metadata
 
             ILGenerator generator = dynamicMethod.GetILGenerator();
 
-            if (realMethod == null)
+            if (constructorInfo is null)
             {
+                Debug.Assert(type.IsValueType);
+
                 LocalBuilder local = generator.DeclareLocal(type);
 
                 generator.Emit(OpCodes.Ldloca_S, local);
@@ -49,7 +52,7 @@ namespace System.Text.Json.Serialization.Metadata
             }
             else
             {
-                generator.Emit(OpCodes.Newobj, realMethod);
+                generator.Emit(OpCodes.Newobj, constructorInfo);
                 if (type.IsValueType)
                 {
                     // Since C# 10 it's now possible to have parameterless constructors in structs

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionMemberAccessor.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionMemberAccessor.cs
index 606ad6aba79c260458fb978c078ed89121bca8a1..8627a24f3f49267f09242cb96d7e8d2e3a20472d 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionMemberAccessor.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionMemberAccessor.cs
@@ -10,35 +10,26 @@ namespace System.Text.Json.Serialization.Metadata
 {
     internal sealed class ReflectionMemberAccessor : MemberAccessor
     {
-        private sealed class ConstructorContext
-        {
-            [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)]
-            private readonly Type _type;
-
-            public ConstructorContext([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type)
-                => _type = type;
-
-            public object? CreateInstance()
-                => Activator.CreateInstance(_type, nonPublic: false);
-        }
-
-        public override Func<object>? CreateConstructor(
-            [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type)
+        public override Func<object>? CreateParameterlessConstructor(
+            [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type,
+            ConstructorInfo? ctorInfo)
         {
             Debug.Assert(type != null);
-            ConstructorInfo? realMethod = type.GetConstructor(BindingFlags.Public | BindingFlags.Instance, binder: null, Type.EmptyTypes, modifiers: null);
+            Debug.Assert(ctorInfo is null || ctorInfo.GetParameters().Length == 0);
 
             if (type.IsAbstract)
             {
                 return null;
             }
 
-            if (realMethod == null && !type.IsValueType)
+            if (ctorInfo is null)
             {
-                return null;
+                return type.IsValueType
+                    ? () => Activator.CreateInstance(type, nonPublic: false)!
+                    : null;
             }
 
-            return new ConstructorContext(type).CreateInstance!;
+            return () => ctorInfo.Invoke(null);
         }
 
         public override Func<object[], T> CreateParameterizedConstructor<T>(ConstructorInfo constructor)

diff --git a/src/libraries/System.Text.Json/tests/Common/ConstructorTests/ConstructorTests.AttributePresence.cs b/src/libraries/System.Text.Json/tests/Common/ConstructorTests/ConstructorTests.AttributePresence.cs
index 681d495b8c752c49881e52a1b1a68b06c336d025..f54692a5c59a44f793efdcf413daed8a747042dd 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Common/ConstructorTests/ConstructorTests.AttributePresence.cs
+++ b/src/libraries/System.Text.Json/tests/Common/ConstructorTests/ConstructorTests.AttributePresence.cs
@@ -39,6 +39,24 @@ namespace System.Text.Json.Serialization.Tests
             }
         }
 
+        [Theory]
+        [InlineData(typeof(PrivateParameterlessCtor_WithAttribute), false)]
+        [InlineData(typeof(InternalParameterlessCtor_WithAttribute), true)]
+        [InlineData(typeof(ProtectedParameterlessCtor_WithAttribute), false)]
+        public async Task NonPublicParameterlessCtors_WithJsonConstructorAttribute_WorksAsExpected(Type type, bool isAccessibleBySourceGen)
+        {
+            if (!Serializer.IsSourceGeneratedSerializer || isAccessibleBySourceGen)
+            {
+                object? result = await Serializer.DeserializeWrapper("{}", type);
+                Assert.IsType(type, result);
+            }
+            else
+            {
+                NotSupportedException ex = await Assert.ThrowsAsync<NotSupportedException>(() => Serializer.DeserializeWrapper("{}", type));
+                Assert.Contains("JsonConstructorAttribute", ex.ToString());
+            }
+        }
+
         [Fact]
         public async Task SinglePublicParameterizedCtor_SingleParameterlessCtor_NoAttribute_Supported_UseParameterlessCtor()
         {

diff --git a/src/libraries/System.Text.Json/tests/Common/TestClasses/TestClasses.Constructor.cs b/src/libraries/System.Text.Json/tests/Common/TestClasses/TestClasses.Constructor.cs
index 285c5b7893473363d57e93f1607ffcbfe96a66ee..f12cf89e41de5978f4060dab1877f361f2dc514e 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Common/TestClasses/TestClasses.Constructor.cs
+++ b/src/libraries/System.Text.Json/tests/Common/TestClasses/TestClasses.Constructor.cs
@@ -74,6 +74,33 @@ namespace System.Text.Json.Serialization.Tests
         protected ProtectedParameterizedCtor_WithAttribute(int x) => X = x;
     }
 
+    public class PrivateParameterlessCtor_WithAttribute
+    {
+        public int X { get; }
+
+        [JsonConstructor]
+        private PrivateParameterlessCtor_WithAttribute()
+            => X = 42;
+    }
+
+    public class ProtectedParameterlessCtor_WithAttribute
+    {
+        public int X { get; }
+
+        [JsonConstructor]
+        protected ProtectedParameterlessCtor_WithAttribute()
+            => X = 42;
+    }
+
+    public class InternalParameterlessCtor_WithAttribute
+    {
+        public int X { get; }
+
+        [JsonConstructor]
+        internal InternalParameterlessCtor_WithAttribute()
+            => X = 42;
+    }
+
     public class PrivateParameterlessCtor_InternalParameterizedCtor_WithMultipleAttributes
     {
         [JsonConstructor]

diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ConstructorTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ConstructorTests.cs
index b2a5fd465da7661756eeb24270f2e693687cb4f1..d1769f491104dbba916e962578226b2dc658c797 100644 (file)
--- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ConstructorTests.cs
+++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ConstructorTests.cs
@@ -41,6 +41,9 @@ namespace System.Text.Json.SourceGeneration.Tests
         [JsonSerializable(typeof(PrivateParameterizedCtor_WithAttribute))]
         [JsonSerializable(typeof(InternalParameterizedCtor_WithAttribute))]
         [JsonSerializable(typeof(ProtectedParameterizedCtor_WithAttribute))]
+        [JsonSerializable(typeof(PrivateParameterlessCtor_WithAttribute))]
+        [JsonSerializable(typeof(InternalParameterlessCtor_WithAttribute))]
+        [JsonSerializable(typeof(ProtectedParameterlessCtor_WithAttribute))]
         [JsonSerializable(typeof(SinglePublicParameterizedCtor))]
         [JsonSerializable(typeof(SingleParameterlessCtor_MultiplePublicParameterizedCtor))]
         [JsonSerializable(typeof(SingleParameterlessCtor_MultiplePublicParameterizedCtor_Struct))]
@@ -186,6 +189,9 @@ namespace System.Text.Json.SourceGeneration.Tests
         [JsonSerializable(typeof(PrivateParameterizedCtor_WithAttribute))]
         [JsonSerializable(typeof(InternalParameterizedCtor_WithAttribute))]
         [JsonSerializable(typeof(ProtectedParameterizedCtor_WithAttribute))]
+        [JsonSerializable(typeof(PrivateParameterlessCtor_WithAttribute))]
+        [JsonSerializable(typeof(InternalParameterlessCtor_WithAttribute))]
+        [JsonSerializable(typeof(ProtectedParameterlessCtor_WithAttribute))]
         [JsonSerializable(typeof(SinglePublicParameterizedCtor))]
         [JsonSerializable(typeof(SingleParameterlessCtor_MultiplePublicParameterizedCtor))]
         [JsonSerializable(typeof(SingleParameterlessCtor_MultiplePublicParameterizedCtor_Struct))]