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

JSON needs object initializer equivalent APIs #73219

Closed
Tracked by #71967
krwq opened this issue Aug 2, 2022 · 5 comments
Closed
Tracked by #71967

JSON needs object initializer equivalent APIs #73219

krwq opened this issue Aug 2, 2022 · 5 comments
Labels
area-System.Text.Json Cost:XL Work that requires one engineer more than 4 weeks source-generator Indicates an issue with a source generator feature User Story A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@krwq
Copy link
Member

krwq commented Aug 2, 2022

Recently C# has added two features which rely on the object initializer syntax:

  • init-only properties
  • required keyword

In the current state JSON reflection APIs can ignore the fact that property is init-only and still set it and required constraint is not enforced. For source-gen on the other hand regular setter code simply won't compile and similarly for required properties constructor will cause compile errors.

We should:

  • update the contract model to allow for such construction - most likely this will be the same work as Add support for parameterized constructors in System.Text.Json contract customization (converters) #71944
  • update source-gen code to use those APIs for init-only/required properties - this will likely need rather major refactoring of source-gen code which currently uses reflection APIs (reflection implementation over Roslyn APIs) which is missing a way to check if property is required - we will likely need to change it to use Roslyn APIs directly
  • possibly, adjust reflection code to behave consistently with source-gen

One way to solve this problem is to treat init-only and required properties the same way as arguments to parametrized constructor, then passing them to parametrized constructor assigns them in the object initializer.

Relevant issues:

@krwq krwq added area-System.Text.Json User Story A single user-facing feature. Can be grouped under an epic. Cost:XL Work that requires one engineer more than 4 weeks labels Aug 2, 2022
@krwq krwq added this to the 8.0.0 milestone Aug 2, 2022
@ghost
Copy link

ghost commented Aug 2, 2022

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

Recently C# has added two features which rely on the object initializer syntax:

  • init-only properties
  • required keyword

In the current state JSON reflection APIs can ignore the fact that property is init-only and still set it and required constraint is not enforced. For source-gen on the other hand regular setter code simply won't compile and similarly for required properties constructor will cause compile errors.

We should:

  • update the contract model to allow for such construction - most likely this will be the same work as Add support for parameterized constructors in System.Text.Json contract customization (converters) #71944
  • update source-gen code to use those APIs for init-only/required properties - this will likely need rather major refactoring of source-gen code which currently uses reflection APIs (reflection implementation over Roslyn APIs) which is missing a way to check if property is required - we will likely need to change it to use Roslyn APIs directly
  • possibly, adjust reflection code to behave consistently with source-gen

One way to solve this problem is to treat init-only and required properties the same way as arguments to parametrized constructor, then passing them to parametrized constructor assigns them in the object initializer.

Relevant issues:

Author: krwq
Assignees: -
Labels:

area-System.Text.Json, User Story, Cost:XL

Milestone: 8.0.0

@jkotas
Copy link
Member

jkotas commented Aug 2, 2022

(reflection implementation over Roslyn APIs) which is missing a way to check if property is required

The reflection object does allow you to check whether the property is required by looking for System.Runtime.CompilerServices.RequiredMemberAttribute.

@eiriktsarpalis
Copy link
Member

We might want to address this in combination with #71944.

@eiriktsarpalis eiriktsarpalis added the source-generator Indicates an issue with a source generator feature label Sep 29, 2022
@eiriktsarpalis eiriktsarpalis modified the milestones: 8.0.0, Future Jan 23, 2023
@eiriktsarpalis
Copy link
Member

Fixed in #83147. Any outstanding work is tracked by the issues linked in the description.

@amis92
Copy link

amis92 commented Jun 12, 2023

So it's completed, but still in Future milestone - that's intentional?

@eiriktsarpalis eiriktsarpalis modified the milestones: Future, 8.0.0 Jun 13, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json Cost:XL Work that requires one engineer more than 4 weeks source-generator Indicates an issue with a source generator feature User Story A single user-facing feature. Can be grouped under an epic.
Projects
None yet
Development

No branches or pull requests

4 participants