Fix STJ source generator emitting culture-dependent minus signs for negative JsonPropertyOrder#121278
Conversation
…alues Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
|
@copilot, are there any other cases of this in the STJ emitter? Places where something like a number or DateTime or something else that's culture-sensitive may end up rendering with the current culture inappropriately? I'm wondering whether a more encompassing fix might be to instead surround the emitter with setting CultureInfo.CurrentCulture to InvariantCulture (and then setting it back in a finally). |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a culture-specific bug in the JSON source generator where negative JsonPropertyOrder values could generate invalid C# code in locales that use non-ASCII minus signs (e.g., Finnish locale uses U+2212 instead of U+002D).
Key changes:
- Modified the source generator to use culture-invariant formatting when emitting property order values
- Added a comprehensive unit test that validates the fix by testing code generation under the Finnish locale
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs | Fixed numeric formatting to use InvariantCulture when generating code for property.Order values |
| src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs | Added test to verify negative JsonPropertyOrder values generate valid code across different locales |
...tem.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs
Outdated
Show resolved
Hide resolved
I searched through the emitter and found:
However, wrapping the entire
I'll implement that approach - setting |
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
@copilot, you didn't make the changes. Please wrap the invocation of the emitter with such a try/finally. |
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
I've implemented the try/finally wrapper approach. The Emit invocation is now wrapped with code that sets |
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn3.11.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn4.0.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn4.0.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn4.0.cs
Outdated
Show resolved
Hide resolved
...tem.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs
Outdated
Show resolved
Hide resolved
…atting, use ThreadCultureChange helper Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn3.11.cs
Outdated
Show resolved
Hide resolved
…erator Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn3.11.cs
Outdated
Show resolved
Hide resolved
|
/ba-g all failures known / unrelated |
Fix STJ source generator producing non-compilable code on some locales
This PR addresses an issue where the System.Text.Json source generator produces non-compilable code when using
JsonPropertyOrderwith negative values on certain locales (e.g., fi_FI.UTF-8).Changes Made
Technical Details
The issue was in the source generator where numeric values were being directly interpolated into generated C# code without specifying culture. On locales like fi_FI, negative numbers use the Unicode minus sign (U+2212) instead of the ASCII hyphen-minus, making the generated code invalid.
Fix:
Wrapped the entire source generator execution (both parsing and emitting) with code that sets
CultureInfo.CurrentCulturetoInvariantCultureand restores it in a finally block. This comprehensive approach:Files Modified:
using System.Globalization;since culture is now managed in calling codeTests:
NegativeJsonPropertyOrderGeneratesValidCodethat tests negative JsonPropertyOrder with fi_FI culture using ThreadCultureChange helperFixes #121277
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.