-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Avoid referencing types of JsonIgnore'd properties. #120543
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
Avoid referencing types of JsonIgnore'd properties. #120543
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the System.Text.Json source generator to avoid referencing types of JsonIgnore'd properties unless the type is already used elsewhere in the type graph. This prevents inadvertent referencing of types marked as experimental in the source generator.
- Updates source generator logic to use stub
JsonPropertyInfo<object>for ignored properties of unused types - Adds test infrastructure improvements with logging support
- Includes new test cases for experimental types with JsonIgnore
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| System.Text.Json.SourceGeneration.Roslyn4.4.Unit.Tests.csproj | Updates Roslyn API version from 4.4 to 4.8 |
| JsonSourceGeneratorTests.cs | Adds logging support and new test methods for experimental type scenarios |
| CompilationHelper.cs | Adds logging infrastructure and diagnostic output capabilities |
| JsonSourceGenerator.Emitter.cs | Core logic changes to avoid referencing types of ignored properties |
...ies/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/CompilationHelper.cs
Show resolved
Hide resolved
|
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis |
...tem.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs
Outdated
Show resolved
Hide resolved
...tem.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests I suggest considering adding:
- A mixture of experimental and obsolete properties and fields on a type
- A type hierarchy that hits the case of naming collisions
- Scenarios that do not compile successfully, asserting where diagnostics are expected
...tem.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs
Outdated
Show resolved
Hide resolved
357b2fc to
3ff1732
Compare
Co-authored-by: Copilot <[email protected]>
This reverts commit ed3bcc3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the extra tests. Looks great.
|
/backport to release/10.0 |
|
Started backporting to release/10.0: https://github.com/dotnet/runtime/actions/runs/18384128909 |
Updates the source generator so that generated code does not reference types of JsonIgnore'd properties unless the type is already present elsewhere in the type graph. Instead, it replaces the ignored property with a stub
JsonPropertyInfo<object>instance which is necessary for certain runtime validations performed by the serializer. This prevents inadvertent referencing of types marked as experimental in the source generator.