From 18b11ef5967198311e2674d7a8a9444d027bee03 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 20:27:53 +0000 Subject: [PATCH 1/2] fix: parse YAML numeric literals invariantly [patch] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit YamlDeserializer parsed Int32 and Double literals with the TryParse overloads that read against Thread.CurrentThread.CurrentCulture, while YamlSerializer writes them through YamlDotNet's scalar emitter, which is always dot-decimal. On a culture that groups with "." and separates decimals with "," — de-DE, fr-FR, es-ES, it-IT, ru-RU, pl-PL, nl-NL and the rest of continental Europe — double.TryParse's default NumberStyles of Float | AllowThousands read "3.14" as 314. No error was raised: the literal was silently corrupted, and round-tripping a .coder.yaml file changed the behaviour of the program it described. Pass NumberStyles and CultureInfo.InvariantCulture at both sites, matching the pattern the generators and the inspector already use (LanguageGeneratorBase, CSharpGenerator, AstFields.AssignInteger). bool.TryParse is left alone: it accepts only "true"/"false" and has no culture-sensitive overload. Fixes #73 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01B1XQbfXNGfPRfGr4bRjsT3 --- .../CultureInvariantDeserializationTests.cs | 141 ++++++++++++++++++ Coder/Serialization/YamlDeserializer.cs | 7 +- 2 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 Coder.Test/Serialization/CultureInvariantDeserializationTests.cs diff --git a/Coder.Test/Serialization/CultureInvariantDeserializationTests.cs b/Coder.Test/Serialization/CultureInvariantDeserializationTests.cs new file mode 100644 index 0000000..a432476 --- /dev/null +++ b/Coder.Test/Serialization/CultureInvariantDeserializationTests.cs @@ -0,0 +1,141 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Coder.Test.Serialization; + +using System.Globalization; +using ktsu.Coder.Ast; +using ktsu.Coder.Serialization; + +/// +/// Tests that YAML documents round-trip identically regardless of the current culture. +/// +/// The serializer emits scalars through YamlDotNet, which writes numbers invariantly +/// (dot-decimal). The deserializer must therefore read them back invariantly too; parsing +/// against the current culture turns "3.14" into 314 on a culture that groups with "." +/// (de-DE, fr-FR, es-ES, ...), silently corrupting the loaded AST. +/// +[TestClass] +[DoNotParallelize] +public class CultureInvariantDeserializationTests +{ + /// + /// A culture whose decimal separator is "," and whose group separator is ".", like de-DE, + /// fr-FR, es-ES and the rest of continental Europe. That pairing is what makes a dot-decimal + /// literal parse to a wrong value rather than fail outright: "3.14" reads as three thousand + /// one hundred and four tenths of nothing — 314. + /// + /// It is built from the invariant culture rather than named, because the test project runs in + /// globalization-invariant mode where new CultureInfo("de-DE") throws. Building it this + /// way also keeps the test hermetic: it asserts against a fixed number format rather than + /// whatever ICU data the host happens to carry. + /// + private static CultureInfo DotGroupingCulture() + { + CultureInfo culture = (CultureInfo)CultureInfo.InvariantCulture.Clone(); + culture.NumberFormat.NumberDecimalSeparator = ","; + culture.NumberFormat.NumberGroupSeparator = "."; + culture.NumberFormat.NegativeSign = "−"; + return culture; + } + + [TestMethod] + public void DeserializeFractionLiteral_UnderDotGroupingCulture_RoundTripsValue() + { + LiteralExpression literal = Literal.DecimalValue(3.14); + + LiteralExpression roundTripped = RoundTripUnder>(DotGroupingCulture(), literal); + + Assert.AreEqual(3.14, roundTripped.Value); + } + + [TestMethod] + public void DeserializeNegativeFractionLiteral_UnderDotGroupingCulture_RoundTripsValue() + { + LiteralExpression literal = Literal.DecimalValue(-0.125); + + LiteralExpression roundTripped = RoundTripUnder>(DotGroupingCulture(), literal); + + Assert.AreEqual(-0.125, roundTripped.Value); + } + + [TestMethod] + public void DeserializeIntegerLiteral_UnderDotGroupingCulture_RoundTripsValue() + { + LiteralExpression literal = Literal.Number(-42); + + LiteralExpression roundTripped = RoundTripUnder>(DotGroupingCulture(), literal); + + Assert.AreEqual(-42, roundTripped.Value); + } + + [TestMethod] + public void DeserializeBooleanLiteral_UnderDotGroupingCulture_RoundTripsValue() + { + LiteralExpression literal = Literal.Bool(true); + + LiteralExpression roundTripped = RoundTripUnder>(DotGroupingCulture(), literal); + + Assert.IsTrue(roundTripped.Value); + } + + [TestMethod] + public void SerializeFractionLiteral_UnderDotGroupingCulture_WritesDotDecimal() + { + // The deserializer reads invariantly, so the serializer must write invariantly too; + // this pins the half of the contract the fix depends on. + string yaml = UnderCulture(DotGroupingCulture(), () => new YamlSerializer().Serialize(Literal.DecimalValue(3.14))); + + StringAssert.Contains(yaml, "3.14"); + } + + [TestMethod] + public void DeserializeFunctionBody_UnderDotGroupingCulture_PreservesReturnedFraction() + { + FunctionDeclaration function = new("Rate") { ReturnType = "double" }; + function.Body.Add(new ReturnStatement(Literal.DecimalValue(0.5))); + + FunctionDeclaration roundTripped = RoundTripUnder(DotGroupingCulture(), function); + + ReturnStatement returned = (ReturnStatement)roundTripped.Body[0]; + LiteralExpression value = (LiteralExpression)returned.Expression!; + Assert.AreEqual(0.5, value.Value); + } + + /// + /// Serializes and deserializes with installed + /// as the current culture for both halves of the trip. + /// + private static T RoundTripUnder(CultureInfo culture, AstNode node) where T : AstNode + { + AstNode? deserialized = UnderCulture(culture, () => + { + string yaml = new YamlSerializer().Serialize(node); + return new YamlDeserializer().Deserialize(yaml); + }); + + Assert.IsNotNull(deserialized); + Assert.IsInstanceOfType(deserialized); + return (T)deserialized; + } + + /// + /// Runs on a dedicated thread with as the + /// current culture, so the ambient culture of the test host is never mutated. + /// + private static TResult UnderCulture(CultureInfo culture, Func action) + { + // LongRunning gets a dedicated thread rather than a pool one, so the culture is discarded + // with the thread instead of outliving the test on a thread someone else will be handed. + // The task carries any failure back to this thread, so the round trip needs no catch of its own. + return Task.Factory.StartNew( + () => + { + CultureInfo.CurrentCulture = culture; + CultureInfo.CurrentUICulture = culture; + return action(); + }, + CancellationToken.None, + TaskCreationOptions.LongRunning, + TaskScheduler.Default).GetAwaiter().GetResult(); + } +} diff --git a/Coder/Serialization/YamlDeserializer.cs b/Coder/Serialization/YamlDeserializer.cs index 883abb0..bf22676 100644 --- a/Coder/Serialization/YamlDeserializer.cs +++ b/Coder/Serialization/YamlDeserializer.cs @@ -5,6 +5,7 @@ namespace ktsu.Coder.Serialization; using System; using System.Collections.Generic; using System.Collections.ObjectModel; +using System.Globalization; using System.Linq; using System.Text.RegularExpressions; using ktsu.Coder.Ast; @@ -118,7 +119,7 @@ _ when nodeType.StartsWith("leaf<", StringComparison.OrdinalIgnoreCase) || nodeT return valueType switch { "String" => new AstLeafNode(nodeData?.ToString() ?? string.Empty), - "Int32" when int.TryParse(nodeData?.ToString(), out int intValue) => new AstLeafNode(intValue), + "Int32" when int.TryParse(nodeData?.ToString(), NumberStyles.Integer, CultureInfo.InvariantCulture, out int intValue) => new AstLeafNode(intValue), "Boolean" when bool.TryParse(nodeData?.ToString(), out bool boolValue) => new AstLeafNode(boolValue), _ => null, }; @@ -1258,9 +1259,9 @@ private UnaryExpression DeserializeUnaryExpression(object? nodeData) Expression? result = valueType switch { "String" => new LiteralExpression(value?.ToString() ?? string.Empty), - "Int32" when int.TryParse(value?.ToString(), out int intValue) => new LiteralExpression(intValue), + "Int32" when int.TryParse(value?.ToString(), NumberStyles.Integer, CultureInfo.InvariantCulture, out int intValue) => new LiteralExpression(intValue), "Boolean" when bool.TryParse(value?.ToString(), out bool boolValue) => new LiteralExpression(boolValue), - "Double" when double.TryParse(value?.ToString(), out double doubleValue) => new LiteralExpression(doubleValue), + "Double" when double.TryParse(value?.ToString(), NumberStyles.Float, CultureInfo.InvariantCulture, out double doubleValue) => new LiteralExpression(doubleValue), _ => null, }; From 3b3ab1197aa5dc9e6c0603957de906f3eb1fed38 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 20:44:20 +0000 Subject: [PATCH 2/2] style: use Assert.Contains over StringAssert.Contains [patch] MSTEST0046, flagged by SonarCloud on the new test file. StringAssert is superseded in MSTest 4; Assert.Contains takes the substring first. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01B1XQbfXNGfPRfGr4bRjsT3 --- .../Serialization/CultureInvariantDeserializationTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Coder.Test/Serialization/CultureInvariantDeserializationTests.cs b/Coder.Test/Serialization/CultureInvariantDeserializationTests.cs index a432476..2d1f8f2 100644 --- a/Coder.Test/Serialization/CultureInvariantDeserializationTests.cs +++ b/Coder.Test/Serialization/CultureInvariantDeserializationTests.cs @@ -85,7 +85,7 @@ public void SerializeFractionLiteral_UnderDotGroupingCulture_WritesDotDecimal() // this pins the half of the contract the fix depends on. string yaml = UnderCulture(DotGroupingCulture(), () => new YamlSerializer().Serialize(Literal.DecimalValue(3.14))); - StringAssert.Contains(yaml, "3.14"); + Assert.Contains("3.14", yaml); } [TestMethod]