Make a number of improvements to the System.Text.Json source generator (#81239)

* Ensure singleton JsonTypeInfo instances are preconfigured using JsonSerializerOptions.

* Fix indentation & whitespace issues in the source generator.

* Fix #76829.

* Fix nullability annotations in GetTypeInfo gen.

* Add clarifying comment.

* Add testing for types without metadata or fast-path generation.

diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
index 9743eae..5455d3a 100644 (file)
--- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
+++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
@@ -26,7 +26,6 @@ namespace System.Text.Json.SourceGeneration
             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";
@@ -152,49 +151,50 @@ namespace System.Text.Json.SourceGeneration
                 string? @namespace = _currentContext.ContextType.Namespace;
                 bool isInGlobalNamespace = @namespace == JsonConstants.GlobalNamespaceValue;
 
-                StringBuilder sb = new(@"// <auto-generated/>
+                StringBuilder sb = new("""
+// <auto-generated/>
 
 #nullable enable annotations
 #nullable disable warnings
 
 // Suppress warnings about [Obsolete] member usage in generated code.
-#pragma warning disable CS0618");
+#pragma warning disable CS0612, 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));
@@ -1146,24 +1146,20 @@ private void {serializeMethodName}({Utf8JsonWriterTypeRef} {WriterVarName}, {val
                 string typeInfoPropertyTypeRef = $"{JsonTypeInfoTypeRef}<{typeCompilableName}>";
 
                 return @$"private {typeInfoPropertyTypeRef}? _{typeFriendlyName};
+
 /// <summary>
 /// Defines the source generated JSON serialization contract metadata for a given type.
 /// </summary>
 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}";
@@ -1324,13 +1320,14 @@ private static {JsonConverterTypeRef} {GetConverterFromFactoryMethodName}({JsonS
 
                 sb.Append(
 @$"/// <inheritdoc/>
-public override {JsonTypeInfoTypeRef} GetTypeInfo({TypeTypeRef} type)
+public override {JsonTypeInfoTypeRef}? GetTypeInfo({TypeTypeRef} type)
 {{");
-
-                HashSet<TypeGenerationSpec> 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)
+                // 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)
                     {
@@ -1344,22 +1341,21 @@ public override {JsonTypeInfoTypeRef} GetTypeInfo({TypeTypeRef} type)
                 }
 
                 sb.AppendLine(@"
-    return null!;
+    return null;
 }");
 
                 // Explicit IJsonTypeInfoResolver implementation
                 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});
     }}
 ");
                     }
@@ -1393,8 +1389,13 @@ private static readonly {JsonEncodedTextTypeRef} {name_varName_pair.Value} = {Js
 
             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) =>

diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx
index 9333325..cb48851 100644 (file)
--- a/src/libraries/System.Text.Json/src/Resources/Strings.resx
+++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx
@@ -617,9 +617,6 @@
   <data name="FSharpDiscriminatedUnionsNotSupported" xml:space="preserve">
     <value>F# discriminated union serialization is not supported. Consider authoring a custom converter for the type.</value>
   </data>
-  <data name="NoMetadataForTypeCtorParams" xml:space="preserve">
-    <value>TypeInfoResolver '{0}' did not provide constructor parameter metadata for type '{1}'.</value>
-  </data>
   <data name="Polymorphism_BaseConverterDoesNotSupportMetadata" xml:space="preserve">
     <value>The converter for polymorphic type '{0}' does not support metadata writes or reads.</value>
   </data>

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 b07c544..014af6f 100644 (file)
--- 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 @@ namespace System.Text.Json.Serialization
 
         /// <summary>
         /// 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.
         /// </summary>
         /// <remarks>
         /// 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 13a55af..8c1414d 100644 (file)
--- 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 @@ namespace System.Text.Json.Serialization.Metadata
             JsonParameterInfoValues[] array;
             if (CtorParamInitFunc == null || (array = CtorParamInitFunc()) == null)
             {
-                if (SerializeHandler == null)
-                {
-                    ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeCtorParams(Options.TypeInfoResolver, Type);
-                }
-
                 array = Array.Empty<JsonParameterInfoValues>();
                 MetadataSerializationNotSupported = true;
             }
@@ -144,11 +139,6 @@ namespace System.Text.Json.Serialization.Metadata
                     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 6fe7601..419732a 100644 (file)
--- 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
@@ -746,12 +746,6 @@ namespace System.Text.Json
         }
 
         [DoesNotReturn]
-        public static void ThrowInvalidOperationException_NoMetadataForTypeCtorParams(IJsonTypeInfoResolver? resolver, Type type)
-        {
-            throw new InvalidOperationException(SR.Format(SR.NoMetadataForTypeCtorParams, resolver?.GetType().FullName ?? "<null>", type));
-        }
-
-        [DoesNotReturn]
         public static void ThrowMissingMemberException_MissingFSharpCoreMember(string missingFsharpCoreMember)
         {
             throw new MissingMemberException(SR.Format(SR.MissingFSharpCoreMember, missingFsharpCoreMember));

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 9917cd3..971b858 100644 (file)
--- 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;
 
@@ -209,6 +210,18 @@ namespace System.Text.Json.SourceGeneration.Tests
         }
 
         [Fact]
+        public void TypeWithoutMetadataOrFastPathHandler_SerializationThrowsInvalidOperationException()
+        {
+            JsonTypeInfo<ClassWithCustomConverterProperty> typeInfo = SerializationContext.Default.ClassWithCustomConverterProperty;
+            Assert.Null(typeInfo.SerializeHandler);
+            Assert.Empty(typeInfo.Properties);
+
+            var value = new ClassWithCustomConverterProperty();
+            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(value, typeInfo));
+            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize("""{"Property":42}""", typeInfo));
+        }
+
+        [Fact]
         public override void RoundTripLocation()
         {
             Location expected = CreateLocation();

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 d91e0a7..ce52184 100644 (file)
--- 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 @@ namespace Test
     {
         [Obsolete(""This is a test"")]
         public bool Test { get; set; }
+
+        [Obsolete]
+        public bool Test2 { get; set; }
     }
 }
 ";