Cleanup JsonTypeInfo construction (#67700)
authorKrzysztof Wicher <kwicher@microsoft.com>
Thu, 7 Apr 2022 15:07:43 +0000 (17:07 +0200)
committerGitHub <noreply@github.com>
Thu, 7 Apr 2022 15:07:43 +0000 (17:07 +0200)
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs

index 4c97708..2f12221 100644 (file)
@@ -59,7 +59,7 @@ namespace System.Text.Json
                     // Some of the validation is done during construction (i.e. validity of JsonConverter, inner types etc.)
                     // therefore we need to unwrap TargetInvocationException for better user experience
                     ExceptionDispatchInfo.Capture(ex.InnerException).Throw();
-                    throw ex.InnerException;
+                    throw null!;
                 }
 #endif
             }
index 1184d9c..c59a849 100644 (file)
@@ -81,19 +81,7 @@ namespace System.Text.Json.Serialization.Metadata
         public static JsonTypeInfo<T> CreateValueInfo<T>(JsonSerializerOptions options, JsonConverter converter)
         {
             JsonTypeInfo<T> info = new SourceGenJsonTypeInfo<T>(converter, options);
-            info.PropertyInfoForTypeInfo = CreateJsonPropertyInfoForClassInfo(typeof(T), info, converter, options);
             return info;
         }
-
-        internal static JsonPropertyInfo CreateJsonPropertyInfoForClassInfo(
-            Type type,
-            JsonTypeInfo typeInfo,
-            JsonConverter converter,
-            JsonSerializerOptions options)
-        {
-            JsonPropertyInfo propertyInfo = converter.CreateJsonPropertyInfo();
-            propertyInfo.InitializeForTypeInfo(type, typeInfo, converter, options);
-            return propertyInfo;
-        }
     }
 }
index 812c500..0c6f500 100644 (file)
@@ -90,24 +90,21 @@ namespace System.Text.Json.Serialization.Metadata
             _isConfigured = true;
         }
 
-        internal virtual void GetPolicies(JsonIgnoreCondition? ignoreCondition)
+        internal void GetPolicies(JsonIgnoreCondition? ignoreCondition)
         {
-            if (!IsForTypeInfo)
-            {
-                Debug.Assert(MemberInfo != null);
-                DetermineSerializationCapabilities(ignoreCondition);
-                DeterminePropertyName();
-                DetermineIgnoreCondition(ignoreCondition);
-
-                JsonPropertyOrderAttribute? orderAttr = GetAttribute<JsonPropertyOrderAttribute>(MemberInfo);
-                if (orderAttr != null)
-                {
-                    Order = orderAttr.Order;
-                }
+            Debug.Assert(MemberInfo != null);
+            DetermineSerializationCapabilities(ignoreCondition);
+            DeterminePropertyName();
+            DetermineIgnoreCondition(ignoreCondition);
 
-                JsonNumberHandlingAttribute? attribute = GetAttribute<JsonNumberHandlingAttribute>(MemberInfo);
-                NumberHandling = attribute?.Handling;
+            JsonPropertyOrderAttribute? orderAttr = GetAttribute<JsonPropertyOrderAttribute>(MemberInfo);
+            if (orderAttr != null)
+            {
+                Order = orderAttr.Order;
             }
+
+            JsonNumberHandlingAttribute? attribute = GetAttribute<JsonNumberHandlingAttribute>(MemberInfo);
+            NumberHandling = attribute?.Handling;
         }
 
         private void DeterminePropertyName()
@@ -330,7 +327,7 @@ namespace System.Text.Json.Serialization.Metadata
         internal bool HasGetter { get; set; }
         internal bool HasSetter { get; set; }
 
-        internal virtual void Initialize(
+        internal abstract void Initialize(
             Type parentClassType,
             Type declaredPropertyType,
             ConverterStrategy converterStrategy,
@@ -338,25 +335,8 @@ namespace System.Text.Json.Serialization.Metadata
             bool isVirtual,
             JsonConverter converter,
             JsonIgnoreCondition? ignoreCondition,
-            JsonNumberHandling? parentTypeNumberHandling,
-            JsonSerializerOptions options)
-        {
-            Debug.Assert(converter != null);
-
-            DeclaringType = parentClassType;
-            PropertyType = declaredPropertyType;
-            ConverterStrategy = converterStrategy;
-            MemberInfo = memberInfo;
-            IsVirtual = isVirtual;
-            ConverterBase = converter;
-            Options = options;
-        }
-
-        internal abstract void InitializeForTypeInfo(
-            Type declaredType,
-            JsonTypeInfo runtimeTypeInfo,
-            JsonConverter converter,
-            JsonSerializerOptions options);
+            JsonSerializerOptions options,
+            JsonTypeInfo? jsonTypeInfo = null);
 
         internal bool IgnoreDefaultValuesOnRead { get; private set; }
         internal bool IgnoreDefaultValuesOnWrite { get; private set; }
@@ -483,7 +463,7 @@ namespace System.Text.Json.Serialization.Metadata
 
         internal Type DeclaringType { get; set; } = null!;
 
-        internal MemberInfo? MemberInfo { get; private set; }
+        internal MemberInfo? MemberInfo { get; set; }
 
         internal JsonTypeInfo JsonTypeInfo
         {
index 0f257bd..321c50f 100644 (file)
@@ -43,78 +43,89 @@ namespace System.Text.Json.Serialization.Metadata
             bool isVirtual,
             JsonConverter converter,
             JsonIgnoreCondition? ignoreCondition,
-            JsonNumberHandling? parentTypeNumberHandling,
-            JsonSerializerOptions options)
+            JsonSerializerOptions options,
+            JsonTypeInfo? jsonTypeInfo = null)
         {
-            base.Initialize(
-                parentClassType,
-                declaredPropertyType,
-                converterStrategy,
-                memberInfo,
-                isVirtual,
-                converter,
-                ignoreCondition,
-                parentTypeNumberHandling,
-                options);
-
-            switch (memberInfo)
+            Debug.Assert(converter != null);
+
+            PropertyType = declaredPropertyType;
+            ConverterStrategy = converterStrategy;
+            if (jsonTypeInfo != null)
             {
-                case PropertyInfo propertyInfo:
-                    {
-                        bool useNonPublicAccessors = GetAttribute<JsonIncludeAttribute>(propertyInfo) != null;
+                JsonTypeInfo = jsonTypeInfo;
+            }
 
-                        MethodInfo? getMethod = propertyInfo.GetMethod;
-                        if (getMethod != null && (getMethod.IsPublic || useNonPublicAccessors))
-                        {
-                            HasGetter = true;
-                            Get = options.MemberAccessorStrategy.CreatePropertyGetter<T>(propertyInfo);
-                        }
+            ConverterBase = converter;
+            Options = options;
+            DeclaringType = parentClassType;
+            MemberInfo = memberInfo;
+            IsVirtual = isVirtual;
 
-                        MethodInfo? setMethod = propertyInfo.SetMethod;
-                        if (setMethod != null && (setMethod.IsPublic || useNonPublicAccessors))
+            if (memberInfo != null)
+            {
+                switch (memberInfo)
+                {
+                    case PropertyInfo propertyInfo:
                         {
-                            HasSetter = true;
-                            Set = options.MemberAccessorStrategy.CreatePropertySetter<T>(propertyInfo);
-                        }
+                            bool useNonPublicAccessors = GetAttribute<JsonIncludeAttribute>(propertyInfo) != null;
 
-                        MemberType = MemberTypes.Property;
+                            MethodInfo? getMethod = propertyInfo.GetMethod;
+                            if (getMethod != null && (getMethod.IsPublic || useNonPublicAccessors))
+                            {
+                                HasGetter = true;
+                                Get = options.MemberAccessorStrategy.CreatePropertyGetter<T>(propertyInfo);
+                            }
 
-                        break;
-                    }
+                            MethodInfo? setMethod = propertyInfo.SetMethod;
+                            if (setMethod != null && (setMethod.IsPublic || useNonPublicAccessors))
+                            {
+                                HasSetter = true;
+                                Set = options.MemberAccessorStrategy.CreatePropertySetter<T>(propertyInfo);
+                            }
 
-                case FieldInfo fieldInfo:
-                    {
-                        Debug.Assert(fieldInfo.IsPublic);
+                            MemberType = MemberTypes.Property;
 
-                        HasGetter = true;
-                        Get = options.MemberAccessorStrategy.CreateFieldGetter<T>(fieldInfo);
+                            break;
+                        }
 
-                        if (!fieldInfo.IsInitOnly)
+                    case FieldInfo fieldInfo:
                         {
-                            HasSetter = true;
-                            Set = options.MemberAccessorStrategy.CreateFieldSetter<T>(fieldInfo);
-                        }
+                            Debug.Assert(fieldInfo.IsPublic);
 
-                        MemberType = MemberTypes.Field;
+                            HasGetter = true;
+                            Get = options.MemberAccessorStrategy.CreateFieldGetter<T>(fieldInfo);
 
-                        break;
-                    }
+                            if (!fieldInfo.IsInitOnly)
+                            {
+                                HasSetter = true;
+                                Set = options.MemberAccessorStrategy.CreateFieldSetter<T>(fieldInfo);
+                            }
 
-                default:
-                    {
-                        IsForTypeInfo = true;
-                        HasGetter = true;
-                        HasSetter = true;
+                            MemberType = MemberTypes.Field;
 
-                        break;
-                    }
-            }
+                            break;
+                        }
 
-            _converterIsExternalAndPolymorphic = !converter.IsInternalConverter && PropertyType != converter.TypeToConvert;
-            PropertyTypeCanBeNull = PropertyType.CanBeNull();
-            _propertyTypeEqualsTypeToConvert = typeof(T) == PropertyType;
+                    default:
+                        {
+                            Debug.Fail($"Invalid memberInfo type: {memberInfo.GetType().FullName}");
+                            break;
+                        }
+                }
+
+                // TODO (perf): can we pre-compute some of these values during source gen?
+                _converterIsExternalAndPolymorphic = !converter.IsInternalConverter && PropertyType != converter.TypeToConvert;
+                PropertyTypeCanBeNull = PropertyType.CanBeNull();
+                _propertyTypeEqualsTypeToConvert = typeof(T) == PropertyType;
 
-            GetPolicies(ignoreCondition);
+                GetPolicies(ignoreCondition);
+            }
+            else
+            {
+                IsForTypeInfo = true;
+                HasGetter = true;
+                HasSetter = true;
+            }
         }
 
         internal void InitializeForSourceGen(JsonSerializerOptions options, JsonPropertyInfoValues<T> propertyInfo)
@@ -189,30 +200,6 @@ namespace System.Text.Json.Serialization.Metadata
             }
         }
 
-        /// <summary>
-        /// Create a <see cref="JsonPropertyInfo"/> for a given Type.
-        /// See <seealso cref="JsonTypeInfo.PropertyInfoForTypeInfo"/>.
-        /// </summary>
-        internal override void InitializeForTypeInfo(
-            Type declaredType,
-            JsonTypeInfo runtimeTypeInfo,
-            JsonConverter converter,
-            JsonSerializerOptions options)
-        {
-            PropertyType = declaredType;
-            ConverterStrategy = converter.ConverterStrategy;
-            JsonTypeInfo = runtimeTypeInfo;
-            ConverterBase = converter;
-            Options = options;
-            IsForTypeInfo = true;
-            HasGetter = true;
-            HasSetter = true;
-            // TODO (perf): can we pre-compute some of these values during source gen?
-            _converterIsExternalAndPolymorphic = !converter.IsInternalConverter && declaredType != converter.TypeToConvert;
-            PropertyTypeCanBeNull = declaredType.CanBeNull();
-            _propertyTypeEqualsTypeToConvert = typeof(T) == declaredType;
-        }
-
         internal override JsonConverter ConverterBase
         {
             get
index 73bcbef..b5e5624 100644 (file)
@@ -58,8 +58,8 @@ namespace System.Text.Json.Serialization.Metadata
             bool isVirtual,
             JsonConverter converter,
             JsonSerializerOptions options,
-            JsonNumberHandling? parentTypeNumberHandling = null,
-            JsonIgnoreCondition? ignoreCondition = null)
+            JsonIgnoreCondition? ignoreCondition = null,
+            JsonTypeInfo? jsonTypeInfo = null)
         {
             // Create the JsonPropertyInfo instance.
             JsonPropertyInfo jsonPropertyInfo = converter.CreateJsonPropertyInfo();
@@ -72,8 +72,8 @@ namespace System.Text.Json.Serialization.Metadata
                 isVirtual,
                 converter,
                 ignoreCondition,
-                parentTypeNumberHandling,
-                options);
+                options,
+                jsonTypeInfo);
 
             return jsonPropertyInfo;
         }
@@ -85,17 +85,17 @@ namespace System.Text.Json.Serialization.Metadata
         private static JsonPropertyInfo CreatePropertyInfoForTypeInfo(
             Type declaredPropertyType,
             JsonConverter converter,
-            JsonNumberHandling? numberHandling,
-            JsonSerializerOptions options)
+            JsonSerializerOptions options,
+            JsonTypeInfo? jsonTypeInfo = null)
         {
             JsonPropertyInfo jsonPropertyInfo = CreateProperty(
                 declaredPropertyType: declaredPropertyType,
                 memberInfo: null, // Not a real property so this is null.
                 parentClassType: ObjectType, // a dummy value (not used)
                 isVirtual: false,
-                converter,
-                options,
-                parentTypeNumberHandling: numberHandling);
+                converter: converter,
+                options: options,
+                jsonTypeInfo: jsonTypeInfo);
 
             Debug.Assert(jsonPropertyInfo.IsForTypeInfo);
 
index baf4290..f009137 100644 (file)
@@ -152,7 +152,7 @@ namespace System.Text.Json.Serialization.Metadata
         /// TypeInfo (for the cases mentioned above). In addition, methods that have a JsonPropertyInfo argument would also likely
         /// need to add an argument for JsonTypeInfo.
         /// </remarks>
-        internal JsonPropertyInfo PropertyInfoForTypeInfo { get; set; }
+        internal JsonPropertyInfo PropertyInfoForTypeInfo { get; private set; }
 
         /// <summary>
         /// Returns a helper class used for computing the default value.
@@ -162,24 +162,11 @@ namespace System.Text.Json.Serialization.Metadata
 
         internal JsonNumberHandling? NumberHandling { get; set; }
 
-        internal JsonTypeInfo()
-        {
-            Debug.Assert(false, "This constructor should not be called.");
-        }
-
-        internal JsonTypeInfo(Type type, JsonSerializerOptions options!!)
-        {
-            Type = type;
-            Options = options;
-            // Setting this option is deferred to the initialization methods of the various metadada info types.
-            PropertyInfoForTypeInfo = null!;
-        }
-
         internal JsonTypeInfo(Type type, JsonConverter converter, JsonSerializerOptions options)
         {
             Type = type;
             Options = options;
-            PropertyInfoForTypeInfo = CreatePropertyInfoForTypeInfo(Type, converter, NumberHandling, Options);
+            PropertyInfoForTypeInfo = CreatePropertyInfoForTypeInfo(Type, converter, Options, this);
             ElementType = converter.ElementType;
 
             switch (PropertyInfoForTypeInfo.ConverterStrategy)
index 6e4a5a4..f5e3e34 100644 (file)
@@ -15,19 +15,10 @@ namespace System.Text.Json.Serialization.Metadata
     {
         private Action<Utf8JsonWriter, T>? _serialize;
 
-        internal JsonTypeInfo(Type type, JsonSerializerOptions options) :
-            base(type, options)
-        { }
-
         internal JsonTypeInfo(JsonConverter converter, JsonSerializerOptions options)
             : base(typeof(T), converter, options)
         { }
 
-        internal JsonTypeInfo()
-        {
-            Debug.Assert(false, "This constructor should not be called.");
-        }
-
         /// <summary>
         /// Serializes an instance of <typeparamref name="T"/> using
         /// <see cref="JsonSourceGenerationOptionsAttribute"/> values specified at design time.
index ee1df13..b69c63a 100644 (file)
@@ -176,7 +176,7 @@ namespace System.Text.Json.Serialization.Metadata
                 ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateTypeAttribute(Type, typeof(JsonExtensionDataAttribute));
             }
 
-            JsonPropertyInfo jsonPropertyInfo = AddProperty(memberInfo, memberType, declaringType, isVirtual, typeNumberHandling, Options);
+            JsonPropertyInfo jsonPropertyInfo = AddProperty(memberInfo, memberType, declaringType, isVirtual, Options);
             Debug.Assert(jsonPropertyInfo.NameAsString != null);
 
             if (hasExtensionAttribute)
@@ -197,7 +197,6 @@ namespace System.Text.Json.Serialization.Metadata
             Type memberType,
             Type parentClassType,
             bool isVirtual,
-            JsonNumberHandling? parentTypeNumberHandling,
             JsonSerializerOptions options)
         {
             JsonIgnoreCondition? ignoreCondition = JsonPropertyInfo.GetAttribute<JsonIgnoreAttribute>(memberInfo)?.Condition;
@@ -221,7 +220,6 @@ namespace System.Text.Json.Serialization.Metadata
                 isVirtual,
                 converter,
                 options,
-                parentTypeNumberHandling,
                 ignoreCondition);
         }
 
index 6dc1a5b..c9e5ea8 100644 (file)
@@ -18,40 +18,28 @@ namespace System.Text.Json.Serialization.Metadata
         /// Creates serialization metadata for a type using a simple converter.
         /// </summary>
         public SourceGenJsonTypeInfo(JsonConverter converter, JsonSerializerOptions options)
-            : base(typeof(T), options)
+            : base(converter, options)
         {
-            ElementType = converter.ElementType;
         }
 
         /// <summary>
         /// Creates serialization metadata for an object.
         /// </summary>
-        public SourceGenJsonTypeInfo(JsonSerializerOptions options, JsonObjectInfoValues<T> objectInfo) : base(typeof(T), options)
+        public SourceGenJsonTypeInfo(JsonSerializerOptions options, JsonObjectInfoValues<T> objectInfo) : base(GetConverter(objectInfo), options)
         {
-#pragma warning disable CS8714
-            // The type cannot be used as type parameter in the generic type or method.
-            // Nullability of type argument doesn't match 'notnull' constraint.
-            JsonConverter converter;
-
             if (objectInfo.ObjectWithParameterizedConstructorCreator != null)
             {
-                converter = new JsonMetadataServicesConverter<T>(
-                    () => new LargeObjectWithParameterizedConstructorConverter<T>(),
-                    ConverterStrategy.Object);
                 CreateObjectWithArgs = objectInfo.ObjectWithParameterizedConstructorCreator;
                 CtorParamInitFunc = objectInfo.ConstructorParameterMetadataInitializer;
             }
             else
             {
-                converter = new JsonMetadataServicesConverter<T>(() => new ObjectDefaultConverter<T>(), ConverterStrategy.Object);
                 SetCreateObjectFunc(objectInfo.ObjectCreator);
             }
-#pragma warning restore CS8714
+
 
             PropInitFunc = objectInfo.PropertyMetadataInitializer;
-            ElementType = converter.ElementType;
             SerializeHandler = objectInfo.SerializeHandler;
-            PropertyInfoForTypeInfo = JsonMetadataServices.CreateJsonPropertyInfoForClassInfo(typeof(T), this, converter, Options);
             NumberHandling = objectInfo.NumberHandling;
         }
 
@@ -64,23 +52,41 @@ namespace System.Text.Json.Serialization.Metadata
             Func<JsonConverter<T>> converterCreator,
             object? createObjectWithArgs = null,
             object? addFunc = null)
-            : base(typeof(T), options)
+            : base(GetConverter(collectionInfo, converterCreator), options)
         {
-            ConverterStrategy strategy = collectionInfo.KeyInfo == null ? ConverterStrategy.Enumerable : ConverterStrategy.Dictionary;
-            JsonConverter<T> converter = new JsonMetadataServicesConverter<T>(converterCreator, strategy);
-
-            KeyType = converter.KeyType;
-            ElementType = converter.ElementType;
             KeyTypeInfo = collectionInfo.KeyInfo;
             ElementTypeInfo = collectionInfo.ElementInfo ?? throw new ArgumentNullException(nameof(collectionInfo.ElementInfo));
             NumberHandling = collectionInfo.NumberHandling;
-            PropertyInfoForTypeInfo = JsonMetadataServices.CreateJsonPropertyInfoForClassInfo(typeof(T), this, converter, options);
             SerializeHandler = collectionInfo.SerializeHandler;
             CreateObjectWithArgs = createObjectWithArgs;
             AddMethodDelegate = addFunc;
             SetCreateObjectFunc(collectionInfo.ObjectCreator);
         }
 
+        private static JsonConverter GetConverter(JsonObjectInfoValues<T> objectInfo)
+        {
+#pragma warning disable CS8714
+            // The type cannot be used as type parameter in the generic type or method.
+            // Nullability of type argument doesn't match 'notnull' constraint.
+            if (objectInfo.ObjectWithParameterizedConstructorCreator != null)
+            {
+                return new JsonMetadataServicesConverter<T>(
+                    () => new LargeObjectWithParameterizedConstructorConverter<T>(),
+                    ConverterStrategy.Object);
+            }
+            else
+            {
+                return new JsonMetadataServicesConverter<T>(() => new ObjectDefaultConverter<T>(), ConverterStrategy.Object);
+            }
+#pragma warning restore CS8714
+        }
+
+        private static JsonConverter GetConverter(JsonCollectionInfoValues<T> collectionInfo, Func<JsonConverter<T>> converterCreator)
+        {
+            ConverterStrategy strategy = collectionInfo.KeyInfo == null ? ConverterStrategy.Enumerable : ConverterStrategy.Dictionary;
+            return new JsonMetadataServicesConverter<T>(converterCreator, strategy);
+        }
+
         internal override void LateAddProperties()
         {
             AddPropertiesUsingSourceGenInfo();