Newtonsoft.json: Add support for generic JsonConverter instantiation

[TylerBrinkley]

Consider the following value wrapper struct and its associated json converter.

[JsonConverter(typeof(ValueConverter<>))]
struct Value<T>
{
    public static implicit operator T(Value<T> value) => value._value;

    public static implicit operator Value<T>(T value) => new Value<T>(value);

    private readonly T _value;

    public Value(T value)
    {
        _value = value;
    }
}

class ValueConverter<T> : JsonConverter
{
    public override bool CanConvert(Type objectType) => objectType == typeof(Value<T>);

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer) => new Value<T>(serializer.Deserialize<T>(reader));

    public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) => serializer.Serialize(writer, (T)(Value<T>)value, typeof(T));
}

Running the following code I currently get an ArgumentException of "Invalid type owner for DynamicMethod."

JsonConvert.SerializeObject(new Value<bool>(true));

Obviously the problem is that the converter type provided with the JsonConverterAttribute is an open generic type and Json.NET doesn't know how to instantiate it. The provided converter type is open because currently C# doesn't allow use of a types' type parameters in its attributes.

When an open generic converter type is provided I would like Json.NET to implicitly use the generic type arguments of the type the attribute is applied to using the MakeGenericType method on Type in order to instantiate the json converter.

[TylerBrinkley]

With the current version of Json.NET I was able to implement this same thing using the custom contract resolver below. But I do think this should be implemented directly within Json.NET since it isn't a breaking change as the existing behavior throws an exception and the feature can be very useful.

c# public sealed class CustomContractResolver : DefaultContractResolver { protected override JsonConverter ResolveContractConverter(Type objectType) { var typeInfo = objectType.GetTypeInfo(); if (typeInfo.IsGenericType && !typeInfo.IsGenericTypeDefinition) { var jsonConverterAttribute = typeInfo.GetCustomAttribute<JsonConverterAttribute>(); if (jsonConverterAttribute != null && jsonConverterAttribute.ConverterType.GetTypeInfo().IsGenericTypeDefinition) { return (JsonConverter)Activator.CreateInstance(jsonConverterAttribute.ConverterType.MakeGenericType(typeInfo.GenericTypeArguments), jsonConverterAttribute.ConverterParameters); } } return base.ResolveContractConverter(objectType); } }

[JamesNK]

There are a lot of problems and questions I can see.

1. JsonConverterAttribute can also be placed on fields and attributes. What happens there? Always use the type's generic arguments? Or would it use the property/field's type?
2. There are also other attributes where a converter can be created from. I believe there is ItemsConverterType on JsonProperty, and maybe some other locations. If it works for some ways of creating converters then it should work for all.
3. What about if the converter has multiple generic arguments? Error thrown?
4. What about if the type has multiple generic arguments? Which one to use? Error thrown?

[TylerBrinkley]

Thanks for thinking through this. Those are great points.

1. I think it should use the generic arguments of the type it's going to convert so when applied to a type it will use the type's generic arguments and when applied to a property or field it will use the property or field type's generic arguments. I've updated my PR to support properties and fields.
2. I'm unsure of how to handle this as I don't know how to retrieve the items type. Should I retrieve the property or field's type and then see if it implements IEnumerable<T>. Then retrieve the T and get it's generic arguments?
3. & 4. Multiple generic arguments would be supported so long as the number of generic arguments for the converter and type are the same. It will use the MakeGenericType method which will throw the exceptions if the provided generic arguments don't work with the converter type.

[TylerBrinkley]

I just pushed another commit to the PR to support ItemsConverterType by the method I proposed above.

[JamesNK]

I spent a ton of time looking at this today and I'm still not sure about it. Is this something people would actually find useful? What problems is it solving? No one has ever mentioned it before and I'm really really not a fan of adding features just for the hell of it.

Also why would it be that a types generic type arguments would be used for the converter's generic type arguments? What about if a dev has a converter and they what the type they are converting as the type argument?

This feels like a solution that is in search of a problem.

[TylerBrinkley]

Thank you for your time considering this feature request. This feature is definitely more of a power user feature and I can understand your hesitation adding it.

I'm currently using this feature with a custom contract resolver in an OSS project for a Value wrapper struct which also contains a dirty checking property. Before I realized this could be accomplished with a custom contract resolver, binding the converter to the type was a real pain as the converter had to be specified as non-generic with a type dictionary mapping to a generic converter through an interface as specified here.

I think the type's generic type arguments flowing over to the converter's generic type arguments makes sense when applied to types but I can see how others might expect something different when used on fields and properties. I originally only envisioned this to be applied to types.

As this can be handled with a custom contract resolver the lack of this feature is not a big problem to me. Again thank you for your consideration.

[JamesNK]

This is where I got up to when I stopped - https://github.com/JamesNK/Newtonsoft.Json/commit/7665113d955aee99f7a506103b142650d6c833d5

Hmmm, I'm torn on this feature. I'll give it more thought.

[TylerBrinkley]

That looks really good.

I was thinking about your suggestion of using the actual type as the generic type argument for the converter. It seems to me this would only make sense if the type itself is a generic type parameter of the enclosing type such as in the following cases. I've updated my branch to support these cases at https://github.com/TylerBrinkley/Newtonsoft.Json/commit/b747a5c7a0dffcbab591996497816f3f410aa5ac

```c#
public sealed class ClassWithGenericEnumProperty
{
[JsonConverter(typeof(GenericEnumConverter<>))]
public TEnum EnumValue { get; set; }
}

public sealed class ClassWithGenericEnumCollectionProperty
{
[JsonProperty(ItemConverterType = typeof(GenericEnumConverter<>))]
public TEnum[] EnumValues { get; set; }
}

[JsonArray(ItemConverterType = typeof(GenericEnumConverter<>))]
public sealed class ListWithEnumItemGenericConverter : List
{
}

[JsonDictionary(ItemConverterType = typeof(GenericEnumConverter<>))]
public sealed class DictionaryWithEnumItemGenericConverter : Dictionary
{
}

public sealed class GenericEnumConverter : JsonConverter
{
public override bool CanConvert(Type objectType) => objectType == typeof(TEnum);

public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
{
    if (reader.TokenType == JsonToken.Integer)
    {
        return Enum.ToObject(typeof(TEnum), reader.Value);
    }
    return Enum.Parse(typeof(TEnum), reader.Value.ToString().Substring(typeof(TEnum).Name.Length + 1));
}

public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
{
    string str = value.ToString();
    if (char.IsDigit(str[0]) || str[0] == '-' || str[0] == '+')
    {
        writer.WriteRawValue(str);
    }
    else
    {
        writer.WriteValue($"{typeof(TEnum).Name}.{str}");
    }
}

}
```

[flq]

Morning all,
indeed it seems that not many people find it useful. I would have found it useful today. my use case: the serializer is not within my control, hence I use the JsonConvert attribute. I am writing a graph query where I get some kind of tree returned. The tree structure itself is somewhat peculiar but always has the same structure - meanwhile the payload can change. So ideally I would want to serialize to a

[JsonConverter(typeof(GremlinTreeConverter<T>)]
class GremlinTree<T>

However, what I do now is to have a non-generic converter that build the generic one with reflection based on the objectType that enters the converter. If it would be used a lot I would have a cache of pre-built converters that dispatches based on the objectType

[TylerBrinkley]

@flq that was my initial way of handling the lack of support for this but then switched to the custom contract resolver route as I had direct control over the serialization of the type.

It'd be great if the PR #1359 for this would be accepted.

[mchandschuh]

I would also find use in this feature. Thanks for providing the custom contract resolver work around.

[thomaslevesque]

I just found a case where I needed this too. Basically, this can be useful any time you want to customize the serialization of a generic type.

[PGsHub]

I came upon this same issue and @TylerBrinkley 's CustomContractResolver saved the day. Thanks a lot! I don't think custom serialization of a generic type is that exotic, so would be great to have native support in the future.

[ArtemBakhmat-Sparkybit]

Hello, JamesNK
Can you merge this pull-request into master? This problem is very urgent for me
Thank you very much
Artem

[ArtemBakhmat-Sparkybit]

With the current version of Json.NET I was able to implement this same thing using the custom contract resolver below. But I do think this should be implemented directly within Json.NET since it isn't a breaking change as the existing behavior throws an exception and the feature can be very useful.

public sealed class CustomContractResolver : DefaultContractResolver
{
    protected override JsonConverter ResolveContractConverter(Type objectType)
    {
        var typeInfo = objectType.GetTypeInfo();
        if (typeInfo.IsGenericType && !typeInfo.IsGenericTypeDefinition)
        {
            var jsonConverterAttribute = typeInfo.GetCustomAttribute<JsonConverterAttribute>();
            if (jsonConverterAttribute != null && jsonConverterAttribute.ConverterType.GetTypeInfo().IsGenericTypeDefinition)
            {
                return (JsonConverter)Activator.CreateInstance(jsonConverterAttribute.ConverterType.MakeGenericType(typeInfo.GenericTypeArguments), jsonConverterAttribute.ConverterParameters);
            }
        }
        return base.ResolveContractConverter(objectType);
    }
}

Hello, Tyler. Can you describe how can we use CustomContractResolver in order to pass generic parameters to custom attribute?

Here is how I'm using it. I created CustomContractResolver (yours) and I'm intending using it like this:
var response = await ExecuteAsync(resource, method, data);
var settings = new JsonSerializerSettings {ContractResolver = new CustomContractResolver()};
var result = JsonConvert.DeserializeObject(response.Content, settings);

But I'm getting error while passing generic type as parameter to json-converter: [JsonConverter(typeof(SingleValueCollectionConverter))]
public List Data { get; set; }
Error: Attribute argument cannot use type parameters.

If I use generic attribute without passing parameter like so
[JsonConverter(typeof(SingleValueCollectionConverter<>))]
I currently get an ArgumentException of "Invalid type owner for DynamicMethod."

Where am I wrong?
Thanks

[TylerBrinkley]

If I use generic attribute without passing parameter like so
[JsonConverter(typeof(SingleValueCollectionConverter<>))]
I currently get an ArgumentException of "Invalid type owner for DynamicMethod."

The above should work just fine so long as you're using the CustomContractResolver. I'm guessing you're serializing or deserializing the type somewhere else in your code which isn't using the CustomContractResolver.