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

Implement JsonStringEnumConverter<TEnum> #87149

Closed
wants to merge 10 commits into from
Closed

Implement JsonStringEnumConverter<TEnum> #87149

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
efd71ca
Fix NativeAOT support for JsonStringEnumConverter.
eiriktsarpalis Jun 5, 2023
c4fb01f
Fix merge issue.
eiriktsarpalis Jun 5, 2023
1e45d00
Implement JsonStringEnumConverter<TEnum>.
eiriktsarpalis Jun 6, 2023
dfe0de6
Remove leftover comments.
eiriktsarpalis Jun 6, 2023
ca10408
Revert unnecessary suppression.
eiriktsarpalis Jun 6, 2023
543b9e4
Update compatibility suppressions.
eiriktsarpalis Jun 6, 2023
d6ea143
Add missing XML docs
eiriktsarpalis Jun 6, 2023
37c6b6f
Fix capitalization.
eiriktsarpalis Jun 6, 2023
aede061
add local project compatibility suppressions
eiriktsarpalis Jun 7, 2023
a6a0475
Update src/libraries/System.Text.Json/ref/System.Text.Json.cs
eiriktsarpalis Jun 7, 2023
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
3 changes: 3 additions & 0 deletions src/libraries/System.Text.Json/gen/Helpers/KnownTypeSymbols.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,9 @@ public KnownTypeSymbols(Compilation compilation)
public INamedTypeSymbol? JsonUnmappedMemberHandlingAttributeType => GetOrResolveType("System.Text.Json.Serialization.JsonUnmappedMemberHandlingAttribute", ref _JsonUnmappedMemberHandlingAttributeType);
private Option<INamedTypeSymbol?> _JsonUnmappedMemberHandlingAttributeType;

public INamedTypeSymbol? JsonStringEnumConverterType => GetOrResolveType("System.Text.Json.Serialization.JsonStringEnumConverter", ref _JsonStringEnumConverterType);
private Option<INamedTypeSymbol?> _JsonStringEnumConverterType;
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved

// Unsupported types
public INamedTypeSymbol? DelegateType => _DelegateType ??= Compilation.GetSpecialType(SpecialType.System_Delegate);
private INamedTypeSymbol? _DelegateType;
Expand Down
8 changes: 8 additions & 0 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.DiagnosticDescriptors.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ internal static class DiagnosticDescriptors
category: JsonConstants.SystemTextJsonSourceGenerationName,
defaultSeverity: DiagnosticSeverity.Warning,
isEnabledByDefault: true);

public static DiagnosticDescriptor JsonStringEnumConverterNotSupportedInAot { get; } = new DiagnosticDescriptor(
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this new diagnostic? Isn't the RequiresDynamicCode attribute on JsonStringEnumConverter enough to devs that JsonStringEnumConverter is not supported in native AOT?

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 was added in reaction to #87149 (comment) not triggering at all either at design time or at compile time. If we can get to the point that this always warns then I could remove that diagnostic.

Copy link
Member

Choose a reason for hiding this comment

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

That comment will not be a problem in publish - both trimmer and AOT compiler will correctly warn if necessary.
The only "hole" is in the analyzer - which will not report this warning currently.
That said - if the specific diagnostic will always provide better UX than the more general RDC warning (even if the analyzer did report it) - so it might be worth keeping this just for the better UX.

Downside is that if we fix the analyzer this might produce two warnings.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we deprecate the diagnostic in the future if the analyzer does get fixed?

Copy link
Member

Choose a reason for hiding this comment

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

Probably - we may need to figure out some versioning stuff, but it shouldn't be too hard.

id: "SYSLIB1040",
title: new LocalizableResourceString(nameof(SR.JsonStringEnumConverterNotSupportedTitle), SR.ResourceManager, typeof(FxResources.System.Text.Json.SourceGeneration.SR)),
messageFormat: new LocalizableResourceString(nameof(SR.JsonStringEnumConverterNotSupportedMessageFormat), SR.ResourceManager, typeof(FxResources.System.Text.Json.SourceGeneration.SR)),
category: JsonConstants.SystemTextJsonSourceGenerationName,
defaultSeverity: DiagnosticSeverity.Warning,
isEnabledByDefault: true);
}
}
}
111 changes: 69 additions & 42 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 @@ -225,8 +225,13 @@ private static SourceText GenerateForTypeWithBuiltInConverter(ContextGenerationS

string typeFQN = typeMetadata.TypeRef.FullyQualifiedName;
string typeInfoPropertyName = typeMetadata.TypeInfoPropertyName;
string metadataInitSource = $"{JsonTypeInfoReturnValueLocalVariableName} = {JsonMetadataServicesTypeRef}.{GetCreateValueInfoMethodRef(typeFQN)}({OptionsLocalVariableName}, {JsonMetadataServicesTypeRef}.{typeInfoPropertyName}Converter);";
GenerateTypeInfoProperty(writer, typeMetadata, metadataInitSource);

GenerateTypeInfoFactoryHeader(writer, typeMetadata);
writer.WriteLine($"""
{JsonTypeInfoReturnValueLocalVariableName} = {JsonMetadataServicesTypeRef}.{GetCreateValueInfoMethodRef(typeFQN)}({OptionsLocalVariableName}, {JsonMetadataServicesTypeRef}.{typeInfoPropertyName}Converter);
""");

GenerateTypeInfoFactoryFooter(writer);

return CompleteSourceFileAndReturnText(writer);
}
Expand All @@ -238,12 +243,16 @@ private static SourceText GenerateForTypeWithCustomConverter(ContextGenerationSp
SourceWriter writer = CreateSourceWriterWithContextHeader(contextSpec);

string typeFQN = typeMetadata.TypeRef.FullyQualifiedName;
string metadataInitSource = $$"""
{{JsonConverterTypeRef}} converter = {{ExpandConverterMethodName}}(typeof({{typeFQN}}), new {{typeMetadata.ConverterType.FullyQualifiedName}}(), {{OptionsLocalVariableName}});
{{JsonTypeInfoReturnValueLocalVariableName}} = {{JsonMetadataServicesTypeRef}}.{{GetCreateValueInfoMethodRef(typeFQN)}} ({{OptionsLocalVariableName}}, converter);
""";
string converterFQN = typeMetadata.ConverterType.FullyQualifiedName;

GenerateTypeInfoFactoryHeader(writer, typeMetadata);

GenerateTypeInfoProperty(writer, typeMetadata, metadataInitSource);
writer.WriteLine($"""
{JsonConverterTypeRef} converter = {ExpandConverterMethodName}(typeof({typeFQN}), new {converterFQN}(), {OptionsLocalVariableName});
{JsonTypeInfoReturnValueLocalVariableName} = {JsonMetadataServicesTypeRef}.{GetCreateValueInfoMethodRef(typeFQN)} ({OptionsLocalVariableName}, converter);
""");

GenerateTypeInfoFactoryFooter(writer);

return CompleteSourceFileAndReturnText(writer);
}
Expand All @@ -257,13 +266,14 @@ private static SourceText GenerateForNullable(ContextGenerationSpec contextSpec,
string typeFQN = typeMetadata.TypeRef.FullyQualifiedName;
string underlyingTypeFQN = typeMetadata.NullableUnderlyingType.FullyQualifiedName;

string metadataInitSource = $$"""
{{JsonTypeInfoReturnValueLocalVariableName}} = {{JsonMetadataServicesTypeRef}}.{{GetCreateValueInfoMethodRef(typeFQN)}}(
{{OptionsLocalVariableName}},
{{JsonMetadataServicesTypeRef}}.GetNullableConverter<{{underlyingTypeFQN}}>({{OptionsLocalVariableName}}));
""";
GenerateTypeInfoFactoryHeader(writer, typeMetadata);

writer.WriteLine($$"""
{{JsonConverterTypeRef}} converter = {{JsonMetadataServicesTypeRef}}.GetNullableConverter<{{underlyingTypeFQN}}>({{OptionsLocalVariableName}});
{{JsonTypeInfoReturnValueLocalVariableName}} = {{JsonMetadataServicesTypeRef}}.{{GetCreateValueInfoMethodRef(typeFQN)}}({{OptionsLocalVariableName}}, converter);
""");

GenerateTypeInfoProperty(writer, typeMetadata, metadataInitSource);
GenerateTypeInfoFactoryFooter(writer);

return CompleteSourceFileAndReturnText(writer);
}
Expand All @@ -273,9 +283,13 @@ private static SourceText GenerateForUnsupportedType(ContextGenerationSpec conte
SourceWriter writer = CreateSourceWriterWithContextHeader(contextSpec);

string typeFQN = typeMetadata.TypeRef.FullyQualifiedName;
string metadataInitSource = $"{JsonTypeInfoReturnValueLocalVariableName} = {JsonMetadataServicesTypeRef}.{GetCreateValueInfoMethodRef(typeFQN)}({OptionsLocalVariableName}, {JsonMetadataServicesTypeRef}.GetUnsupportedTypeConverter<{typeFQN}>());";

GenerateTypeInfoProperty(writer, typeMetadata, metadataInitSource);
GenerateTypeInfoFactoryHeader(writer, typeMetadata);
writer.WriteLine($"""
{JsonTypeInfoReturnValueLocalVariableName} = {JsonMetadataServicesTypeRef}.{GetCreateValueInfoMethodRef(typeFQN)}({OptionsLocalVariableName}, {JsonMetadataServicesTypeRef}.GetUnsupportedTypeConverter<{typeFQN}>());
""");

GenerateTypeInfoFactoryFooter(writer);

return CompleteSourceFileAndReturnText(writer);
}
Expand All @@ -285,8 +299,13 @@ private static SourceText GenerateForEnum(ContextGenerationSpec contextSpec, Typ
SourceWriter writer = CreateSourceWriterWithContextHeader(contextSpec);

string typeFQN = typeMetadata.TypeRef.FullyQualifiedName;
string metadataInitSource = $"{JsonTypeInfoReturnValueLocalVariableName} = {JsonMetadataServicesTypeRef}.{GetCreateValueInfoMethodRef(typeFQN)}({OptionsLocalVariableName}, {JsonMetadataServicesTypeRef}.GetEnumConverter<{typeFQN}>({OptionsLocalVariableName}));";
GenerateTypeInfoProperty(writer, typeMetadata, metadataInitSource);

GenerateTypeInfoFactoryHeader(writer, typeMetadata);
writer.WriteLine($"""
{JsonTypeInfoReturnValueLocalVariableName} = {JsonMetadataServicesTypeRef}.{GetCreateValueInfoMethodRef(typeFQN)}({OptionsLocalVariableName}, {JsonMetadataServicesTypeRef}.GetEnumConverter<{typeFQN}>({OptionsLocalVariableName}));
""");

GenerateTypeInfoFactoryFooter(writer);

return CompleteSourceFileAndReturnText(writer);
}
Expand Down Expand Up @@ -349,18 +368,20 @@ private SourceText GenerateForCollection(ContextGenerationSpec contextSpec, Type
break;
}

string metadataInitSource = $$"""
var {{InfoVarName}} = new {{JsonCollectionInfoValuesTypeRef}}<{{typeFQN}}>()
GenerateTypeInfoFactoryHeader(writer, typeGenerationSpec);

writer.WriteLine($$"""
var {{InfoVarName}} = new {{JsonCollectionInfoValuesTypeRef}}<{{typeFQN}}>
{
{{ObjectCreatorPropName}} = {{FormatDefaultConstructorExpr(typeGenerationSpec)}},
{{NumberHandlingPropName}} = {{GetNumberHandlingAsStr(typeGenerationSpec.NumberHandling)}},
{{SerializeHandlerPropName}} = {{serializeMethodName ?? "null"}}
};

{{JsonTypeInfoReturnValueLocalVariableName}} = {{JsonMetadataServicesTypeRef}}.{{createCollectionMethodExpr}};
""";
""");

GenerateTypeInfoProperty(writer, typeGenerationSpec, metadataInitSource);
GenerateTypeInfoFactoryFooter(writer);

if (serializeMethodName != null)
{
Expand Down Expand Up @@ -491,8 +512,10 @@ private SourceText GenerateForObject(ContextGenerationSpec contextSpec, TypeGene
const string ObjectInfoVarName = "objectInfo";
string genericArg = typeMetadata.TypeRef.FullyQualifiedName;

string metadataInitSource = $$"""
var {{ObjectInfoVarName}} = new {{JsonObjectInfoValuesTypeRef}}<{{genericArg}}>()
GenerateTypeInfoFactoryHeader(writer, typeMetadata);

writer.WriteLine($$"""
var {{ObjectInfoVarName}} = new {{JsonObjectInfoValuesTypeRef}}<{{genericArg}}>
{
{{ObjectCreatorPropName}} = {{creatorInvocation}},
ObjectWithParameterizedConstructorCreator = {{parameterizedCreatorInvocation}},
Expand All @@ -503,25 +526,24 @@ private SourceText GenerateForObject(ContextGenerationSpec contextSpec, TypeGene
};

{{JsonTypeInfoReturnValueLocalVariableName}} = {{JsonMetadataServicesTypeRef}}.CreateObjectInfo<{{typeMetadata.TypeRef.FullyQualifiedName}}>({{OptionsLocalVariableName}}, {{ObjectInfoVarName}});
""";
""");

if (typeMetadata.UnmappedMemberHandling != null)
if (typeMetadata is { UnmappedMemberHandling: not null } or { PreferredPropertyObjectCreationHandling: not null })
{
metadataInitSource += $"""

{JsonTypeInfoReturnValueLocalVariableName}.{UnmappedMemberHandlingPropName} = {GetUnmappedMemberHandlingAsStr(typeMetadata.UnmappedMemberHandling.Value)};
""";
}
writer.WriteLine();

if (typeMetadata.PreferredPropertyObjectCreationHandling != null)
{
metadataInitSource += $"""
if (typeMetadata.UnmappedMemberHandling != null)
{
writer.WriteLine($"{JsonTypeInfoReturnValueLocalVariableName}.{UnmappedMemberHandlingPropName} = {GetUnmappedMemberHandlingAsStr(typeMetadata.UnmappedMemberHandling.Value)};");
}

{JsonTypeInfoReturnValueLocalVariableName}.{PreferredPropertyObjectCreationHandlingPropName} = {GetObjectCreationHandlingAsStr(typeMetadata.PreferredPropertyObjectCreationHandling.Value)};
""";
if (typeMetadata.PreferredPropertyObjectCreationHandling != null)
{
writer.WriteLine($"{JsonTypeInfoReturnValueLocalVariableName}.{PreferredPropertyObjectCreationHandlingPropName} = {GetObjectCreationHandlingAsStr(typeMetadata.PreferredPropertyObjectCreationHandling.Value)};");
}
}

GenerateTypeInfoProperty(writer, typeMetadata, metadataInitSource);
GenerateTypeInfoFactoryFooter(writer);

if (propInitMethodName != null)
{
Expand Down Expand Up @@ -596,15 +618,17 @@ private void GeneratePropMetadataInitFunc(SourceWriter writer, string propInitMe
string? converterInstantiationExpr = null;
if (property.ConverterType != null)
{
string converterFQN = property.ConverterType.FullyQualifiedName;
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})";
? $"{GetConverterForNullablePropertyMethodName}<{nullableUnderlyingType.FullyQualifiedName}>(new {converterFQN}(), {OptionsLocalVariableName})"
: $"({JsonConverterTypeRef}<{propertyTypeFQN}>){ExpandConverterMethodName}(typeof({propertyTypeFQN}), new {converterFQN}(), {OptionsLocalVariableName})";
}

writer.WriteLine($$"""
var {{InfoVarName}}{{i}} = new {{JsonPropertyInfoValuesTypeRef}}<{{propertyTypeFQN}}>()
var {{InfoVarName}}{{i}} = new {{JsonPropertyInfoValuesTypeRef}}<{{propertyTypeFQN}}>
{
IsProperty = {{FormatBool(property.IsProperty)}},
IsPublic = {{FormatBool(property.IsPublic)}},
Expand Down Expand Up @@ -986,7 +1010,7 @@ private static DefaultCheckType GetDefaultCheckType(ContextGenerationSpec contex
};
}

private static void GenerateTypeInfoProperty(SourceWriter writer, TypeGenerationSpec typeMetadata, string metadataInitSource)
private static void GenerateTypeInfoFactoryHeader(SourceWriter writer, TypeGenerationSpec typeMetadata)
{
string typeFQN = typeMetadata.TypeRef.FullyQualifiedName;
string typeInfoPropertyName = typeMetadata.TypeInfoPropertyName;
Expand All @@ -1010,15 +1034,18 @@ private static void GenerateTypeInfoProperty(SourceWriter writer, TypeGeneration
""");

writer.Indentation += 2;
writer.WriteLine(metadataInitSource);
}

private static void GenerateTypeInfoFactoryFooter(SourceWriter writer)
{
writer.Indentation -= 2;

// NB OriginatingResolver should be the last property set by the source generator.
writer.WriteLine($$"""
}

{{JsonTypeInfoReturnValueLocalVariableName}}.{{OriginatingResolverPropertyName}} = this;
return jsonTypeInfo;
return {{JsonTypeInfoReturnValueLocalVariableName}};
}
""");
}
Expand Down
Loading