Skip to content

Commit a1bc3b1

Browse files
Copilotstephentoub
Copilot
and
stephentoub
authored
Fix STJ source generator emitting culture-dependent minus signs for negative JsonPropertyOrder (#121278)
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 `JsonPropertyOrder` with negative values on certain locales (e.g., fi_FI.UTF-8). 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.CurrentCulture` to `InvariantCulture` and restores it in a finally block. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: stephentoub <[email protected]> Co-authored-by: Stephen Toub <[email protected]>
1 parent 704d429 commit a1bc3b1

File tree

3 files changed

+128
-35
lines changed

3 files changed

+128
-35
lines changed

‎src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn3.11.cs‎

Lines changed: 43 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.Collections.Generic;
55
using System.Collections.Immutable;
6+
using System.Globalization;
67
using System.Threading;
78
using Microsoft.CodeAnalysis;
89
using Microsoft.CodeAnalysis.CSharp;
@@ -43,45 +44,58 @@ public void Initialize(GeneratorInitializationContext context)
4344
/// <param name="executionContext"></param>
4445
public void Execute(GeneratorExecutionContext executionContext)
4546
{
46-
if (executionContext.SyntaxContextReceiver is not SyntaxContextReceiver receiver || receiver.ContextClassDeclarations == null)
47+
// Ensure the source generator parses and emits using invariant culture.
48+
// This prevents issues such as locale-specific negative signs (e.g., U+2212 in fi-FI)
49+
// from being written to generated source files.
50+
// Note: RS1035 is already disabled at the file level for this Roslyn version.
51+
CultureInfo originalCulture = CultureInfo.CurrentCulture;
52+
CultureInfo.CurrentCulture = CultureInfo.InvariantCulture;
53+
try
4754
{
48-
// nothing to do yet
49-
return;
50-
}
55+
if (executionContext.SyntaxContextReceiver is not SyntaxContextReceiver receiver || receiver.ContextClassDeclarations == null)
56+
{
57+
// nothing to do yet
58+
return;
59+
}
5160

52-
// Stage 1. Parse the identified JsonSerializerContext classes and store the model types.
53-
KnownTypeSymbols knownSymbols = new(executionContext.Compilation);
54-
Parser parser = new(knownSymbols);
61+
// Stage 1. Parse the identified JsonSerializerContext classes and store the model types.
62+
KnownTypeSymbols knownSymbols = new(executionContext.Compilation);
63+
Parser parser = new(knownSymbols);
5564

56-
List<ContextGenerationSpec>? contextGenerationSpecs = null;
57-
foreach ((ClassDeclarationSyntax? contextClassDeclaration, SemanticModel semanticModel) in receiver.ContextClassDeclarations)
58-
{
59-
ContextGenerationSpec? contextGenerationSpec = parser.ParseContextGenerationSpec(contextClassDeclaration, semanticModel, executionContext.CancellationToken);
60-
if (contextGenerationSpec is null)
65+
List<ContextGenerationSpec>? contextGenerationSpecs = null;
66+
foreach ((ClassDeclarationSyntax? contextClassDeclaration, SemanticModel semanticModel) in receiver.ContextClassDeclarations)
6167
{
62-
continue;
68+
ContextGenerationSpec? contextGenerationSpec = parser.ParseContextGenerationSpec(contextClassDeclaration, semanticModel, executionContext.CancellationToken);
69+
if (contextGenerationSpec is null)
70+
{
71+
continue;
72+
}
73+
74+
(contextGenerationSpecs ??= new()).Add(contextGenerationSpec);
6375
}
6476

65-
(contextGenerationSpecs ??= new()).Add(contextGenerationSpec);
66-
}
77+
// Stage 2. Report any diagnostics gathered by the parser.
78+
foreach (DiagnosticInfo diagnosticInfo in parser.Diagnostics)
79+
{
80+
executionContext.ReportDiagnostic(diagnosticInfo.CreateDiagnostic());
81+
}
6782

68-
// Stage 2. Report any diagnostics gathered by the parser.
69-
foreach (DiagnosticInfo diagnosticInfo in parser.Diagnostics)
70-
{
71-
executionContext.ReportDiagnostic(diagnosticInfo.CreateDiagnostic());
72-
}
83+
if (contextGenerationSpecs is null)
84+
{
85+
return;
86+
}
7387

74-
if (contextGenerationSpecs is null)
75-
{
76-
return;
88+
// Stage 3. Emit source code from the spec models.
89+
OnSourceEmitting?.Invoke(contextGenerationSpecs.ToImmutableArray());
90+
Emitter emitter = new(executionContext);
91+
foreach (ContextGenerationSpec contextGenerationSpec in contextGenerationSpecs)
92+
{
93+
emitter.Emit(contextGenerationSpec);
94+
}
7795
}
78-
79-
// Stage 3. Emit source code from the spec models.
80-
OnSourceEmitting?.Invoke(contextGenerationSpecs.ToImmutableArray());
81-
Emitter emitter = new(executionContext);
82-
foreach (ContextGenerationSpec contextGenerationSpec in contextGenerationSpecs)
96+
finally
8397
{
84-
emitter.Emit(contextGenerationSpec);
98+
CultureInfo.CurrentCulture = originalCulture;
8599
}
86100
}
87101

‎src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn4.0.cs‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Collections.Immutable;
5+
using System.Globalization;
56
using System.Linq;
67
using Microsoft.CodeAnalysis;
78
using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -42,10 +43,26 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
4243
.Combine(knownTypeSymbols)
4344
.Select(static (tuple, cancellationToken) =>
4445
{
45-
Parser parser = new(tuple.Right);
46-
ContextGenerationSpec? contextGenerationSpec = parser.ParseContextGenerationSpec(tuple.Left.ContextClass, tuple.Left.SemanticModel, cancellationToken);
47-
ImmutableEquatableArray<DiagnosticInfo> diagnostics = parser.Diagnostics.ToImmutableEquatableArray();
48-
return (contextGenerationSpec, diagnostics);
46+
// Ensure the source generator parses using invariant culture.
47+
// This prevents issues such as locale-specific negative signs (e.g., U+2212 in fi-FI)
48+
// from being written to generated source files.
49+
#pragma warning disable RS1035 // CultureInfo.CurrentCulture is banned in analyzers
50+
CultureInfo originalCulture = CultureInfo.CurrentCulture;
51+
CultureInfo.CurrentCulture = CultureInfo.InvariantCulture;
52+
try
53+
{
54+
#pragma warning restore RS1035
55+
Parser parser = new(tuple.Right);
56+
ContextGenerationSpec? contextGenerationSpec = parser.ParseContextGenerationSpec(tuple.Left.ContextClass, tuple.Left.SemanticModel, cancellationToken);
57+
ImmutableEquatableArray<DiagnosticInfo> diagnostics = parser.Diagnostics.ToImmutableEquatableArray();
58+
return (contextGenerationSpec, diagnostics);
59+
#pragma warning disable RS1035
60+
}
61+
finally
62+
{
63+
CultureInfo.CurrentCulture = originalCulture;
64+
}
65+
#pragma warning restore RS1035
4966
})
5067
#if ROSLYN4_4_OR_GREATER
5168
.WithTrackingName(SourceGenerationSpecTrackingName)
@@ -69,8 +86,23 @@ private void ReportDiagnosticsAndEmitSource(SourceProductionContext sourceProduc
6986
}
7087

7188
OnSourceEmitting?.Invoke(ImmutableArray.Create(input.ContextGenerationSpec));
72-
Emitter emitter = new(sourceProductionContext);
73-
emitter.Emit(input.ContextGenerationSpec);
89+
90+
// Ensure the source generator emits number literals using invariant culture.
91+
// This prevents issues such as locale-specific negative signs (e.g., U+2212 in fi-FI)
92+
// from being written to generated source files.
93+
#pragma warning disable RS1035 // CultureInfo.CurrentCulture is banned in analyzers
94+
CultureInfo originalCulture = CultureInfo.CurrentCulture;
95+
CultureInfo.CurrentCulture = CultureInfo.InvariantCulture;
96+
try
97+
{
98+
Emitter emitter = new(sourceProductionContext);
99+
emitter.Emit(input.ContextGenerationSpec);
100+
}
101+
finally
102+
{
103+
CultureInfo.CurrentCulture = originalCulture;
104+
}
105+
#pragma warning restore RS1035
74106
}
75107

76108
/// <summary>

‎src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Linq;
5+
using System.Tests;
56
using Microsoft.CodeAnalysis;
67
using Microsoft.CodeAnalysis.CSharp;
78
using Xunit;
@@ -965,5 +966,51 @@ public partial class MyContext : JsonSerializerContext
965966
Assert.NotEmpty(result.NewCompilation.GetDiagnostics().Where(d => d.Id == "EXP001"));
966967
}
967968
#endif
969+
970+
[Fact]
971+
public void NegativeJsonPropertyOrderGeneratesValidCode()
972+
{
973+
// Test for https://github.com/dotnet/runtime/issues/121277
974+
// Verify that negative JsonPropertyOrder values generate compilable code
975+
// even on locales that use non-ASCII minus signs (e.g., fi_FI uses U+2212)
976+
string source = """
977+
using System.Text.Json.Serialization;
978+
979+
namespace Test
980+
{
981+
public class MyClass
982+
{
983+
[JsonPropertyOrder(-1)]
984+
public int FirstProperty { get; set; }
985+
986+
[JsonPropertyOrder(0)]
987+
public int SecondProperty { get; set; }
988+
989+
[JsonPropertyOrder(-100)]
990+
public int ThirdProperty { get; set; }
991+
}
992+
993+
[JsonSerializable(typeof(MyClass))]
994+
public partial class MyContext : JsonSerializerContext
995+
{
996+
}
997+
}
998+
""";
999+
1000+
// Test with fi_FI culture which uses U+2212 minus sign for negative numbers
1001+
using (new ThreadCultureChange("fi-FI"))
1002+
{
1003+
Compilation compilation = CompilationHelper.CreateCompilation(source);
1004+
JsonSourceGeneratorResult result = CompilationHelper.RunJsonSourceGenerator(compilation, logger: logger);
1005+
1006+
// The generated code should compile without errors
1007+
// If the bug exists, we'd see CS1525, CS1002, CS1056, or CS0201 errors
1008+
var errors = result.NewCompilation.GetDiagnostics()
1009+
.Where(d => d.Severity == DiagnosticSeverity.Error)
1010+
.ToList();
1011+
1012+
Assert.Empty(errors);
1013+
}
1014+
}
9681015
}
9691016
}

0 commit comments

Comments
 (0)