From 8faa4e6e134b2ce39fa57377d8082dc406fc1ffa Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 26 Jan 2023 20:33:41 +0000 Subject: [PATCH 1/6] Ensure singleton JsonTypeInfo instances are preconfigured using JsonSerializerOptions. --- .../gen/JsonSourceGenerator.Emitter.cs | 20 +++++-------------- .../src/Resources/Strings.resx | 3 --- .../Serialization/JsonSerializerContext.cs | 2 +- .../Metadata/SourceGenJsonTypeInfoOfT.cs | 10 ---------- .../Text/Json/ThrowHelper.Serialization.cs | 6 ------ 5 files changed, 6 insertions(+), 35 deletions(-) diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs index 9743eaec41f3e..55d88387d9226 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs @@ -26,7 +26,6 @@ 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 MakeReadOnlyMethodName = "MakeReadOnly"; private const string InfoVarName = "info"; private const string PropertyInfoVarName = "propertyInfo"; internal const string JsonContextVarName = "jsonContext"; @@ -1151,19 +1150,14 @@ private static string GenerateForType(TypeGenerationSpec typeMetadata, string me /// public {typeInfoPropertyTypeRef} {typeFriendlyName} {{ - get => _{typeFriendlyName} ??= {typeMetadata.CreateTypeInfoMethodName}({OptionsInstanceVariableName}, makeReadOnly: true); + get => _{typeFriendlyName} ??= ({typeInfoPropertyTypeRef}){OptionsInstanceVariableName}.GetTypeInfo(typeof({typeCompilableName})); }} -private {typeInfoPropertyTypeRef} {typeMetadata.CreateTypeInfoMethodName}({JsonSerializerOptionsTypeRef} {OptionsLocalVariableName}, bool makeReadOnly) +private {typeInfoPropertyTypeRef} {typeMetadata.CreateTypeInfoMethodName}({JsonSerializerOptionsTypeRef} {OptionsLocalVariableName}) {{ {typeInfoPropertyTypeRef}? {JsonTypeInfoReturnValueLocalVariableName} = null; {WrapWithCheckForCustomConverter(metadataInitSource, typeCompilableName)} - if (makeReadOnly) - {{ - {JsonTypeInfoReturnValueLocalVariableName}.{MakeReadOnlyMethodName}(); - }} - return {JsonTypeInfoReturnValueLocalVariableName}; }} {additionalSource}"; @@ -1327,10 +1321,7 @@ private string GetGetTypeInfoImplementation() public override {JsonTypeInfoTypeRef} GetTypeInfo({TypeTypeRef} type) {{"); - HashSet types = new(_currentContext.TypesWithMetadataGenerated); - - // 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) { @@ -1351,15 +1342,14 @@ private string GetGetTypeInfoImplementation() sb.AppendLine(); sb.Append(@$"{JsonTypeInfoTypeRef}? {JsonTypeInfoResolverTypeRef}.GetTypeInfo({TypeTypeRef} type, {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}, makeReadOnly: false); + return {metadata.CreateTypeInfoMethodName}({OptionsLocalVariableName}); }} "); } diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx index 933332550aa1b..cb488513e0981 100644 --- a/src/libraries/System.Text.Json/src/Resources/Strings.resx +++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx @@ -617,9 +617,6 @@ F# discriminated union serialization is not supported. Consider authoring a custom converter for the type. - - TypeInfoResolver '{0}' did not provide constructor parameter metadata for type '{1}'. - The converter for polymorphic type '{0}' does not support metadata writes or reads. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs index b07c54438ef05..014af6fa1e702 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs @@ -15,7 +15,7 @@ public abstract partial class JsonSerializerContext : IJsonTypeInfoResolver /// /// Gets the run time specified options of the context. If no options were passed - /// when instanciating the context, then a new instance is bound and returned. + /// when instantiating the context, then a new instance is bound and returned. /// /// /// The options instance cannot be mutated once it is bound to the context instance. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs index 13a55afba0818..8c1414d0a05c6 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs @@ -107,11 +107,6 @@ internal override JsonParameterInfoValues[] GetParameterInfoValues() JsonParameterInfoValues[] array; if (CtorParamInitFunc == null || (array = CtorParamInitFunc()) == null) { - if (SerializeHandler == null) - { - ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeCtorParams(Options.TypeInfoResolver, Type); - } - array = Array.Empty(); MetadataSerializationNotSupported = true; } @@ -144,11 +139,6 @@ internal override void LateAddProperties() return; } - if (SerializeHandler == null) - { - ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeProperties(Options.TypeInfoResolver, Type); - } - MetadataSerializationNotSupported = true; return; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs index 6fe76016a365d..419732adbe4e9 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs @@ -745,12 +745,6 @@ public static void ThrowInvalidOperationException_NoMetadataForTypeProperties(IJ throw GetInvalidOperationException_NoMetadataForTypeProperties(resolver, type); } - [DoesNotReturn] - public static void ThrowInvalidOperationException_NoMetadataForTypeCtorParams(IJsonTypeInfoResolver? resolver, Type type) - { - throw new InvalidOperationException(SR.Format(SR.NoMetadataForTypeCtorParams, resolver?.GetType().FullName ?? "", type)); - } - [DoesNotReturn] public static void ThrowMissingMemberException_MissingFSharpCoreMember(string missingFsharpCoreMember) { From 82faed5e42e9981dfd8f94461a3d924622e8cd32 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 26 Jan 2023 20:34:32 +0000 Subject: [PATCH 2/6] Fix indentation & whitespace issues in the source generator. --- .../gen/JsonSourceGenerator.Emitter.cs | 63 ++++++++++--------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs index 55d88387d9226..9b241161a2f8f 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs @@ -151,49 +151,50 @@ private void AddSource(string fileName, string source, bool isRootContextDef = f string? @namespace = _currentContext.ContextType.Namespace; bool isInGlobalNamespace = @namespace == JsonConstants.GlobalNamespaceValue; - StringBuilder sb = new(@"// + StringBuilder sb = new(""" +// #nullable enable annotations #nullable disable warnings // Suppress warnings about [Obsolete] member usage in generated code. -#pragma warning disable CS0618"); +#pragma warning disable CS0618 + + +"""); + int indentation = 0; if (!isInGlobalNamespace) { - sb.Append(@$" - -namespace {@namespace} -{{"); + sb.AppendLine($$""" +namespace {{@namespace}} +{ +"""); + indentation++; } for (int i = 0; i < declarationCount - 1; i++) { - string declarationSource = $@" -{declarationList[declarationCount - 1 - i]} -{{"; - sb.Append($@" -{IndentSource(declarationSource, numIndentations: i + 1)} -"); + string declarationSource = $$""" +{{declarationList[declarationCount - 1 - i]}} +{ +"""; + sb.AppendLine(IndentSource(declarationSource, numIndentations: indentation++)); } // Add the core implementation for the derived context class. - string partialContextImplementation = $@" -{generatedCodeAttributeSource}{declarationList[0]}{(interfaceImplementation is null ? "" : ": " + interfaceImplementation)} -{{ - {IndentSource(source, Math.Max(1, declarationCount - 1))} -}}"; - sb.AppendLine(IndentSource(partialContextImplementation, numIndentations: declarationCount)); - - // Match curly brace for each containing type. - for (int i = 0; i < declarationCount - 1; i++) - { - sb.AppendLine(IndentSource("}", numIndentations: declarationCount + i + 1)); - } + string partialContextImplementation = $$""" +{{generatedCodeAttributeSource}}{{declarationList[0]}}{{(interfaceImplementation is null ? "" : ": " + interfaceImplementation)}} +{ +{{IndentSource(source, 1)}} +} +"""; + sb.AppendLine(IndentSource(partialContextImplementation, numIndentations: indentation--)); - if (!isInGlobalNamespace) + // Match curly braces for each containing type/namespace. + for (; indentation >= 0; indentation--) { - sb.AppendLine("}"); + sb.AppendLine(IndentSource("}", numIndentations: indentation)); } _sourceGenerationContext.AddSource(fileName, SourceText.From(sb.ToString(), Encoding.UTF8)); @@ -1145,6 +1146,7 @@ private static string GenerateForType(TypeGenerationSpec typeMetadata, string me string typeInfoPropertyTypeRef = $"{JsonTypeInfoTypeRef}<{typeCompilableName}>"; return @$"private {typeInfoPropertyTypeRef}? _{typeFriendlyName}; + /// /// Defines the source generated JSON serialization contract metadata for a given type. /// @@ -1383,8 +1385,13 @@ private string GetPropertyNameInitialization() private static string IndentSource(string source, int numIndentations) { - Debug.Assert(numIndentations >= 1); - return source.Replace(Environment.NewLine, $"{Environment.NewLine}{new string(' ', 4 * numIndentations)}"); // 4 spaces per indentation. + if (numIndentations == 0) + { + return source; + } + + string indentation = new string(' ', 4 * numIndentations); // 4 spaces per indentation. + return indentation + source.Replace(Environment.NewLine, Environment.NewLine + indentation); } private static string GetNumberHandlingAsStr(JsonNumberHandling? numberHandling) => From d287f93153a1e26cbd244075e68957196c7ec159 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 26 Jan 2023 20:35:32 +0000 Subject: [PATCH 3/6] Fix #76829. --- .../System.Text.Json/gen/JsonSourceGenerator.Emitter.cs | 2 +- .../JsonSourceGeneratorTests.cs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs index 9b241161a2f8f..320ea48dd7646 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs @@ -158,7 +158,7 @@ private void AddSource(string fileName, string source, bool isRootContextDef = f #nullable disable warnings // Suppress warnings about [Obsolete] member usage in generated code. -#pragma warning disable CS0618 +#pragma warning disable CS0612, CS0618 """); diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs index d91e0a7f37a6b..ce52184065a5f 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs @@ -735,6 +735,9 @@ public class ClassWithObsolete { [Obsolete(""This is a test"")] public bool Test { get; set; } + + [Obsolete] + public bool Test2 { get; set; } } } "; From 54204704e897b96eb69674ea31f1ea763bac594e Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 26 Jan 2023 20:39:00 +0000 Subject: [PATCH 4/6] Fix nullability annotations in GetTypeInfo gen. --- .../System.Text.Json/gen/JsonSourceGenerator.Emitter.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs index 320ea48dd7646..628bc22cc3174 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs @@ -1320,7 +1320,7 @@ private string GetGetTypeInfoImplementation() sb.Append( @$"/// -public override {JsonTypeInfoTypeRef} GetTypeInfo({TypeTypeRef} type) +public override {JsonTypeInfoTypeRef}? GetTypeInfo({TypeTypeRef} type) {{"); foreach (TypeGenerationSpec metadata in _currentContext.TypesWithMetadataGenerated) @@ -1337,7 +1337,7 @@ private string GetGetTypeInfoImplementation() } sb.AppendLine(@" - return null!; + return null; }"); // Explicit IJsonTypeInfoResolver implementation From 42feb1b123cba07d170777c6ffb06a1cf77e6e5e Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 26 Jan 2023 20:44:41 +0000 Subject: [PATCH 5/6] Add clarifying comment. --- .../System.Text.Json/gen/JsonSourceGenerator.Emitter.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs index 628bc22cc3174..5455d3abf12bc 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs @@ -1322,7 +1322,11 @@ private string GetGetTypeInfoImplementation() @$"/// public override {JsonTypeInfoTypeRef}? GetTypeInfo({TypeTypeRef} type) {{"); - + // This method body grows linearly over the number of generated types. + // In line with https://github.com/dotnet/runtime/issues/77897 we should + // eventually replace this method with a direct call to Options.GetTypeInfo(). + // We can't do this currently because Options.GetTypeInfo throws whereas + // this GetTypeInfo returns null for unsupported types, so we need new API to support it. foreach (TypeGenerationSpec metadata in _currentContext.TypesWithMetadataGenerated) { if (metadata.ClassType != ClassType.TypeUnsupportedBySourceGen) From e94f94d87b30df0ff33bdcb1014a47c888e32fdb Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 26 Jan 2023 21:47:55 +0000 Subject: [PATCH 6/6] Add testing for types without metadata or fast-path generation. --- .../SerializationContextTests.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/SerializationContextTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/SerializationContextTests.cs index 9917cd3d282fd..971b858108daa 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/SerializationContextTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/SerializationContextTests.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; using System.Reflection; using Xunit; @@ -208,6 +209,18 @@ public override void EnsureFastPathGeneratedAsExpected() Assert.Null(SerializationContext.Default.PolymorphicClass.SerializeHandler); } + [Fact] + public void TypeWithoutMetadataOrFastPathHandler_SerializationThrowsInvalidOperationException() + { + JsonTypeInfo typeInfo = SerializationContext.Default.ClassWithCustomConverterProperty; + Assert.Null(typeInfo.SerializeHandler); + Assert.Empty(typeInfo.Properties); + + var value = new ClassWithCustomConverterProperty(); + Assert.Throws(() => JsonSerializer.Serialize(value, typeInfo)); + Assert.Throws(() => JsonSerializer.Deserialize("""{"Property":42}""", typeInfo)); + } + [Fact] public override void RoundTripLocation() {