-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Remove fallback to reflection converter resolution #72228
Remove fallback to reflection converter resolution #72228
Conversation
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis Issue DetailsContributes to #71714 by removing fallback to the reflection converters for any resolver other than the default reflection-based resolver. More specifically this change:
|
...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs
Show resolved
Hide resolved
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs
Show resolved
Hide resolved
...ries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs
Show resolved
Hide resolved
...t.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Converters.cs
Show resolved
Hide resolved
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs
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.
LGTM with some comments
…Info & EffectiveConverter
… caching context.
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs
Outdated
Show resolved
Hide resolved
...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs
Show resolved
Hide resolved
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs
Show resolved
Hide resolved
...libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs
Outdated
Show resolved
Hide resolved
....Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs
Outdated
Show resolved
Hide resolved
....Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs
Outdated
Show resolved
Hide resolved
…a shared caching context." This reverts commit 4f10c60.
Contributes to #71714 by removing fallback to the reflection converters for any resolver other than the default reflection-based resolver. More specifically this change:
DefaultContractResolver
(andJsonSerializerOptions.GetConverter
whenever a resolver is left unspecified for backward compatibility purposes).JsonPropertyInfo
explicitly lazy unless theJsonTypeInfo
for the declared type has already been cached.JsonSerializerOptions.GetConverter
for options instances tied toJsonSerializerContext
: the method will throwNotSupportedException
for any type not explicitly supported by the context and will not query the reflection-based converter factories. This change is inline with the proposal in Remove implicit fallback to reflection-based serialization in System.Text.Json sourcegen #71714.Nullable<T>
and F# optionals: types whose converters perform eager resolution for the converters of their element types.Fix #71714.