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

STJ source generator does not correctly wrap custom converters on nullable properties #81833

Closed
eiriktsarpalis opened this issue Feb 8, 2023 · 3 comments
Labels
area-System.Text.Json bug source-generator Indicates an issue with a source generator feature
Milestone

Comments

@eiriktsarpalis
Copy link
Member

Consider the following code:

var value = new MyPoco { Value = MyEnum.A };

Console.WriteLine(JsonSerializer.Serialize(value)); // works as expected
Console.WriteLine(JsonSerializer.Serialize(value, MyContext.Default.MyPoco));
// System.ArgumentException: GenericArguments[0], 'System.Nullable`1[MyEnum]', on 'System.Text.Json.Serialization.Converters.EnumConverter`1[T]' violates the constraint of type 'T'.
//   at System.RuntimeTypeHandle.Instantiate(RuntimeType inst)
//   at System.RuntimeType.MakeGenericType(Type[] instantiation)
//   -- - End of inner exception stack trace ---
//   at System.RuntimeType.ValidateGenericArguments(MemberInfo definition, RuntimeType[] genericArguments, Exception e)
//   at System.RuntimeType.MakeGenericType(Type[] instantiation)
//   at System.Text.Json.Serialization.Converters.EnumConverterFactory.Create(Type enumType, EnumConverterOptions converterOptions, JsonNamingPolicy namingPolicy, JsonSerializerOptions options)
//   at MyContext.GetConverterFromFactory(JsonSerializerOptions options, Type type, JsonConverterFactory factory) in C: \Users\eitsarpa\source\repos\ConsoleApp1\ConsoleApp1\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyContext.g.cs:line 77
//   at MyContext.GetConverterFromFactory[T] (JsonSerializerOptions options, JsonConverterFactory factory) in C: \Users\eitsarpa\source\repos\ConsoleApp1\ConsoleApp1\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyContext.g.cs:line 72

public class MyPoco
{
    [JsonConverter(typeof(JsonStringEnumConverter))]
    public MyEnum? Value { get; set; }
}

public enum MyEnum { A, B, C };

[JsonSerializable(typeof(MyPoco))]
public partial class MyContext : JsonSerializerContext
{ }

By convention, the reflection serializer automatically wraps custom property converters on nullable types. We should update the source generator to do the same.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 8, 2023
@ghost
Copy link

ghost commented Feb 8, 2023

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Consider the following code:

var value = new MyPoco { Value = MyEnum.A };

Console.WriteLine(JsonSerializer.Serialize(value)); // works as expected
Console.WriteLine(JsonSerializer.Serialize(value, MyContext.Default.MyPoco));
// System.ArgumentException: GenericArguments[0], 'System.Nullable`1[MyEnum]', on 'System.Text.Json.Serialization.Converters.EnumConverter`1[T]' violates the constraint of type 'T'.
//   at System.RuntimeTypeHandle.Instantiate(RuntimeType inst)
//   at System.RuntimeType.MakeGenericType(Type[] instantiation)
//   -- - End of inner exception stack trace ---
//   at System.RuntimeType.ValidateGenericArguments(MemberInfo definition, RuntimeType[] genericArguments, Exception e)
//   at System.RuntimeType.MakeGenericType(Type[] instantiation)
//   at System.Text.Json.Serialization.Converters.EnumConverterFactory.Create(Type enumType, EnumConverterOptions converterOptions, JsonNamingPolicy namingPolicy, JsonSerializerOptions options)
//   at MyContext.GetConverterFromFactory(JsonSerializerOptions options, Type type, JsonConverterFactory factory) in C: \Users\eitsarpa\source\repos\ConsoleApp1\ConsoleApp1\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyContext.g.cs:line 77
//   at MyContext.GetConverterFromFactory[T] (JsonSerializerOptions options, JsonConverterFactory factory) in C: \Users\eitsarpa\source\repos\ConsoleApp1\ConsoleApp1\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyContext.g.cs:line 72

public class MyPoco
{
    [JsonConverter(typeof(JsonStringEnumConverter))]
    public MyEnum? Value { get; set; }
}

public enum MyEnum { A, B, C };

[JsonSerializable(typeof(MyPoco))]
public partial class MyContext : JsonSerializerContext
{ }

By convention, the reflection serializer automatically wraps custom property converters on nullable types. We should update the source generator to do the same.

Author: eiriktsarpalis
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@eiriktsarpalis eiriktsarpalis added bug source-generator Indicates an issue with a source generator feature and removed untriaged New issue has not been triaged by the area owner labels Feb 8, 2023
@eiriktsarpalis eiriktsarpalis added this to the Future milestone Feb 8, 2023
@eiriktsarpalis
Copy link
Member Author

Should be addressed in conjunction with #79311

@eiriktsarpalis
Copy link
Member Author

Fixed in #84208

@ghost ghost locked as resolved and limited conversation to collaborators Jul 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json bug source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

No branches or pull requests

1 participant