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

fix(76268): Fix invalid behavior for ignored properties with "new" mo… #84756

Conversation

Maximys
Copy link
Contributor

@Maximys Maximys commented Apr 13, 2023

Simple change Dictionary<TKey,TValue>.Add(TKey, TValue) for adding new ignored property to Dictionary<TKey,TValue>.Item[TKey]

Fixes #76268

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Apr 13, 2023
@ghost
Copy link

ghost commented Apr 13, 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

Simple change Dictionary<TKey,TValue>.Add(TKey, TValue) for adding new ignored property to Dictionary<TKey,TValue>.Item[TKey]

Fixes #76268

Author: Maximys
Assignees: -
Labels:

area-System.Text.Json

Milestone: -

@Maximys
Copy link
Contributor Author

Maximys commented Apr 24, 2023

@jeffhandley , can you review current PR quickly? I think, #66900 linked with the same lines and I want to fix it separately.

@layomia
Copy link
Contributor

layomia commented May 10, 2023

@Maximys could you take a look at the test failures? My guess is that the change makes source generation fail in some way e.g. not producing source good to implement the abstract members.

@Maximys
Copy link
Contributor Author

Maximys commented May 10, 2023

@layomia , ok, I'll try to look at the test failures a bit late.

@Maximys Maximys force-pushed the bugfix/76268-fix-invalid-processing-of-jsonignoreattribute branch 4 times, most recently from 98033bb to 42387a7 Compare May 11, 2023 10:56
@Maximys
Copy link
Contributor Author

Maximys commented May 11, 2023

@layomia , I had just fixed failed tests by amend. Could you review again?

Copy link
Member

@eiriktsarpalis eiriktsarpalis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@eiriktsarpalis eiriktsarpalis force-pushed the bugfix/76268-fix-invalid-processing-of-jsonignoreattribute branch from 42387a7 to 5f6670a Compare June 5, 2023 09:10
@eiriktsarpalis eiriktsarpalis merged commit b27814c into dotnet:main Jun 5, 2023
@Maximys Maximys deleted the bugfix/76268-fix-invalid-processing-of-jsonignoreattribute branch June 6, 2023 04:32
@ghost ghost locked as resolved and limited conversation to collaborators Jul 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding JsonIgnoreAttribute to new property fails with ArgumentException
3 participants