Skip to content

Support reference type custom converters in source gen #57592

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

Merged
merged 1 commit into from
Aug 18, 2021
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
18 changes: 16 additions & 2 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -277,10 +277,12 @@ private string GenerateForTypeWithUnknownConverter(TypeGenerationSpec typeMetada
string typeCompilableName = typeMetadata.TypeRef;
string typeFriendlyName = typeMetadata.TypeInfoPropertyName;

StringBuilder sb = new();
string metadataInitSource;

// TODO (https://github.com/dotnet/runtime/issues/52218): consider moving this verification source to common helper.
string metadataInitSource = $@"{JsonConverterTypeRef} converter = {typeMetadata.ConverterInstantiationLogic};
if (typeMetadata.IsValueType)
{
metadataInitSource = $@"{JsonConverterTypeRef} converter = {typeMetadata.ConverterInstantiationLogic};
{TypeTypeRef} typeToConvert = typeof({typeCompilableName});
if (!converter.CanConvert(typeToConvert))
{{
Copy link
Member

@eiriktsarpalis eiriktsarpalis Aug 18, 2021

Choose a reason for hiding this comment

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

Nit: given that there is a some amount of duplication between the ref type and value type generated code, is there any way we could splice in the nullable check, i.e some thing like

if (!converter.CanConvert(typeToConvert))
{{
     {!typeof(T).IsValueType ? "" : EmitNullableTypeHandling()}

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a fair suggestion, and I'll leave it up to @layomia to change it if desired. However, in this case I think it is more readable\understandable to have separate ifs since the amount of emit duplication is only 5 LOC or so.

Expand Down Expand Up @@ -309,6 +311,18 @@ private string GenerateForTypeWithUnknownConverter(TypeGenerationSpec typeMetada
}}

_{typeFriendlyName} = {JsonMetadataServicesTypeRef}.{GetCreateValueInfoMethodRef(typeCompilableName)}({OptionsInstanceVariableName}, converter);";
}
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);";
}

return GenerateForType(typeMetadata, metadataInitSource);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ public interface ITestContext
public JsonTypeInfo<object[]> ObjectArray { get; }
public JsonTypeInfo<string> String { get; }
public JsonTypeInfo<RealWorldContextTests.ClassWithEnumAndNullable> ClassWithEnumAndNullable { get; }
public JsonTypeInfo<ClassWithCustomConverter> ClassWithCustomConverter { get; }
public JsonTypeInfo<StructWithCustomConverter> StructWithCustomConverter { get; }
public JsonTypeInfo<ClassWithBadCustomConverter> ClassWithBadCustomConverter { get; }
public JsonTypeInfo<StructWithBadCustomConverter> StructWithBadCustomConverter { get; }
}

internal partial class JsonContext : JsonSerializerContext
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ namespace System.Text.Json.SourceGeneration.Tests
[JsonSerializable(typeof(object[]))]
[JsonSerializable(typeof(string))]
[JsonSerializable(typeof(RealWorldContextTests.ClassWithEnumAndNullable))]
[JsonSerializable(typeof(ClassWithCustomConverter))]
[JsonSerializable(typeof(StructWithCustomConverter))]
[JsonSerializable(typeof(ClassWithBadCustomConverter))]
[JsonSerializable(typeof(StructWithBadCustomConverter))]
internal partial class MetadataAndSerializationContext : JsonSerializerContext, ITestContext
{
}
Expand Down Expand Up @@ -58,6 +62,10 @@ public override void EnsureFastPathGeneratedAsExpected()
Assert.Null(MetadataAndSerializationContext.Default.ObjectArray.Serialize);
Assert.Null(MetadataAndSerializationContext.Default.String.Serialize);
Assert.NotNull(MetadataAndSerializationContext.Default.ClassWithEnumAndNullable.Serialize);
Assert.NotNull(MetadataAndSerializationContext.Default.ClassWithCustomConverter);
Assert.NotNull(MetadataAndSerializationContext.Default.StructWithCustomConverter);
Assert.Throws<InvalidOperationException>(() => MetadataAndSerializationContext.Default.ClassWithBadCustomConverter);
Assert.Throws<InvalidOperationException>(() => MetadataAndSerializationContext.Default.StructWithBadCustomConverter);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ namespace System.Text.Json.SourceGeneration.Tests
[JsonSerializable(typeof(object[]), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(string), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(RealWorldContextTests.ClassWithEnumAndNullable), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(ClassWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(StructWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(ClassWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(StructWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(ClassWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(StructWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
internal partial class MetadataWithPerTypeAttributeContext : JsonSerializerContext, ITestContext
{
}
Expand Down Expand Up @@ -55,6 +61,10 @@ public override void EnsureFastPathGeneratedAsExpected()
Assert.Null(MetadataWithPerTypeAttributeContext.Default.ObjectArray.Serialize);
Assert.Null(MetadataWithPerTypeAttributeContext.Default.String.Serialize);
Assert.Null(MetadataWithPerTypeAttributeContext.Default.ClassWithEnumAndNullable.Serialize);
Assert.Null(MetadataWithPerTypeAttributeContext.Default.ClassWithCustomConverter.Serialize);
Assert.Null(MetadataWithPerTypeAttributeContext.Default.StructWithCustomConverter.Serialize);
Assert.Throws<InvalidOperationException>(() => MetadataWithPerTypeAttributeContext.Default.ClassWithBadCustomConverter.Serialize);
Assert.Throws<InvalidOperationException>(() => MetadataWithPerTypeAttributeContext.Default.StructWithBadCustomConverter.Serialize);
}
}

Expand All @@ -79,6 +89,10 @@ public override void EnsureFastPathGeneratedAsExpected()
[JsonSerializable(typeof(object[]))]
[JsonSerializable(typeof(string))]
[JsonSerializable(typeof(RealWorldContextTests.ClassWithEnumAndNullable))]
[JsonSerializable(typeof(ClassWithCustomConverter))]
[JsonSerializable(typeof(StructWithCustomConverter))]
[JsonSerializable(typeof(ClassWithBadCustomConverter))]
[JsonSerializable(typeof(StructWithBadCustomConverter))]
internal partial class MetadataContext : JsonSerializerContext, ITestContext
{
}
Expand Down Expand Up @@ -110,6 +124,10 @@ public override void EnsureFastPathGeneratedAsExpected()
Assert.Null(MetadataContext.Default.ObjectArray.Serialize);
Assert.Null(MetadataContext.Default.String.Serialize);
Assert.Null(MetadataContext.Default.ClassWithEnumAndNullable.Serialize);
Assert.Null(MetadataContext.Default.ClassWithCustomConverter.Serialize);
Assert.Null(MetadataContext.Default.StructWithCustomConverter.Serialize);
Assert.Throws<InvalidOperationException>(() => MetadataContext.Default.ClassWithBadCustomConverter.Serialize);
Assert.Throws<InvalidOperationException>(() => MetadataContext.Default.StructWithBadCustomConverter.Serialize);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ namespace System.Text.Json.SourceGeneration.Tests
[JsonSerializable(typeof(object[]), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(string), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(RealWorldContextTests.ClassWithEnumAndNullable), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(StructWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(StructWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
internal partial class MixedModeContext : JsonSerializerContext, ITestContext
{
}
Expand Down Expand Up @@ -56,6 +60,10 @@ public override void EnsureFastPathGeneratedAsExpected()
Assert.Null(MixedModeContext.Default.ObjectArray.Serialize);
Assert.Null(MixedModeContext.Default.String.Serialize);
Assert.NotNull(MixedModeContext.Default.ClassWithEnumAndNullable.Serialize);
Assert.Null(MixedModeContext.Default.ClassWithCustomConverter.Serialize);
Assert.Null(MixedModeContext.Default.StructWithCustomConverter.Serialize);
Assert.Throws<InvalidOperationException>(() => MixedModeContext.Default.ClassWithBadCustomConverter.Serialize);
Assert.Throws<InvalidOperationException>(() => MixedModeContext.Default.StructWithBadCustomConverter.Serialize);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,64 @@ public virtual void RoundTripTypeNameClash()
VerifyRepeatedLocation(expected, obj);
}

[Fact]
public virtual void RoundTripWithCustomConverter_Class()
{
const string Json = "{\"MyInt\":142}";

ClassWithCustomConverter obj = new ClassWithCustomConverter()
{
MyInt = 42
};

string json = JsonSerializer.Serialize(obj, DefaultContext.ClassWithCustomConverter);
Assert.Equal(Json, json);

obj = JsonSerializer.Deserialize(Json, DefaultContext.ClassWithCustomConverter);
Assert.Equal(42, obj.MyInt);
}

[Fact]
public virtual void RoundTripWithCustomConverter_Struct()
{
const string Json = "{\"MyInt\":142}";

StructWithCustomConverter obj = new StructWithCustomConverter()
{
MyInt = 42
};

string json = JsonSerializer.Serialize(obj, DefaultContext.StructWithCustomConverter);
Assert.Equal(Json, json);

obj = JsonSerializer.Deserialize(Json, DefaultContext.StructWithCustomConverter);
Assert.Equal(42, obj.MyInt);
}

[Fact]
public virtual void BadCustomConverter_Class()
{
const string Json = "{\"MyInt\":142}";

Assert.Throws<InvalidOperationException>(() =>
JsonSerializer.Serialize(new ClassWithBadCustomConverter(), DefaultContext.ClassWithBadCustomConverter));

Assert.Throws<InvalidOperationException>(() =>
JsonSerializer.Deserialize(Json, DefaultContext.ClassWithBadCustomConverter));
}

[Fact]
public virtual void BadCustomConverter_Struct()
{
const string Json = "{\"MyInt\":142}";

Assert.Throws<InvalidOperationException>(() =>
JsonSerializer.Serialize(new StructWithBadCustomConverter(), DefaultContext.StructWithBadCustomConverter));

Assert.Throws<InvalidOperationException>(() =>
JsonSerializer.Deserialize(Json, DefaultContext.StructWithBadCustomConverter));
}

protected static Location CreateLocation()
{
return new Location
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ namespace System.Text.Json.SourceGeneration.Tests
[JsonSerializable(typeof(object[]))]
[JsonSerializable(typeof(string))]
[JsonSerializable(typeof(RealWorldContextTests.ClassWithEnumAndNullable))]
[JsonSerializable(typeof(ClassWithCustomConverter))]
[JsonSerializable(typeof(StructWithCustomConverter))]
[JsonSerializable(typeof(ClassWithBadCustomConverter))]
[JsonSerializable(typeof(StructWithBadCustomConverter))]
internal partial class SerializationContext : JsonSerializerContext, ITestContext
{
}
Expand All @@ -51,6 +55,12 @@ internal partial class SerializationContext : JsonSerializerContext, ITestContex
[JsonSerializable(typeof(object[]), GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(string), GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(RealWorldContextTests.ClassWithEnumAndNullable), GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(StructWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(StructWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(StructWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
internal partial class SerializationWithPerTypeAttributeContext : JsonSerializerContext, ITestContext
{
}
Expand All @@ -76,6 +86,10 @@ internal partial class SerializationWithPerTypeAttributeContext : JsonSerializer
[JsonSerializable(typeof(object[]), GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(string), GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(RealWorldContextTests.ClassWithEnumAndNullable), GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(StructWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(StructWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
internal partial class SerializationContextWithCamelCase : JsonSerializerContext, ITestContext
{
}
Expand Down Expand Up @@ -112,6 +126,10 @@ public override void EnsureFastPathGeneratedAsExpected()
Assert.Null(SerializationContext.Default.ObjectArray.Serialize);
Assert.Null(SerializationContext.Default.String.Serialize);
Assert.NotNull(SerializationContext.Default.ClassWithEnumAndNullable.Serialize);
Assert.Null(SerializationContext.Default.ClassWithCustomConverter.Serialize);
Assert.Null(SerializationContext.Default.StructWithCustomConverter.Serialize);
Assert.Throws<InvalidOperationException>(() => SerializationContext.Default.ClassWithBadCustomConverter.Serialize);
Assert.Throws<InvalidOperationException>(() => SerializationContext.Default.StructWithBadCustomConverter.Serialize);
}

[Fact]
Expand Down Expand Up @@ -370,6 +388,10 @@ public override void EnsureFastPathGeneratedAsExpected()
Assert.Null(SerializationWithPerTypeAttributeContext.Default.ObjectArray.Serialize);
Assert.Null(SerializationWithPerTypeAttributeContext.Default.String.Serialize);
Assert.NotNull(SerializationWithPerTypeAttributeContext.Default.ClassWithEnumAndNullable.Serialize);
Assert.Null(SerializationWithPerTypeAttributeContext.Default.ClassWithCustomConverter.Serialize);
Assert.Null(SerializationWithPerTypeAttributeContext.Default.StructWithCustomConverter.Serialize);
Assert.Throws<InvalidOperationException>(() => SerializationWithPerTypeAttributeContext.Default.ClassWithBadCustomConverter.Serialize);
Assert.Throws<InvalidOperationException>(() => SerializationWithPerTypeAttributeContext.Default.StructWithBadCustomConverter.Serialize);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,110 @@ public class JsonMessage
}

internal struct MyStruct { }

/// <summary>
/// Custom converter that adds\substract 100 from MyIntProperty.
/// </summary>
public class CustomConverterForClass : JsonConverter<ClassWithCustomConverter>
{
public override ClassWithCustomConverter Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
if (reader.TokenType != JsonTokenType.StartObject)
{
throw new JsonException("No StartObject");
}

ClassWithCustomConverter obj = new();

reader.Read();
if (reader.TokenType != JsonTokenType.PropertyName &&
reader.GetString() != "MyInt")
{
throw new JsonException("Wrong property name");
}

reader.Read();
obj.MyInt = reader.GetInt32() - 100;

reader.Read();
if (reader.TokenType != JsonTokenType.EndObject)
{
throw new JsonException("No EndObject");
}

return obj;
}

public override void Write(Utf8JsonWriter writer, ClassWithCustomConverter value, JsonSerializerOptions options)
{
writer.WriteStartObject();
writer.WriteNumber(nameof(ClassWithCustomConverter.MyInt), value.MyInt + 100);
writer.WriteEndObject();
}
}

[JsonConverter(typeof(CustomConverterForClass))]
public class ClassWithCustomConverter
{
public int MyInt { get; set; }
}

[JsonConverter(typeof(CustomConverterForStruct))] // Invalid
public struct ClassWithBadCustomConverter
{
public int MyInt { get; set; }
}

/// <summary>
/// Custom converter that adds\substract 100 from MyIntProperty.
/// </summary>
public class CustomConverterForStruct : JsonConverter<StructWithCustomConverter>
{
public override StructWithCustomConverter Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
if (reader.TokenType != JsonTokenType.StartObject)
{
throw new JsonException("No StartObject");
}

StructWithCustomConverter obj = new();

reader.Read();
if (reader.TokenType != JsonTokenType.PropertyName &&
reader.GetString() != "MyInt")
{
throw new JsonException("Wrong property name");
}

reader.Read();
obj.MyInt = reader.GetInt32() - 100;

reader.Read();
if (reader.TokenType != JsonTokenType.EndObject)
{
throw new JsonException("No EndObject");
}

return obj;
}

public override void Write(Utf8JsonWriter writer, StructWithCustomConverter value, JsonSerializerOptions options)
{
writer.WriteStartObject();
writer.WriteNumber(nameof(StructWithCustomConverter.MyInt), value.MyInt + 100);
writer.WriteEndObject();
}
}

[JsonConverter(typeof(CustomConverterForStruct))]
public struct StructWithCustomConverter
{
public int MyInt { get; set; }
}

[JsonConverter(typeof(CustomConverterForClass))] // Invalid
public struct StructWithBadCustomConverter
{
public int MyInt { get; set; }
}
}