Remove fallback to reflection converter resolution (#72228)

* Remove fallback to reflection converter resolution

* Address feedback

* Use JsonTypeInfo instead of JsonConverter when resolving JsonPropertyInfo & EffectiveConverter

* Fix rare test failure caused by using a locked instance with a shared caching context.

* Address feedback

* Revert "Fix rare test failure caused by using a locked instance with a shared caching context."

This reverts commit 4f10c60.

* minor cleanup

src/libraries/System.Text.Json/src/Resources/Strings.resx

@@ -581,9 +581,6 @@
   <data name="ConverterForPropertyMustBeValid" xml:space="preserve">
     <value>The generic type of the converter for property '{0}.{1}' must match with the specified converter type '{2}'. The converter must not be 'null'.</value>
   </data>
-  <data name="BuiltInConvertersNotRooted" xml:space="preserve">
-    <value>Built-in type converters have not been initialized. There is no converter available for type '{0}'. To root all built-in converters, use a 'JsonSerializerOptions'-based method of the 'JsonSerializer'.</value>
-  </data>
   <data name="NoMetadataForType" xml:space="preserve">
     <value>Metadata for type '{0}' was not provided by TypeInfoResolver of type '{1}'. If using source generation, ensure that all root types passed to the serializer have been indicated with 'JsonSerializableAttribute', along with any types that might be serialized polymorphically.</value>
   </data>

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/FSharp/FSharpTypeConverterFactory.cs

@@ -37,12 +37,12 @@ public override bool CanConvert(Type typeToConvert) =>
                 case FSharpKind.Option:
                     elementType = typeToConvert.GetGenericArguments()[0];
                     converterFactoryType = typeof(FSharpOptionConverter<,>).MakeGenericType(typeToConvert, elementType);
-                    constructorArguments = new object[] { options.GetConverterFromTypeInfo(elementType) };
+                    constructorArguments = new object[] { options.GetConverterInternal(elementType) };
                     break;
                 case FSharpKind.ValueOption:
                     elementType = typeToConvert.GetGenericArguments()[0];
                     converterFactoryType = typeof(FSharpValueOptionConverter<,>).MakeGenericType(typeToConvert, elementType);
-                    constructorArguments = new object[] { options.GetConverterFromTypeInfo(elementType) };
+                    constructorArguments = new object[] { options.GetConverterInternal(elementType) };
                     break;
                 case FSharpKind.List:
                     elementType = typeToConvert.GetGenericArguments()[0];

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectConverter.cs

@@ -86,7 +86,7 @@ internal override void WriteAsPropertyNameCore(Utf8JsonWriter writer, object? va
             Debug.Assert(value != null);
 
             Type runtimeType = value.GetType();
-            JsonConverter runtimeConverter = options.GetConverterFromTypeInfo(runtimeType);
+            JsonConverter runtimeConverter = options.GetConverterInternal(runtimeType);
             if (runtimeConverter == this)
             {
                 ThrowHelper.ThrowNotSupportedException_DictionaryKeyTypeNotSupported(runtimeType, this);

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/NullableConverterFactory.cs

@@ -22,7 +22,7 @@ public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializer
 
             Type valueTypeToConvert = typeToConvert.GetGenericArguments()[0];
 
-            JsonConverter valueConverter = options.GetConverterFromTypeInfo(valueTypeToConvert);
+            JsonConverter valueConverter = options.GetConverterInternal(valueTypeToConvert);
             Debug.Assert(valueConverter != null);
 
             // If the value type has an interface or object converter, just return that converter directly.

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs

@@ -67,8 +67,6 @@ internal virtual void ReadElementAndSetProperty(
             throw new InvalidOperationException(SR.NodeJsonObjectCustomConverterNotAllowedOnExtensionProperty);
         }
 
-        internal abstract JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo declaringTypeInfo, JsonSerializerOptions options);
-
         internal abstract JsonParameterInfo CreateJsonParameterInfo();
 
         internal abstract JsonConverter<TTarget> CreateCastingConverter<TTarget>();

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs

@@ -32,13 +32,6 @@ protected JsonConverterFactory() { }
         /// </returns>
         public abstract JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options);
 
-        internal override JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo declaringTypeInfo, JsonSerializerOptions options)
-        {
-            Debug.Fail("We should never get here.");
-
-            throw new InvalidOperationException();
-        }
-
         internal override JsonParameterInfo CreateJsonParameterInfo()
         {
             Debug.Fail("We should never get here.");

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs

@@ -69,14 +69,6 @@ public override bool CanConvert(Type typeToConvert)
 
         internal override ConverterStrategy ConverterStrategy => ConverterStrategy.Value;
 
-        internal sealed override JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo declaringTypeInfo, JsonSerializerOptions options)
-        {
-            return new JsonPropertyInfo<T>(declaringTypeInfo.Type, declaringTypeInfo, options)
-            {
-                DefaultConverterForType = this
-            };
-        }
-
         internal sealed override JsonParameterInfo CreateJsonParameterInfo()
         {
             return new JsonParameterInfo<T>();

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs

@@ -45,44 +45,34 @@ public JsonTypeInfo GetTypeInfo(Type type)
                 ThrowHelper.ThrowArgumentException_CannotSerializeInvalidType(nameof(type), type, null, null);
             }
 
-            JsonTypeInfo? typeInfo;
-            if (IsLockedInstance)
-            {
-                typeInfo = GetCachingContext()?.GetOrAddJsonTypeInfo(type);
-                typeInfo?.EnsureConfigured();
-            }
-            else
-            {
-                typeInfo = GetTypeInfoNoCaching(type);
-            }
-
-            if (typeInfo is null)
-            {
-                ThrowHelper.ThrowNotSupportedException_NoMetadataForType(type, TypeInfoResolver);
-            }
-
-            return typeInfo;
+            return GetTypeInfoInternal(type, resolveIfMutable: true);
         }
 
         /// <summary>
-        /// This method returns configured non-null JsonTypeInfo
+        /// Same as GetTypeInfo but without validation and additional knobs.
         /// </summary>
-        internal JsonTypeInfo GetTypeInfoCached(Type type)
+        internal JsonTypeInfo GetTypeInfoInternal(Type type, bool ensureConfigured = true, bool resolveIfMutable = false)
         {
             JsonTypeInfo? typeInfo = null;
 
             if (IsLockedInstance)
             {
                 typeInfo = GetCachingContext()?.GetOrAddJsonTypeInfo(type);
+                if (ensureConfigured)
+                {
+                    typeInfo?.EnsureConfigured();
+                }
+            }
+            else if (resolveIfMutable)
+            {
+                typeInfo = GetTypeInfoNoCaching(type);
             }
 
             if (typeInfo == null)
             {
                 ThrowHelper.ThrowNotSupportedException_NoMetadataForType(type, TypeInfoResolver);
-                return null;
             }
 
-            typeInfo.EnsureConfigured();
             return typeInfo;
         }
 
@@ -108,7 +98,7 @@ internal JsonTypeInfo GetTypeInfoForRootType(Type type)
 
             if (jsonTypeInfo?.Type != type)
             {
-                jsonTypeInfo = GetTypeInfoCached(type);
+                jsonTypeInfo = GetTypeInfoInternal(type);
                 _lastTypeInfo = jsonTypeInfo;
             }
 
@@ -125,12 +115,7 @@ internal void ClearCaches()
         {
             Debug.Assert(IsLockedInstance);
 
-            if (_cachingContext is null && _typeInfoResolver is not null)
-            {
-                _cachingContext = TrackedCachingContexts.GetOrCreate(this);
-            }
-
-            return _cachingContext;
+            return _cachingContext ??= TrackedCachingContexts.GetOrCreate(this);
         }
 
         /// <summary>

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs

@@ -2,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Text.Json.Reflection;
 using System.Text.Json.Serialization;
@@ -48,35 +47,21 @@ public JsonConverter GetConverter(Type typeToConvert)
 
             if (_typeInfoResolver is null)
             {
-                // Backward compatibility -- root the default reflection converters
+                // Backward compatibility -- root & query the default reflection converters
                 // but do not populate the TypeInfoResolver setting.
-                DefaultJsonTypeInfoResolver.RootDefaultInstance();
+                return DefaultJsonTypeInfoResolver.GetConverterForType(typeToConvert, this);
             }
 
-            return GetConverterFromTypeInfo(typeToConvert);
+            return GetConverterInternal(typeToConvert);
         }
 
         /// <summary>
-        /// Same as GetConverter but does not root converters
+        /// Same as GetConverter but without defaulting to reflection converters.
         /// </summary>
-        internal JsonConverter GetConverterFromTypeInfo(Type typeToConvert)
+        internal JsonConverter GetConverterInternal(Type typeToConvert)
         {
-            JsonConverter? converter;
-
-            if (IsLockedInstance)
-            {
-                converter = GetCachingContext()?.GetOrAddJsonTypeInfo(typeToConvert)?.Converter;
-            }
-            else
-            {
-                // We do not want to lock options instance here but we need to return correct answer
-                // which means we need to go through TypeInfoResolver but without caching because that's the
-                // only place which will have correct converter for JsonSerializerContext and reflection
-                // based resolver. It will also work correctly for combined resolvers.
-                converter = GetTypeInfoNoCaching(typeToConvert)?.Converter;
-            }
-
-            return converter ?? GetConverterFromListOrBuiltInConverter(typeToConvert);
+            JsonTypeInfo jsonTypeInfo = GetTypeInfoInternal(typeToConvert, ensureConfigured: false, resolveIfMutable: true);
+            return jsonTypeInfo.Converter;
         }
 
         internal JsonConverter? GetConverterFromList(Type typeToConvert)
@@ -92,28 +77,6 @@ internal JsonConverter GetConverterFromTypeInfo(Type typeToConvert)
             return null;
         }
 
-        internal JsonConverter GetConverterFromListOrBuiltInConverter(Type typeToConvert)
-        {
-            JsonConverter? converter = GetConverterFromList(typeToConvert);
-            return GetCustomOrBuiltInConverter(typeToConvert, converter);
-        }
-
-        internal JsonConverter GetCustomOrBuiltInConverter(Type typeToConvert, JsonConverter? converter)
-        {
-            // Attempt to get built-in converter.
-            converter ??= DefaultJsonTypeInfoResolver.GetBuiltInConverter(typeToConvert);
-            // Expand potential convert converter factory.
-            converter = ExpandConverterFactory(converter, typeToConvert);
-
-            if (!converter.TypeToConvert.IsInSubtypeRelationshipWith(typeToConvert))
-            {
-                ThrowHelper.ThrowInvalidOperationException_SerializationConverterNotCompatible(converter.GetType(), converter.TypeToConvert);
-            }
-
-            CheckConverterNullabilityIsSameAsPropertyType(converter, typeToConvert);
-            return converter;
-        }
-
         [return: NotNullIfNotNull("converter")]
         internal JsonConverter? ExpandConverterFactory(JsonConverter? converter, Type typeToConvert)
         {

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

@@ -18,8 +18,8 @@ internal sealed class CustomJsonTypeInfo<T> : JsonTypeInfo<T>
         /// <summary>
         /// Creates serialization metadata for a type using a simple converter.
         /// </summary>
-        internal CustomJsonTypeInfo(JsonSerializerOptions options)
-            : base(options.GetConverterFromListOrBuiltInConverter(typeof(T)), options)
+        internal CustomJsonTypeInfo(JsonConverter converter, JsonSerializerOptions options)
+            : base(converter, options)
         {
         }
 

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

@@ -81,16 +81,10 @@ void Add(JsonConverter converter) =>
                 converters.Add(converter.TypeToConvert, converter);
         }
 
-        internal static JsonConverter GetBuiltInConverter(Type typeToConvert)
+        private static JsonConverter GetBuiltInConverter(Type typeToConvert)
         {
-            if (s_defaultSimpleConverters == null || s_defaultFactoryConverters == null)
-            {
-                // (De)serialization using serializer's options-based methods has not yet occurred, so the built-in converters are not rooted.
-                // Even though source-gen code paths do not call this method <i.e. JsonSerializerOptions.GetConverter(Type)>, we do not root all the
-                // built-in converters here since we fetch converters for any type included for source generation from the binded context (Priority 1).
-                ThrowHelper.ThrowNotSupportedException_BuiltInConvertersNotRooted(typeToConvert);
-                return null!;
-            }
+            Debug.Assert(s_defaultSimpleConverters != null);
+            Debug.Assert(s_defaultFactoryConverters != null);
 
             JsonConverter? converter;
             if (s_defaultSimpleConverters.TryGetValue(typeToConvert, out converter))
@@ -99,11 +93,11 @@ internal static JsonConverter GetBuiltInConverter(Type typeToConvert)
             }
             else
             {
-                foreach (JsonConverter item in s_defaultFactoryConverters)
+                foreach (JsonConverterFactory factory in s_defaultFactoryConverters)
                 {
-                    if (item.CanConvert(typeToConvert))
+                    if (factory.CanConvert(typeToConvert))
                     {
-                        converter = item;
+                        converter = factory;
                         break;
                     }
                 }
@@ -125,39 +119,28 @@ internal static bool TryGetDefaultSimpleConverter(Type typeToConvert, [NotNullWh
             return s_defaultSimpleConverters.TryGetValue(typeToConvert, out converter);
         }
 
-        // This method gets the runtime information for a given type or property.
-        // The runtime information consists of the following:
-        // - class type,
-        // - element type (if the type is a collection),
-        // - the converter (either native or custom), if one exists.
         [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
         [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
-        internal static JsonConverter GetConverterForMember(
-            Type typeToConvert,
-            MemberInfo memberInfo,
-            JsonSerializerOptions options,
-            out JsonConverter? customConverter)
+        internal static JsonConverter? GetCustomConverterForMember(Type typeToConvert, MemberInfo memberInfo, JsonSerializerOptions options)
         {
             Debug.Assert(memberInfo is FieldInfo or PropertyInfo);
             Debug.Assert(typeToConvert != null);
 
             JsonConverterAttribute? converterAttribute = memberInfo.GetUniqueCustomAttribute<JsonConverterAttribute>(inherit: false);
-            customConverter = converterAttribute is null ? null : GetConverterFromAttribute(converterAttribute, typeToConvert, memberInfo, options);
-
-            return options.TryGetTypeInfoCached(typeToConvert, out JsonTypeInfo? typeInfo)
-                ? typeInfo.Converter
-                : GetConverterForType(typeToConvert, options);
+            return converterAttribute is null ? null : GetConverterFromAttribute(converterAttribute, typeToConvert, memberInfo, options);
         }
 
         [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
         [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
-        internal static JsonConverter GetConverterForType(Type typeToConvert, JsonSerializerOptions options)
+        internal static JsonConverter GetConverterForType(Type typeToConvert, JsonSerializerOptions options, bool resolveJsonConverterAttribute = true)
         {
+            RootDefaultInstance(); // Ensure default converters are rooted.
+
             // Priority 1: Attempt to get custom converter from the Converters list.
             JsonConverter? converter = options.GetConverterFromList(typeToConvert);
 
             // Priority 2: Attempt to get converter from [JsonConverter] on the type being converted.
-            if (converter == null)
+            if (resolveJsonConverterAttribute && converter == null)
             {
                 JsonConverterAttribute? converterAttribute = typeToConvert.GetUniqueCustomAttribute<JsonConverterAttribute>(inherit: false);
                 if (converterAttribute != null)
@@ -166,8 +149,18 @@ internal static JsonConverter GetConverterForType(Type typeToConvert, JsonSerial
                 }
             }
 
-            // Priority 3: Fall back to built-in converters and validate result
-            return options.GetCustomOrBuiltInConverter(typeToConvert, converter);
+            // Priority 3: Query the built-in converters.
+            converter ??= GetBuiltInConverter(typeToConvert);
+
+            // Expand if factory converter & validate.
+            converter = options.ExpandConverterFactory(converter, typeToConvert);
+            if (!converter.TypeToConvert.IsInSubtypeRelationshipWith(typeToConvert))
+            {
+                ThrowHelper.ThrowInvalidOperationException_SerializationConverterNotCompatible(converter.GetType(), converter.TypeToConvert);
+            }
+
+            JsonSerializerOptions.CheckConverterNullabilityIsSameAsPropertyType(converter, typeToConvert);
+            return converter;
         }
 
         [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]

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

@@ -90,7 +90,11 @@ private JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions options
 
         [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
         [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
-        private static JsonTypeInfo<T> CreateReflectionJsonTypeInfo<T>(JsonSerializerOptions options) => new ReflectionJsonTypeInfo<T>(options);
+        private static JsonTypeInfo<T> CreateReflectionJsonTypeInfo<T>(JsonSerializerOptions options)
+        {
+            JsonConverter converter = GetConverterForType(typeof(T), options);
+            return new ReflectionJsonTypeInfo<T>(converter, options);
+        }
 
         /// <summary>
         /// List of JsonTypeInfo modifiers. Modifying callbacks are called consecutively after initial resolution

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

@@ -283,7 +283,7 @@ public static JsonConverter<T> GetEnumConverter<T>(JsonSerializerOptions options
                 ThrowHelper.ThrowArgumentNullException(nameof(options));
             }
 
-            JsonConverter<T> underlyingConverter = GetTypedConverter<T>(options.GetConverterFromTypeInfo(typeof(T)));
+            JsonConverter<T> underlyingConverter = GetTypedConverter<T>(options.GetConverterInternal(typeof(T)));
 
             return new NullableConverter<T>(underlyingConverter);
         }

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

@@ -39,7 +39,7 @@ public JsonTypeInfo JsonTypeInfo
             {
                 Debug.Assert(Options != null);
                 Debug.Assert(ShouldDeserialize);
-                return _jsonTypeInfo ??= Options.GetTypeInfoCached(PropertyType);
+                return _jsonTypeInfo ??= Options.GetTypeInfoInternal(PropertyType);
             }
             set
             {