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

Use underlying type converter for nullable type #84208

Merged
merged 8 commits into from
May 31, 2023
65 changes: 49 additions & 16 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ private sealed partial class Emitter
private const string PropInitMethodNameSuffix = "PropInit";
private const string TryGetTypeInfoForRuntimeCustomConverterMethodName = "TryGetTypeInfoForRuntimeCustomConverter";
private const string ExpandConverterMethodName = "ExpandConverter";
private const string GetConverterForNullablePropertyMethodName = "GetConverterForNullableProperty";
private const string SerializeHandlerPropName = "SerializeHandler";
private const string OptionsLocalVariableName = "options";
private const string ValueVarName = "value";
Expand Down Expand Up @@ -79,6 +80,11 @@ private sealed partial class Emitter
/// </summary>
private readonly Dictionary<string, string> _propertyNames = new();

/// <summary>
/// Indicates that the type graph contains a nullable property with a design-time custom converter declaration.
/// </summary>
private bool _emitGetConverterForNullablePropertyMethod;

/// <summary>
/// The SourceText emit implementation filled by the individual Roslyn versions.
/// </summary>
Expand All @@ -88,6 +94,7 @@ public void Emit(ContextGenerationSpec contextGenerationSpec)
{
Debug.Assert(_typeIndex.Count == 0);
Debug.Assert(_propertyNames.Count == 0);
Debug.Assert(!_emitGetConverterForNullablePropertyMethod);

foreach (TypeGenerationSpec spec in contextGenerationSpec.GeneratedTypes)
{
Expand All @@ -106,14 +113,15 @@ public void Emit(ContextGenerationSpec contextGenerationSpec)
string contextName = contextGenerationSpec.ContextType.Name;

// Add root context implementation.
AddSource($"{contextName}.g.cs", GetRootJsonContextImplementation(contextGenerationSpec));
AddSource($"{contextName}.g.cs", GetRootJsonContextImplementation(contextGenerationSpec, _emitGetConverterForNullablePropertyMethod));

// Add GetJsonTypeInfo override implementation.
AddSource($"{contextName}.GetJsonTypeInfo.g.cs", GetGetTypeInfoImplementation(contextGenerationSpec));

// Add property name initialization.
AddSource($"{contextName}.PropertyNames.g.cs", GetPropertyNameInitialization(contextGenerationSpec));

_emitGetConverterForNullablePropertyMethod = false;
_propertyNames.Clear();
_typeIndex.Clear();
}
Expand Down Expand Up @@ -539,7 +547,7 @@ private SourceText GenerateForObject(ContextGenerationSpec contextSpec, TypeGene
return CompleteSourceFileAndReturnText(writer);
}

private static void GeneratePropMetadataInitFunc(SourceWriter writer, string propInitMethodName, TypeGenerationSpec typeGenerationSpec)
private void GeneratePropMetadataInitFunc(SourceWriter writer, string propInitMethodName, TypeGenerationSpec typeGenerationSpec)
{
Debug.Assert(typeGenerationSpec.PropertyGenSpecs != null);
ImmutableEquatableArray<PropertyGenerationSpec> properties = typeGenerationSpec.PropertyGenSpecs;
Expand Down Expand Up @@ -585,9 +593,15 @@ private static void GeneratePropMetadataInitFunc(SourceWriter writer, string pro
? $"{JsonIgnoreConditionTypeRef}.{property.DefaultIgnoreCondition.Value}"
: "null";

string converterInstantiationExpr = property.ConverterType != null
? $"({JsonConverterTypeRef}<{propertyTypeFQN}>){ExpandConverterMethodName}(typeof({propertyTypeFQN}), new {property.ConverterType.FullyQualifiedName}(), {OptionsLocalVariableName})"
: "null";
string? converterInstantiationExpr = null;
if (property.ConverterType != null)
{
TypeRef? nullableUnderlyingType = _typeIndex[property.PropertyType].NullableUnderlyingType;
_emitGetConverterForNullablePropertyMethod |= nullableUnderlyingType != null;
converterInstantiationExpr = nullableUnderlyingType != null
? $"{GetConverterForNullablePropertyMethodName}<{nullableUnderlyingType.FullyQualifiedName}>(new {property.ConverterType.FullyQualifiedName}(), {OptionsLocalVariableName})"
: $"({JsonConverterTypeRef}<{propertyTypeFQN}>){ExpandConverterMethodName}(typeof({propertyTypeFQN}), new {property.ConverterType.FullyQualifiedName}(), {OptionsLocalVariableName})";
}

writer.WriteLine($$"""
var {{InfoVarName}}{{i}} = new {{JsonPropertyInfoValuesTypeRef}}<{{propertyTypeFQN}}>()
Expand All @@ -596,7 +610,7 @@ private static void GeneratePropMetadataInitFunc(SourceWriter writer, string pro
IsPublic = {{FormatBool(property.IsPublic)}},
IsVirtual = {{FormatBool(property.IsVirtual)}},
DeclaringType = typeof({{property.DeclaringType.FullyQualifiedName}}),
Converter = {{converterInstantiationExpr}},
Converter = {{converterInstantiationExpr ?? "null"}},
Getter = {{getterValue}},
Setter = {{setterValue}},
IgnoreCondition = {{ignoreConditionNamedArg}},
Expand Down Expand Up @@ -1007,7 +1021,7 @@ private static void GenerateTypeInfoProperty(SourceWriter writer, TypeGeneration
""");
}

private static SourceText GetRootJsonContextImplementation(ContextGenerationSpec contextSpec)
private static SourceText GetRootJsonContextImplementation(ContextGenerationSpec contextSpec, bool emitGetConverterForNullablePropertyMethod)
{
string contextTypeRef = contextSpec.ContextType.FullyQualifiedName;
string contextTypeName = contextSpec.ContextType.Name;
Expand Down Expand Up @@ -1048,7 +1062,7 @@ private static SourceText GetRootJsonContextImplementation(ContextGenerationSpec

writer.WriteLine();

GenerateConverterHelpers(writer);
GenerateConverterHelpers(writer, emitGetConverterForNullablePropertyMethod);

return CompleteSourceFileAndReturnText(writer);
}
Expand Down Expand Up @@ -1082,7 +1096,7 @@ private static void GetLogicForDefaultSerializerOptionsInit(ContextGenerationSpe
""");
}

private static void GenerateConverterHelpers(SourceWriter writer)
private static void GenerateConverterHelpers(SourceWriter writer, bool emitGetConverterForNullablePropertyMethod)
{
// The generic type parameter could capture type parameters from containing types,
// so use a name that is unlikely to be used.
Expand All @@ -1109,15 +1123,20 @@ private static void GenerateConverterHelpers(SourceWriter writer)
{{JsonConverterTypeRef}}? converter = options.Converters[i];
if (converter?.CanConvert(type) == true)
{
return {{ExpandConverterMethodName}}(type, converter, options);
return {{ExpandConverterMethodName}}(type, converter, options, validateCanConvert: false);
}
}

return null;
}

private static {{JsonConverterTypeRef}} {{ExpandConverterMethodName}}({{TypeTypeRef}} type, {{JsonConverterTypeRef}} converter, {{JsonSerializerOptionsTypeRef}} options)
private static {{JsonConverterTypeRef}} {{ExpandConverterMethodName}}({{TypeTypeRef}} type, {{JsonConverterTypeRef}} converter, {{JsonSerializerOptionsTypeRef}} options, bool validateCanConvert = true)
{
if (validateCanConvert && !converter.CanConvert(type))
{
throw new {{InvalidOperationExceptionTypeRef}}(string.Format("{{ExceptionMessages.IncompatibleConverterType}}", converter.GetType(), type));
}

if (converter is {{JsonConverterFactoryTypeRef}} factory)
{
converter = factory.CreateConverter(type, options);
Expand All @@ -1126,15 +1145,29 @@ private static void GenerateConverterHelpers(SourceWriter writer)
throw new {{InvalidOperationExceptionTypeRef}}(string.Format("{{ExceptionMessages.InvalidJsonConverterFactoryOutput}}", factory.GetType()));
}
}

if (!converter.CanConvert(type))
{
throw new {{InvalidOperationExceptionTypeRef}}(string.Format("{{ExceptionMessages.IncompatibleConverterType}}", converter.GetType(), type));
}

return converter;
}
""");

if (emitGetConverterForNullablePropertyMethod)
{
writer.WriteLine($$"""

private static {{JsonConverterTypeRef}}<{{TypeParameter}}?> {{GetConverterForNullablePropertyMethodName}}<{{TypeParameter}}>({{JsonConverterTypeRef}} converter, {{JsonSerializerOptionsTypeRef}} options)
where {{TypeParameter}} : struct
{
if (converter.CanConvert(typeof({{TypeParameter}}?)))
{
return ({{JsonConverterTypeRef}}<{{TypeParameter}}?>){{ExpandConverterMethodName}}(typeof({{TypeParameter}}?), converter, options, validateCanConvert: false);
}

converter = {{ExpandConverterMethodName}}(typeof({{TypeParameter}}), converter, options);
{{JsonTypeInfoTypeRef}}<{{TypeParameter}}> typeInfo = {{JsonMetadataServicesTypeRef}}.{{CreateValueInfoMethodName}}<{{TypeParameter}}>(options, converter);
return {{JsonMetadataServicesTypeRef}}.GetNullableConverter<{{TypeParameter}}>(typeInfo);
}
""");
}
}

private static SourceText GetGetTypeInfoImplementation(ContextGenerationSpec contextSpec)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public interface ITestContext
public JsonTypeInfo<StructWithCustomConverterProperty> StructWithCustomConverterProperty { get; }
public JsonTypeInfo<ClassWithCustomConverterFactoryProperty> ClassWithCustomConverterFactoryProperty { get; }
public JsonTypeInfo<StructWithCustomConverterFactoryProperty> StructWithCustomConverterFactoryProperty { get; }
public JsonTypeInfo<ClassWithCustomConverterNullableProperty> ClassWithCustomConverterNullableProperty { get; }
public JsonTypeInfo<ClassWithCustomConverterFactoryNullableProperty> ClassWithCustomConverterFactoryNullableProperty { get; }
public JsonTypeInfo<ClassWithBadCustomConverter> ClassWithBadCustomConverter { get; }
public JsonTypeInfo<StructWithBadCustomConverter> StructWithBadCustomConverter { get; }
public JsonTypeInfo<PersonStruct?> NullablePersonStruct { get; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ namespace System.Text.Json.SourceGeneration.Tests
[JsonSerializable(typeof(StructWithCustomConverterProperty))]
[JsonSerializable(typeof(ClassWithCustomConverterFactoryProperty))]
[JsonSerializable(typeof(StructWithCustomConverterFactoryProperty))]
[JsonSerializable(typeof(ClassWithCustomConverterNullableProperty))]
[JsonSerializable(typeof(ClassWithCustomConverterFactoryNullableProperty))]
[JsonSerializable(typeof(ClassWithBadCustomConverter))]
[JsonSerializable(typeof(StructWithBadCustomConverter))]
[JsonSerializable(typeof(PersonStruct?))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ namespace System.Text.Json.SourceGeneration.Tests
[JsonSerializable(typeof(StructWithCustomConverterProperty), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(ClassWithCustomConverterFactoryProperty), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(StructWithCustomConverterFactoryProperty), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(ClassWithCustomConverterNullableProperty), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(ClassWithCustomConverterFactoryNullableProperty), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(ClassWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(StructWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(PersonStruct?), GenerationMode = JsonSourceGenerationMode.Metadata)]
Expand Down Expand Up @@ -144,6 +146,8 @@ public override void EnsureFastPathGeneratedAsExpected()
[JsonSerializable(typeof(StructWithCustomConverterProperty))]
[JsonSerializable(typeof(ClassWithCustomConverterFactoryProperty))]
[JsonSerializable(typeof(StructWithCustomConverterFactoryProperty))]
[JsonSerializable(typeof(ClassWithCustomConverterNullableProperty))]
[JsonSerializable(typeof(ClassWithCustomConverterFactoryNullableProperty))]
[JsonSerializable(typeof(ClassWithBadCustomConverter))]
[JsonSerializable(typeof(StructWithBadCustomConverter))]
[JsonSerializable(typeof(PersonStruct?))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ namespace System.Text.Json.SourceGeneration.Tests
[JsonSerializable(typeof(StructWithCustomConverterProperty), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithCustomConverterFactoryProperty), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(StructWithCustomConverterFactoryProperty), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithCustomConverterNullableProperty), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithCustomConverterFactoryNullableProperty), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(StructWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(PersonStruct?), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.Linq;
using System.Text.Json.Serialization;
using System.Text.Json.Serialization.Metadata;
using System.Text.Json.Serialization.Tests;
using Xunit;

namespace System.Text.Json.SourceGeneration.Tests
Expand Down Expand Up @@ -257,6 +256,60 @@ public virtual void RoundtripWithCustomConverterProperty_Class()
Assert.Equal(42, obj.Property.Value);
}

[Fact]
public virtual void RoundTripWithCustomConverterNullableProperty()
{
const string Json = "{\"TimeSpan\":42}";

var obj = new ClassWithCustomConverterNullableProperty
{
TimeSpan = TimeSpan.FromSeconds(42)
};

// Types with properties in custom converters do not support fast path serialization.
Assert.True(DefaultContext.ClassWithCustomConverterNullableProperty.SerializeHandler is null);

if (DefaultContext.JsonSourceGenerationMode == JsonSourceGenerationMode.Serialization)
{
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(obj, DefaultContext.ClassWithCustomConverterNullableProperty));
}
else
{
string json = JsonSerializer.Serialize(obj, DefaultContext.ClassWithCustomConverterNullableProperty);
Assert.Equal(Json, json);

obj = JsonSerializer.Deserialize(Json, DefaultContext.ClassWithCustomConverterNullableProperty);
Assert.Equal(42, obj.TimeSpan.Value.TotalSeconds);
}
}

[Fact]
public virtual void RoundTripWithCustomConverterFactoryNullableProperty()
{
const string Json = "{\"MyEnum\":\"Two\"}";

var obj = new ClassWithCustomConverterFactoryNullableProperty
{
MyEnum = SourceGenSampleEnum.Two
};

// Types with properties in custom converters do not support fast path serialization.
Assert.True(DefaultContext.ClassWithCustomConverterFactoryNullableProperty.SerializeHandler is null);

if (DefaultContext.JsonSourceGenerationMode == JsonSourceGenerationMode.Serialization)
{
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(obj, DefaultContext.ClassWithCustomConverterFactoryNullableProperty));
}
else
{
string json = JsonSerializer.Serialize(obj, DefaultContext.ClassWithCustomConverterFactoryNullableProperty);
Assert.Equal(Json, json);

obj = JsonSerializer.Deserialize(Json, DefaultContext.ClassWithCustomConverterFactoryNullableProperty);
Assert.Equal(SourceGenSampleEnum.Two, obj.MyEnum.Value);
}
}

[Fact]
public virtual void RoundtripWithCustomConverterProperty_Struct()
{
Expand Down
Loading