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

System.Text.Json should support deserializing more collection types #107787

Open
4 tasks
eiriktsarpalis opened this issue Sep 13, 2024 · 5 comments
Open
4 tasks
Assignees
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@eiriktsarpalis
Copy link
Member

STJ will today happily serialize any collection type that implements IEnumerable<T>, however things are less than ideal when it comes to deserialization support. While the serializer does include baked-in support for a number of common collection types, we still have a number of unsupported types such as frozen collections, ArraySegment<T> or IReadOnlySet<T>. Things become even more difficult when working with user-defined types: unless the type implements the conventions used by collection initializers users need to write their own custom converters.

This issue proposes that we extend generic support for collection type deserialization by detecting the following patterns (in order of preference):

  1. Collection is mutable adhering to the conventions required by collection initializers or,
  2. Collection declares a span factory via CollectionBuilderAttribute or,
  3. Collection exposes a public constructor accepting a single IEnumerable<T> parameter (where the T must match the T of the implemented IEnumerable<T>).

I have prototyped an implementation of this approach both for reflection and source gen in the typeshape-csharp project. This should add OOTB support for multiple built-in types that aren't supported today, or should make it relatively straightforward to make changes to collections such that they are supported (e.g. by adding CollectinBuilderAttribute annotations). For popular types not conforming to any of these patterns, we should consider adding baked-in converters to support them.

Related Issues

cc @stephentoub @CyrusNajmabadi

@eiriktsarpalis eiriktsarpalis self-assigned this Sep 13, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Sep 13, 2024
@eiriktsarpalis eiriktsarpalis added this to the 10.0.0 milestone Sep 13, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Sep 13, 2024
Copy link
Contributor

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

@eiriktsarpalis eiriktsarpalis added the enhancement Product code improvement that does NOT require public API changes/additions label Sep 13, 2024
@CyrusNajmabadi
Copy link
Member

Hi hi. I see the mention. How can I help?

@eiriktsarpalis
Copy link
Member Author

Was curious to get your thoughts on the proposed use of patterns (and their ordering) when determining a collection construction strategy.

@elgonzo
Copy link

elgonzo commented Sep 13, 2024

How about inline arrays?

I also wonder what would happen if STJ requires a CollectionBuilderAttribute (CBA) for deserialization of inline arrays when (if?) the C# compiler eventually gets built-in support for collection expressions for inline arrays, and i am a bit concerned about that by virtue of STJ requiring CBA for deserializing inline arrays, a very convenient compile-time check of making sure that the collection expression is correctly sized will be again turned into a user-implemented runtime-check (as it is unfortunately currently the case with inline arrays) because the compiler will have to respect the CBA that is only there to satisfy STJ.

@silkfire
Copy link

Does this include the deserialization of ReadOnlyCollection<T> too?

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

4 participants