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

Extend JsonIncludeAttribute and JsonConstructorAttribute support to internal and private members #88452

Merged
merged 2 commits into from
Jul 7, 2023

Conversation

eiriktsarpalis
Copy link
Member

The JsonInclude and JsonConstructor attributes are annotations that can be applied directly to individual members to include them in the JSON contract for a specific type. Historically however these attributes haved either:

  1. only worked on public members (JsonConstructor),
  2. applied arbitrary restrictions for the attribute to have effect (e.g. JsonInclude on properties requires that either the getter or setter should be public for the non-public counterpart to be accessed) or,
  3. applied inconsistently depending on whether the member is a property or a field (JsonInclude).

This PR changes the semantics of both attributes so that they have uniform behavior regardless of accessibility. It assumes that users know what they're doing when annotating a given member and tries to accommodate their request. A notable exception to this rule is the source generator, which will warn/throw a runtime exception if the user tries to include members that are not accessible to the source generated context.

Fix #87431

@ghost
Copy link

ghost commented Jul 5, 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

The JsonInclude and JsonConstructor attributes are annotations that can be applied directly to individual members to include them in the JSON contract for a specific type. Historically however these attributes haved either:

  1. only worked on public members (JsonConstructor),
  2. applied arbitrary restrictions for the attribute to have effect (e.g. JsonInclude on properties requires that either the getter or setter should be public for the non-public counterpart to be accessed) or,
  3. applied inconsistently depending on whether the member is a property or a field (JsonInclude).

This PR changes the semantics of both attributes so that they have uniform behavior regardless of accessibility. It assumes that users know what they're doing when annotating a given member and tries to accommodate their request. A notable exception to this rule is the source generator, which will warn/throw a runtime exception if the user tries to include members that are not accessible to the source generated context.

Fix #87431

Author: eiriktsarpalis
Assignees: eiriktsarpalis
Labels:

area-System.Text.Json

Milestone: -

@jeffhandley
Copy link
Member

A notable exception to this rule is the source generator, which will warn/throw a runtime exception if the user tries to include members that are not accessible to the source generated context.

Is this an application-runtime exception, or is this a source-generator-runtime exception / application-design-time build error?

@eiriktsarpalis
Copy link
Member Author

A notable exception to this rule is the source generator, which will warn/throw a runtime exception if the user tries to include members that are not accessible to the source generated context.

Is this an application-runtime exception, or is this a source-generator-runtime exception / application-design-time build error?

It's both -- a diagnostic warning will be emitted at compile time and an exception will be thrown at run time.

@eiriktsarpalis
Copy link
Member Author

Test failures are unrelated and tracked by #88453

@eiriktsarpalis eiriktsarpalis merged commit 24d7f5a into dotnet:main Jul 7, 2023
97 of 104 checks passed
@eiriktsarpalis eiriktsarpalis deleted the jsoninclude-relax branch July 7, 2023 11:40
@ghost ghost locked as resolved and limited conversation to collaborators Aug 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider extending JsonIncludeAttribute and JsonConstructorAttribute support to internal and private members.
3 participants