Add JSON snake_case naming policy #54128
Conversation
FiniteReality
requested review from
eiriktsarpalis,
jozkee,
layomia and
steveharter
as code owners
dotnet-issue-labeler
bot
added
area-System.Text.Json
new-api-needs-documentation
labels
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to 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. |
|
Tagging subscribers to this area: @eiriktsarpalis, @layomia Issue DetailsNot much attention has been paid to how JSON.Net implements its snake_case policy, but this should be mostly compatible. Some rudimentary tests have been added, copied from CamelCaseUnitTests and tweaked to remove some unreasonable test cases. Fixes #782
|
| return name.ToLowerInvariant(); | ||
| } | ||
|
|
||
| var result = new ValueStringBuilder(2 * name.Length); |
There was a problem hiding this comment.
I'm not sure 2 * length is a reasonable heuristic here, but I wasn't sure how much bigger the "average" property name would be, so I felt just doubling was safer.
There was a problem hiding this comment.
ABC would turn into a_b_c so the absolute maximum would be (2 * name.Length) - 1
There was a problem hiding this comment.
ABC should turn into abc since the entire string is all the same case; it should be almost the case as ID
There was a problem hiding this comment.
so AbCdEf would be ab_cd_ef which gives us name.Length + (int)Math.Ceiling(name.Length/(double)2) - 1
Edit: Actually it is more complicated than that. There are also single char numbers. I would still say that (2 * name.Length) - 1 is the absolute maximum in all cases.
|
|
||
| result.Append(current); | ||
|
|
||
| if (x < name.Length - 1) |
There was a problem hiding this comment.
Thinking about it more, this condition will likely accidentally trigger if two or more numbers are hit in sequence. In this scenario, what would a user expect the name to be converted to? I know abc_1__2_def is most likely incorrect, but should it be abc_12_def, or abc_1_2_def?
There was a problem hiding this comment.
For now I've just left this as abc_1_2_def, and have added a relevant test case.
src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/SnakeCaseUnitTests.cs
Outdated
Show resolved
Hide resolved
|
Gah, rebasing is a pain sometimes. Excuse the force push; I'm resolving conflicts 😅 |
Not much attention has been paid to how JSON.Net implements its snake_case policy, but this should be mostly compatible. Some rudimentary tests have been added, copied from CamelCaseUnitTests. Fixes dotnet#782
terrajobst
added
the
community-contribution
|
Thanks for the PR. Per #782 (comment) and other design-related comments on that thread, this feature needs a bit more work. To do it properly, we should try again in .NET 7. We'll ping this PR at that time. |
ghost
locked as resolved and limited conversation to collaborators
Not much attention has been paid to how JSON.Net implements its snake_case policy, but this should be mostly compatible. Some rudimentary tests have been added, copied from CamelCaseUnitTests and tweaked to remove some unreasonable test cases.
Fixes #782