[STJ] Add support for nullable reference annotations on properties by eiriktsarpalis · Pull Request #102499 · dotnet/runtime

eiriktsarpalis · 2024-05-21T14:45:40Z

Adds support for nullable reference type annotations on JSON property metadata.
Refactors the implementation of JsonParameterInfo metadata such that properties and constructor parameters are associated early in the contract resolution process and annotations of both feed into the JsonPropertyInfo.DisallowNullReads property.

ghost · 2024-05-21T14:45:46Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

dotnet-policy-service · 2024-05-21T14:46:28Z

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

src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs

src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfo.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs

...libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs

src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfoContext.cs

src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs

src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs

src/libraries/System.Text.Json/gen/Model/PropertyGenerationSpec.cs

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.ExceptionMessages.cs

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs

...Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs

...ests/System.Text.Json.SourceGeneration.Tests/System.Text.Json.SourceGeneration.Tests.targets

…ion/JsonSerializerOptions.cs Co-authored-by: David Cantú <dacantu@microsoft.com>

…neration.Tests/System.Text.Json.SourceGeneration.Tests.targets Co-authored-by: David Cantú <dacantu@microsoft.com>

src/libraries/System.Text.Json/System.Text.Json.sln

...Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs

jozkee

LGTM, thanks a lot for taking over.

…otnet#102499) * Progress so far on nullability annotations * Complete implementation and make all NullableAnnotations tests pass. * Update annotations for all failing unit tests. * Update src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs * Update src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs * Address feedback * Update to latest approved API and semantics. * Update src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfoContext.cs Co-authored-by: David Cantú <dacantu@microsoft.com> * Update src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfoContext.cs Co-authored-by: David Cantú <dacantu@microsoft.com> * Update src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs Co-authored-by: David Cantú <dacantu@microsoft.com> * Rename more ignoreNullableAnnotations stragglers. * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs Co-authored-by: David Cantú <dacantu@microsoft.com> * Remove commented out code and address feedback. * Update src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/System.Text.Json.SourceGeneration.Tests.targets Co-authored-by: David Cantú <dacantu@microsoft.com> * Ensure the original parameter name flows exception messages. * Extract exceptions to a throw helper in the new properties. * Extend test coverage to Nullable<T> properties. * Revert sln changes * Add second-pass review improvements. --------- Co-authored-by: David Cantú <dacantu@microsoft.com>

sebastienros · 2024-05-29T20:53:23Z

@eiriktsarpalis this is breaking the ASP.NET E2E tests as we ingested it for preview5. Here are the logs

https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_apis/build/builds/690914/logs/40

…otnet#102499) * Progress so far on nullability annotations * Complete implementation and make all NullableAnnotations tests pass. * Update annotations for all failing unit tests. * Update src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs * Update src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs * Address feedback * Update to latest approved API and semantics. * Update src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfoContext.cs Co-authored-by: David Cantú <dacantu@microsoft.com> * Update src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfoContext.cs Co-authored-by: David Cantú <dacantu@microsoft.com> * Update src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs Co-authored-by: David Cantú <dacantu@microsoft.com> * Rename more ignoreNullableAnnotations stragglers. * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs Co-authored-by: David Cantú <dacantu@microsoft.com> * Remove commented out code and address feedback. * Update src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/System.Text.Json.SourceGeneration.Tests.targets Co-authored-by: David Cantú <dacantu@microsoft.com> * Ensure the original parameter name flows exception messages. * Extract exceptions to a throw helper in the new properties. * Extend test coverage to Nullable<T> properties. * Revert sln changes * Add second-pass review improvements. --------- Co-authored-by: David Cantú <dacantu@microsoft.com>

ghost added area-System.Text.Json new-api-needs-documentation labels May 21, 2024

eiriktsarpalis requested review from jozkee, krwq and stephentoub May 21, 2024 14:45

eiriktsarpalis self-assigned this May 21, 2024