Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Jan 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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/>
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved

#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;
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved

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)
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
{{");

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.
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
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.
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
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)
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
{
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