Make resolver GetTypeInfo methods return new instances on every call

Author: layomia

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs

@@ -28,6 +28,8 @@ private sealed partial class Emitter
             private const string DefaultOptionsStaticVarName = "s_defaultOptions";
             private const string DefaultContextBackingStaticVarName = "s_defaultContext";
             internal const string GetConverterFromFactoryMethodName = "GetConverterFromFactory";
+            private const string GetTypeInfoCoreMethodName = "GetTypeInfoCore";
+            private const string GetTypeInfoMethodName = "GetTypeInfo";
             private const string InfoVarName = "info";
             private const string PropertyInfoVarName = "propertyInfo";
             internal const string JsonContextVarName = "jsonContext";
@@ -37,6 +39,7 @@ private sealed partial class Emitter
             private const string JsonTypeInfoReturnValueLocalVariableName = "jsonTypeInfo";
             private const string PropInitMethodNameSuffix = "PropInit";
             private const string RuntimeCustomConverterFetchingMethodName = "GetRuntimeProvidedCustomConverter";
+            private const string TypeLocalVariableName = "type";
             private const string SerializeHandlerPropName = "SerializeHandler";
             private const string ValueVarName = "value";
             private const string WriterVarName = "writer";
@@ -1227,9 +1230,9 @@ private static string GetFetchLogicForGetCustomConverter_TypesWithFactories()
             {
                 return @$"
 
-private static {JsonConverterTypeRef} {GetConverterFromFactoryMethodName}({JsonSerializerOptionsTypeRef} {OptionsLocalVariableName}, {TypeTypeRef} type, {JsonConverterFactoryTypeRef} factory)
+private static {JsonConverterTypeRef} {GetConverterFromFactoryMethodName}({JsonSerializerOptionsTypeRef} {OptionsLocalVariableName}, {TypeTypeRef} {TypeLocalVariableName}, {JsonConverterFactoryTypeRef} factory)
 {{
-    {JsonConverterTypeRef}? converter = factory.CreateConverter(type, {OptionsLocalVariableName});
+    {JsonConverterTypeRef}? converter = factory.CreateConverter({TypeLocalVariableName}, {OptionsLocalVariableName});
     if (converter == null || converter is {JsonConverterFactoryTypeRef})
     {{
         throw new {InvalidOperationExceptionTypeRef}(string.Format(""{ExceptionMessages.InvalidJsonConverterFactoryOutput}"", factory.GetType()));
@@ -1245,56 +1248,33 @@ private string GetGetTypeInfoImplementation()
 
                 sb.Append(
 @$"/// <inheritdoc/>
-public override {JsonTypeInfoTypeRef} GetTypeInfo({TypeTypeRef} type)
-{{");
-
-                HashSet<TypeGenerationSpec> types = new(_currentContext.TypesWithMetadataGenerated);
+public override {JsonTypeInfoTypeRef} {GetTypeInfoMethodName}({TypeTypeRef} {TypeLocalVariableName})
+    => {GetTypeInfoCoreMethodName}({TypeLocalVariableName}, {OptionsInstanceVariableName});
 
-                // TODO (https://github.com/dotnet/runtime/issues/52218): Make this Dictionary-lookup-based if root-serializable type count > 64.
-                foreach (TypeGenerationSpec metadata in types)
-                {
-                    if (metadata.ClassType != ClassType.TypeUnsupportedBySourceGen)
-                    {
-                        sb.Append($@"
-    if (type == typeof({metadata.TypeRef}))
-    {{
-        return this.{metadata.TypeInfoPropertyName};
-    }}
+{JsonTypeInfoTypeRef}? {JsonTypeInfoResolverTypeRef}.{GetTypeInfoMethodName}({TypeTypeRef} {TypeLocalVariableName}, {JsonSerializerOptionsTypeRef} {OptionsLocalVariableName})
+    => {GetTypeInfoCoreMethodName}({TypeLocalVariableName}, {OptionsLocalVariableName});
 ");
-                    }
-                }
-
-                sb.AppendLine(@"
-    return null!;
-}");
 
-                // Explicit IJsonTypeInfoResolver implementation
                 sb.AppendLine();
-                sb.Append(@$"{JsonTypeInfoTypeRef}? {JsonTypeInfoResolverTypeRef}.GetTypeInfo({TypeTypeRef} type, {JsonSerializerOptionsTypeRef} {OptionsLocalVariableName})
-{{
-    if ({OptionsInstanceVariableName} == {OptionsLocalVariableName})
-    {{
-        return this.GetTypeInfo(type);
-    }}
-    else
-    {{");
+                sb.Append(@$"private {JsonTypeInfoTypeRef}? {GetTypeInfoCoreMethodName}({TypeTypeRef} {TypeLocalVariableName}, {JsonSerializerOptionsTypeRef} {OptionsLocalVariableName})
+{{");
+
                 // TODO (https://github.com/dotnet/runtime/issues/52218): Make this Dictionary-lookup-based if root-serializable type count > 64.
-                foreach (TypeGenerationSpec metadata in types)
+                foreach (TypeGenerationSpec metadata in _currentContext.TypesWithMetadataGenerated)
                 {
                     if (metadata.ClassType != ClassType.TypeUnsupportedBySourceGen)
                     {
                         sb.Append($@"
-        if (type == typeof({metadata.TypeRef}))
-        {{
-            return {metadata.CreateTypeInfoMethodName}({OptionsLocalVariableName});
-        }}
+    if ({TypeLocalVariableName} == typeof({metadata.TypeRef}))
+    {{
+        return {metadata.CreateTypeInfoMethodName}({OptionsLocalVariableName});
+    }}
 ");
                     }
                 }
 
                 sb.Append($@"
-        return null;
-    }}
+    return null;
 }}
 ");
 

src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs

@@ -463,5 +463,49 @@ public TestResolver(Func<Type, JsonSerializerOptions, JsonTypeInfo?> getTypeInfo
 
             public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options) => _getTypeInfo(type, options);
         }
+
+        [Fact]
+        public static void OptionsGetTypeInfoDoesNotReturnContextInstance()
+        {
+            PersonJsonContext context = PersonJsonContext.Default;
+
+            // Context GetTypeInfo(Type) method returns new instance on each call
+            Assert.NotSame(context.GetTypeInfo(typeof(Person)), context.GetTypeInfo(typeof(Person)));
+
+            JsonTypeInfo context_StaticProp_TypeInfo = context.Person;
+            JsonTypeInfo context_Fetched_TypeInfo = context.GetTypeInfo(typeof(Person));
+
+            // Context fetched instance is different from context static instance
+            Assert.NotSame(context_StaticProp_TypeInfo, context_Fetched_TypeInfo);
+            context_Fetched_TypeInfo.Properties.Clear(); // Context fetched instance is mutable
+            Assert.Equal(2, context_StaticProp_TypeInfo.Properties.Count);
+            Assert.Equal(0, context_Fetched_TypeInfo.Properties.Count);
+
+            // Context static instance is not mutable
+            Assert.Throws<InvalidOperationException>(() => context_StaticProp_TypeInfo.CreateObject = null);
+            Assert.Throws<InvalidOperationException>(() => context_StaticProp_TypeInfo.Properties.Clear());
+
+            // Context-owned options is read-only so fetched instance is cached
+            JsonTypeInfo options_Cached_TypeInfo = context.Options.GetTypeInfo(typeof(Person));
+            Assert.Same(options_Cached_TypeInfo, context.Options.GetTypeInfo(typeof(Person)));
+
+            // Context-owned type info is read-only
+            Assert.Throws<InvalidOperationException>(() => options_Cached_TypeInfo.Properties.Clear());
+
+            // Options fetched instance is different from context instances
+            Assert.NotSame(context_StaticProp_TypeInfo, options_Cached_TypeInfo);
+            Assert.NotSame(context_Fetched_TypeInfo, options_Cached_TypeInfo);
+
+            // Context GetTypeInfo(Type, Options) method returns new instance on each call
+            IJsonTypeInfoResolver resolver = context;
+            JsonSerializerOptions options = new() { TypeInfoResolver = context };
+
+            Assert.NotSame(
+                resolver.GetTypeInfo(typeof(Person), options),
+                resolver.GetTypeInfo(typeof(Person), options));
+
+            // Options are still mutable, so fetched type-info's are not cached.
+            Assert.NotSame(options.GetTypeInfo(typeof(Person)), options.GetTypeInfo(typeof(Person)));
+        }
     }
 }