Add JsonNamingPolicyAttribute for member-level naming policies in System.Text.Json#124645
Add JsonNamingPolicyAttribute for member-level naming policies in System.Text.Json#124645eiriktsarpalis merged 14 commits intomainfrom
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-text-json |
…tor support Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
…ttribute Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
...Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs
Outdated
Show resolved
Hide resolved
…e unresolvable derived attributes in source gen Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
|
Addressed both review comments in 95b0107:
|
…tion and source gen paths Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
src/libraries/System.Text.Json/tests/Common/PropertyNameTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
eiriktsarpalis
left a comment
There was a problem hiding this comment.
@copilot Add a test case where the protected attribute constructor is being used to specify a user-defined naming policy. This should be tested both in reflection and the source generator.
...Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs
Show resolved
Hide resolved
tarekgh
left a comment
There was a problem hiding this comment.
Left small suggestions, LGTM otherwise.
|
@copilot please address the new merge conflicts |
|
@copilot please address the merge conflicts. |
Resolved in 8c5da66. The conflicts were in |
|
/ba-g stalled CI legs |
Description
System.Text.Json only supports naming policies globally via
JsonSerializerOptions.PropertyNamingPolicy. This addsJsonNamingPolicyAttributeto allow granular naming policy control at the type, property, and field level.API
Usage
Priority order
JsonPropertyName> member-levelJsonNamingPolicyAttribute> type-levelJsonNamingPolicyAttribute>JsonSerializerOptions.PropertyNamingPolicy> original name.Changes
JsonNamingPolicyAttribute.cs: New attribute with public ctor forJsonKnownNamingPolicyand protected ctor for customJsonNamingPolicysubclassesDefaultJsonTypeInfoResolver.Helpers.cs: Type-levelJsonNamingPolicyAttributeis resolved once per type inPopulatePropertiesand threaded throughAddMembersDeclaredBySuperType→AddMember→CreatePropertyInfo.DeterminePropertyNamechecks member-level then type-level attribute before falling back to global options. Integrated alongside the type-levelJsonIgnoreAttributeresolution from Support generic converters on generic types with JsonConverterAttribute #123209.Parser.cs,KnownTypeSymbols.cs): Parses the attribute at type and member level usingIsAssignableFrom(handles subclasses); derived attributes with custom policies that can't be resolved at compile time fall back toJsonKnownNamingPolicy.Unspecified— this flows throughDetermineEffectiveJsonPropertyNamewhereUnspecifiedmaps to null (returning the original CLR name), and the non-nullmetadataJsonPropertyNameis then emitted to prevent the runtime globalPropertyNamingPolicyfrom incorrectly overriding. BothtypeNamingPolicyandtypeIgnoreConditionparameters are threaded throughProcessTypeCustomAttributes→ParsePropertyGenerationSpecs→ParsePropertyGenerationSpec.JsonNamingPolicyAttributeto public API surfaceAssert.Equalwith triple-quote strings on full JSON output. Includes tests for custom derivedJsonNamingPolicyAttributesubclasses using the protected ctor — reflection tests verify the custom policy is applied correctly, and source gen tests verify both graceful fallback to CLR property names and that the globalPropertyNamingPolicyis prevented from applying when a custom derived naming-policy attribute is present.Original prompt
This section details on the original issue you should resolve
<issue_title>STJ should allow setting naming policies on the member level.</issue_title>
<issue_description>## Motivation
System.Text.Json supports user-defined naming policies for properties/fields which can be specified via the
JsonSerializerOptions.PropertyNamingPolicy. Today such policies can only be specified globally which removes the ability to apply granular policies on the individual type/property/field level.API Proposal
API Usage
cc @stephentoub</issue_description>
Comments on the Issue (you are @copilot in this section)
@eiriktsarpalis FYI @jeffhandley @PranavSenthilnathan something to consider for .NET 10. @eiriktsarpalis I personally prefer the explicit approach, ultimately it's a trade-off between writing less boilerplate and having an explicit contract. @stephentoub If I have a class with 20 members, I'd rather put one attribute on the class then one on each of the 20 properties.As Eric says, it's a tradeoff between verbosity and explicitness.</comment_new>
<comment_new>@stephentoub
In #108232 (comment), you asked about the advantage of:
vs
That's not the comparison that's interesting. The comparison that's interesting is:
vs
And there there's an obvious advantage of the former, that of brevity, maintenance, etc.
That advantage is then weighed against the slightly more explicit nature of JsonPropertyName. I say slightly because both are explicit; it's just a question of whether a constant is provided or whether a constant is provided with a formula over it, e.g. "6" vs "x == 3 and the formula is 2 * x". JsonPropertyName is arguably a bit more explicit, which you see as an advantage.
Hence, "it's a tradeoff between verbosity and explicitness".
I don't believe I'm missing the point.</comment_new>
<comment_new>@eiriktsarpalis
Feel free to create an implementation PR following the approved API shape.</comment_new>
<comment_new>@eiriktsarpalis
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.