Add support for types derived from supported BCL collections

[layomia]

Fixes https://github.com/dotnet/corefx/issues/38767.
Fixes https://github.com/dotnet/corefx/issues/38521.

Supported in this PR: user-defined types that implement natively supported collections that have "add" methods e.g. IList<T>, Queue<T>, IDictionary, IList e tc.

Not supported in this PR: user-defined types that implement natively supported collections that don't have "add" methods e.g. IEnumerable<T>, ICollection, IReadOnyList<T> and immutable collections.

[danmoseley]

At this point, is this required for 3.0? Is risk acceptable?

[layomia]

At this point, is this required for 3.0? Is risk acceptable?

Yes, it fixes some important issues raised by partners and community members, and it's pretty well tested.

https://github.com/dotnet/corefx/issues/38767.
https://github.com/dotnet/corefx/issues/38521.

[danmoseley]

Ok.

[layomia]

cc @pranavkm

[phizch]

This change makes it impossible to have a JsonConverterFactory distinguish between a BCL base class and a derived type.

Specialized collections can be necessary if the json has multiple ways of declaring a list, e.g.

{
  "List1" : 
  {
    "Count": 50,
    "Values" : [ "test", ... ]
  },
  "List2" : [ "test", ... ] 
}

public class A
{
    public MyCollection List1 { get; set; }
    public List<string> List2 { get; set }
}
public class MyCollection : List<string> { }
public class MyConverterFactory : JsonConverterFactory
{
    public override bool CanConvert( Type typeToConvert )
    {
        return ( typeToConvert == typeof( MyCollection ) );
    }
    public override JsonConverter CreateConverter( Type typeToConvert, JsonSerializerOptions options )
    {
        // return the converter
        throw new NotImplementedException();
    }
}

From JsonClassInfo.AddProperty:

    // Get implemented type, if applicable.
    // Will return the propertyType itself if it's a non-enumerable, string, or natively supported collection.
    Type implementedType = GetImplementedCollectionType(propertyType);
    if (implementedType != propertyType)
    {
        jsonInfo = CreateProperty(implementedType, implementedType, implementedType, null, typeof(object), options);
    }
    else
    {
        jsonInfo = CreateProperty(propertyType, propertyType, propertyType, propertyInfo, classType, options);
    }

GetImplementedCollectionType(typeof(MyCollection)) will return List<string>, so CreateProperty will ask any factories to create converters for List.

A solution could be to provide CreateProperty and on to GetClassType with both implementedType and propertyType so that it can send propertyType to any factories, while using the BCL type if no factory can produce a converter for the derived type.

I don't know if I should have opened a new issue for this, but now at least you know..
If you'd like I can open a new issue and implement the changes.

[scalablecory]

@phizch this PR is closed and won't be tracked; please open a new issue for this. It's okay to reference this PR in that, though.

CC @ahsonkhan

[ahsonkhan]

If you'd like I can open a new issue

Yes, please.