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.

Sign up for GitHub

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

Jump to bottom

Support JsonConverterFactory with src-gen #58398

Merged
merged 4 commits into from
Sep 3, 2021
Merged

Support JsonConverterFactory with src-gen #58398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f94d0ff
Support JsonConverterFactory with src-gen
steveharter Aug 30, 2021
c6582d8
Support property factory converters
steveharter Sep 2, 2021
53e8e64
Fixup merge ordering; add tests
steveharter Sep 2, 2021
1f2f755
Ensure context helper methods properly generated for implicit context…
steveharter Sep 3, 2021
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
132 changes: 84 additions & 48 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,19 @@ public sealed partial class JsonSourceGenerator
private sealed partial class Emitter
{
// Literals in generated source
private const string RuntimeCustomConverterFetchingMethodName = "GetRuntimeProvidedCustomConverter";
private const string OptionsInstanceVariableName = "Options";
private const string PropInitMethodNameSuffix = "PropInit";
private const string CtorParamInitMethodNameSuffix = "CtorParamInit";
private const string SerializeMethodNameSuffix = "Serialize";
private const string CreateValueInfoMethodName = "CreateValueInfo";
private const string CtorParamInitMethodNameSuffix = "CtorParamInit";
private const string DefaultOptionsStaticVarName = "s_defaultOptions";
private const string DefaultContextBackingStaticVarName = "s_defaultContext";
private const string WriterVarName = "writer";
private const string ValueVarName = "value";
internal const string GetConverterFromFactoryMethodName = "GetConverterFromFactory";
private const string JsonSerializerContextName = "JsonSerializerContext";
internal const string JsonContextVarName = "jsonContext";
private const string OptionsInstanceVariableName = "Options";
private const string PropInitMethodNameSuffix = "PropInit";
private const string RuntimeCustomConverterFetchingMethodName = "GetRuntimeProvidedCustomConverter";
private const string SerializeMethodNameSuffix = "Serialize";
private const string ValueVarName = "value";
private const string WriterVarName = "writer";

private static AssemblyName _assemblyName = typeof(Emitter).Assembly.GetName();
private static readonly string s_generatedCodeAttributeSource = $@"
Expand Down Expand Up @@ -99,15 +101,24 @@ public void Emit()
{
_currentContext = contextGenerationSpec;

bool generateGetConverterMethodForTypes = false;
bool generateGetConverterMethodForProperties = false;

foreach (TypeGenerationSpec typeGenerationSpec in _currentContext.RootSerializableTypes)
{
GenerateTypeInfo(typeGenerationSpec);

generateGetConverterMethodForTypes |= typeGenerationSpec.HasTypeFactoryConverter;
generateGetConverterMethodForProperties |= typeGenerationSpec.HasPropertyFactoryConverters;
}

string contextName = _currentContext.ContextType.Name;

// Add root context implementation.
AddSource($"{contextName}.g.cs", GetRootJsonContextImplementation(), isRootContextDef: true);
AddSource(
$"{contextName}.g.cs",
GetRootJsonContextImplementation(generateGetConverterMethodForTypes, generateGetConverterMethodForProperties),
isRootContextDef: true);

// Add GetJsonTypeInfo override implementation.
AddSource($"{contextName}.GetJsonTypeInfo.g.cs", GetGetTypeInfoImplementation());
Expand Down Expand Up @@ -283,54 +294,42 @@ private string GenerateForTypeWithUnknownConverter(TypeGenerationSpec typeMetada
string typeCompilableName = typeMetadata.TypeRef;
string typeFriendlyName = typeMetadata.TypeInfoPropertyName;

string metadataInitSource;

// TODO (https://github.com/dotnet/runtime/issues/52218): consider moving this verification source to common helper.
StringBuilder metadataInitSource = new(
$@"{JsonConverterTypeRef} converter = {typeMetadata.ConverterInstantiationLogic};
{TypeTypeRef} typeToConvert = typeof({typeCompilableName});");

if (typeMetadata.IsValueType)
{
metadataInitSource = $@"{JsonConverterTypeRef} converter = {typeMetadata.ConverterInstantiationLogic};
{TypeTypeRef} typeToConvert = typeof({typeCompilableName});
if (!converter.CanConvert(typeToConvert))
{{
{TypeTypeRef}? underlyingType = {NullableTypeRef}.GetUnderlyingType(typeToConvert);
if (underlyingType != null && converter.CanConvert(underlyingType))
metadataInitSource.Append($@"
if (!converter.CanConvert(typeToConvert))
{{
{JsonConverterTypeRef}? actualConverter = converter;

if (converter is {JsonConverterFactoryTypeRef} converterFactory)
{TypeTypeRef}? underlyingType = {NullableTypeRef}.GetUnderlyingType(typeToConvert);
if (underlyingType != null && converter.CanConvert(underlyingType))
{{
actualConverter = converterFactory.CreateConverter(underlyingType, {OptionsInstanceVariableName});

if (actualConverter == null || actualConverter is {JsonConverterFactoryTypeRef})
{{
throw new {InvalidOperationExceptionTypeRef}($""JsonConverterFactory '{{converter}} cannot return a 'null' or 'JsonConverterFactory' value."");
}}
// Allow nullable handling to forward to the underlying type's converter.
converter = {JsonMetadataServicesTypeRef}.GetNullableConverter<{typeCompilableName}>(this.{typeFriendlyName})!;
converter = (({ JsonConverterFactoryTypeRef })converter).CreateConverter(typeToConvert, { OptionsInstanceVariableName })!;
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
}}

// Allow nullable handling to forward to the underlying type's converter.
converter = {JsonMetadataServicesTypeRef}.GetNullableConverter<{typeCompilableName}>(this.{typeFriendlyName});
}}
else
{{
throw new {InvalidOperationExceptionTypeRef}($""The converter '{{converter.GetType()}}' is not compatible with the type '{{typeToConvert}}'."");
}}
}}

_{typeFriendlyName} = {JsonMetadataServicesTypeRef}.{GetCreateValueInfoMethodRef(typeCompilableName)}({OptionsInstanceVariableName}, converter);";
else
{{
throw new {InvalidOperationExceptionTypeRef}($""The converter '{{converter.GetType()}}' is not compatible with the type '{{typeToConvert}}'."");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant to the scope of this PR, but here is another instance of non-localized error messages: #58292.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a run-time exception, not design-time, I think it should remain non-localized.

}}
}}");
}
else
{
metadataInitSource = $@"{JsonConverterTypeRef} converter = {typeMetadata.ConverterInstantiationLogic};
{TypeTypeRef} typeToConvert = typeof({typeCompilableName});
if (!converter.CanConvert(typeToConvert))
{{
throw new {InvalidOperationExceptionTypeRef}($""The converter '{{converter.GetType()}}' is not compatible with the type '{{typeToConvert}}'."");
}}

_{typeFriendlyName} = {JsonMetadataServicesTypeRef}.{GetCreateValueInfoMethodRef(typeCompilableName)}({OptionsInstanceVariableName}, converter);";
metadataInitSource.Append($@"
if (!converter.CanConvert(typeToConvert))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this branch type test for converter factories?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't... The code before that that is shared between value types and reference types will call GetConverterFromFactory() for factories.

Also the error check for CanConvert was pre-existing from this PR. It applies to factory and non-factory cases although it may not be necessary for the factory case, as I'm not sure we double-check this in the serializer case (i.e. I don't think we verify the result from factory.CreateConverter()).

{{
throw new {InvalidOperationExceptionTypeRef}($""The converter '{{converter.GetType()}}' is not compatible with the type '{{typeToConvert}}'."");
}}");
}

return GenerateForType(typeMetadata, metadataInitSource);
metadataInitSource.Append($@"
_{typeFriendlyName} = { JsonMetadataServicesTypeRef }.{ GetCreateValueInfoMethodRef(typeCompilableName)} ({ OptionsInstanceVariableName}, converter); ");

return GenerateForType(typeMetadata, metadataInitSource.ToString());
}

private string GenerateForNullable(TypeGenerationSpec typeMetadata)
Expand Down Expand Up @@ -484,7 +483,7 @@ private string GenerateFastPathFuncForEnumerable(TypeGenerationSpec typeGenerati

Type elementType = valueTypeGenerationSpec.Type;
string? writerMethodToCall = GetWriterMethod(elementType);

string iterationLogic;
string valueToWrite;

Expand Down Expand Up @@ -638,7 +637,6 @@ private string GenerateForObject(TypeGenerationSpec typeMetadata)
private string GeneratePropMetadataInitFunc(TypeGenerationSpec typeGenerationSpec)
{
const string PropVarName = "properties";
const string JsonContextVarName = "jsonContext";

List<PropertyGenerationSpec> properties = typeGenerationSpec.PropertyGenSpecList!;

Expand Down Expand Up @@ -1093,7 +1091,9 @@ private string WrapWithCheckForCustomConverterIfRequired(string source, string t
}}";
}

private string GetRootJsonContextImplementation()
private string GetRootJsonContextImplementation(
bool generateGetConverterMethodForTypes,
bool generateGetConverterMethodForProperties)
{
string contextTypeRef = _currentContext.ContextTypeRef;
string contextTypeName = _currentContext.ContextType.Name;
Expand All @@ -1115,6 +1115,16 @@ private string GetRootJsonContextImplementation()

{GetFetchLogicForRuntimeSpecifiedCustomConverter()}");

if (generateGetConverterMethodForProperties)
{
sb.Append(GetFetchLogicForGetCustomConverter_PropertiesWithFactories());
}

if (generateGetConverterMethodForProperties || generateGetConverterMethodForTypes)
{
sb.Append(GetFetchLogicForGetCustomConverter_TypesWithFactories());
}

return sb.ToString();
}

Expand Down Expand Up @@ -1173,6 +1183,32 @@ private string GetFetchLogicForRuntimeSpecifiedCustomConverter()
}}";
}

private string GetFetchLogicForGetCustomConverter_PropertiesWithFactories()
{
return @$"

private {JsonConverterTypeRef}<T> {GetConverterFromFactoryMethodName}<T>({JsonConverterFactoryTypeRef} factory)
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
{{
return ({JsonConverterTypeRef}<T>) {GetConverterFromFactoryMethodName}(typeof(T), factory);
}}";
}

private string GetFetchLogicForGetCustomConverter_TypesWithFactories()
{
return @$"

private {JsonConverterTypeRef} {GetConverterFromFactoryMethodName}({TypeTypeRef} type, {JsonConverterFactoryTypeRef} factory)
{{
{JsonConverterTypeRef}? converter = factory.CreateConverter(type, {Emitter.OptionsInstanceVariableName});
if (converter == null || converter is {JsonConverterFactoryTypeRef})
{{
throw new {InvalidOperationExceptionTypeRef}($""The converter '{{factory.GetType()}}' cannot return null or a JsonConverterFactory instance."");
}}

return converter;
}}";
}

private string GetGetTypeInfoImplementation()
{
StringBuilder sb = new();
Expand Down
Loading