diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs index db7b990e095d92..393dc0128e3e12 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs +++ b/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( @$"/// -public override {JsonTypeInfoTypeRef} GetTypeInfo({TypeTypeRef} type) -{{"); - - HashSet 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; }} "); diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs index b89fcf34ed5ce0..89a8d8a88b538e 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs @@ -463,5 +463,49 @@ public TestResolver(Func 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(() => context_StaticProp_TypeInfo.CreateObject = null); + Assert.Throws(() => 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(() => 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))); + } } }