Skip to content

Commit

Permalink
Make a number of improvements to the System.Text.Json source generator (
Browse files Browse the repository at this point in the history
dotnet#81239)

* Ensure singleton JsonTypeInfo instances are preconfigured using JsonSerializerOptions.

* Fix indentation & whitespace issues in the source generator.

* Fix dotnet#76829.

* Fix nullability annotations in GetTypeInfo gen.

* Add clarifying comment.

* Add testing for types without metadata or fast-path generation.
  • Loading branch information
eiriktsarpalis authored Jan 27, 2023
1 parent 35e72a1 commit 9e7d3c0
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 66 deletions.
93 changes: 47 additions & 46 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -152,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(@"// <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));
Expand Down Expand Up @@ -1146,24 +1146,20 @@ private static string GenerateForType(TypeGenerationSpec typeMetadata, string me
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}";
Expand Down Expand Up @@ -1324,13 +1320,14 @@ private string GetGetTypeInfoImplementation()

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)
{
Expand All @@ -1344,22 +1341,21 @@ private string GetGetTypeInfoImplementation()
}

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});
}}
");
}
Expand Down Expand Up @@ -1393,8 +1389,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) =>
Expand Down
3 changes: 0 additions & 3 deletions src/libraries/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public abstract partial class JsonSerializerContext : IJsonTypeInfoResolver

/// <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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<JsonParameterInfoValues>();
MetadataSerializationNotSupported = true;
}
Expand Down Expand Up @@ -144,11 +139,6 @@ internal override void LateAddProperties()
return;
}

if (SerializeHandler == null)
{
ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeProperties(Options.TypeInfoResolver, Type);
}

MetadataSerializationNotSupported = true;
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?? "<null>", type));
}

[DoesNotReturn]
public static void ThrowMissingMemberException_MissingFSharpCoreMember(string missingFsharpCoreMember)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -208,6 +209,18 @@ public override void EnsureFastPathGeneratedAsExpected()
Assert.Null(SerializationContext.Default.PolymorphicClass.SerializeHandler);
}

[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()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,9 @@ public class ClassWithObsolete
{
[Obsolete(""This is a test"")]
public bool Test { get; set; }
[Obsolete]
public bool Test2 { get; set; }
}
}
";
Expand Down

0 comments on commit 9e7d3c0

Please sign in to comment.