From 7e587a5273ee5885a90602378ec305ef07775a77 Mon Sep 17 00:00:00 2001 From: matt-edmondson Date: Sun, 1 Nov 2026 04:29:59 +0000 Subject: [PATCH 1/2] Honour AllowParentheses, AllowTrailingSign and AllowCurrencySymbol in Parse [patch] Parse took these styles, and Any and Currency include them, but never read them, so "(1.5)", "1.5-" and "$1,234.50" threw. Strip the parentheses, a trailing sign and a leading or trailing currency symbol before the digits, and read the currency separators once a symbol is present. Fixes #152 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW4sAxuwubJA58cavro --- .../PreciseNumberParseAffixTests.cs | 96 +++++++++++++ PreciseNumber/PreciseNumber.cs | 126 +++++++++++++++++- 2 files changed, 217 insertions(+), 5 deletions(-) create mode 100644 PreciseNumber.Test/PreciseNumberParseAffixTests.cs diff --git a/PreciseNumber.Test/PreciseNumberParseAffixTests.cs b/PreciseNumber.Test/PreciseNumberParseAffixTests.cs new file mode 100644 index 0000000..168dcc9 --- /dev/null +++ b/PreciseNumber.Test/PreciseNumberParseAffixTests.cs @@ -0,0 +1,96 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.PreciseNumber.Test; + +using System.Globalization; + +/// +/// Covers the parts of a number allows around its digits: parentheses, a +/// trailing sign, and a currency symbol. Each case is checked against parsing +/// the same text with the same style and format, so the expectation is the BCL's rather than ours. +/// +[TestClass] +public class PreciseNumberParseAffixTests +{ + // Hand-built formats keep these tests independent of the ICU data installed on the machine. + // Dollars is shaped like en-US, Euros like de-DE, which writes the symbol after a no-break space. + private static readonly NumberFormatInfo Dollars = new() + { + CurrencySymbol = "$", + CurrencyDecimalSeparator = ".", + CurrencyGroupSeparator = ",", + NumberDecimalSeparator = ".", + NumberGroupSeparator = ",", + }; + + private static readonly NumberFormatInfo Euros = new() + { + CurrencySymbol = "€", + CurrencyDecimalSeparator = ",", + CurrencyGroupSeparator = ".", + NumberDecimalSeparator = ",", + NumberGroupSeparator = ".", + }; + + private static NumberFormatInfo Format(string name) => name switch + { + "Dollars" => Dollars, + "Euros" => Euros, + _ => NumberFormatInfo.InvariantInfo, + }; + + [TestMethod] + [DataRow("(1.5)", NumberStyles.Any, "Invariant")] + [DataRow("(1.5)", NumberStyles.AllowParentheses | NumberStyles.AllowDecimalPoint, "Invariant")] + [DataRow("1.5-", NumberStyles.Any, "Invariant")] + [DataRow("1.5-", NumberStyles.AllowTrailingSign | NumberStyles.AllowDecimalPoint, "Invariant")] + [DataRow("1.5+", NumberStyles.Any, "Invariant")] + [DataRow("1.5E2-", NumberStyles.Any, "Invariant")] + [DataRow("¤1.5", NumberStyles.Any, "Invariant")] + [DataRow("$1,234.50", NumberStyles.Currency, "Dollars")] + [DataRow("-$1,234.50", NumberStyles.Any, "Dollars")] + [DataRow("$-1,234.50", NumberStyles.Any, "Dollars")] + [DataRow("($1,234.50)", NumberStyles.Any, "Dollars")] + [DataRow("1.234,50 €", NumberStyles.Currency, "Euros")] + [DataRow("-1.234,50 €", NumberStyles.Currency, "Euros")] + [DataRow("1.234,50 €-", NumberStyles.Currency, "Euros")] + public void TestParseAcceptsWhatDecimalAccepts(string text, NumberStyles style, string format) + { + NumberFormatInfo numberFormat = Format(format); + PreciseNumber expected = decimal.Parse(text, style, numberFormat).ToPreciseNumber(); + + Assert.AreEqual(expected, PreciseNumber.Parse(text, style, numberFormat), $"Parse(\"{text}\", {style}) is wrong"); + Assert.IsTrue(PreciseNumber.TryParse(text, style, numberFormat, out PreciseNumber parsed), $"TryParse(\"{text}\", {style}) failed"); + Assert.AreEqual(expected, parsed, $"TryParse(\"{text}\", {style}) is wrong"); + } + + [TestMethod] + public void TestParseAcceptsANoBreakSpaceBeforeACurrencySymbol() + { + // Windows' de-DE and fr-FR write "1.234,50 €" with U+00A0 before the symbol. decimal only takes + // that from a culture whose own separators use U+00A0, but a copied amount should parse the same + // whether its space is typed or rendered, as group separators already do here. + Assert.AreEqual(PreciseNumber.Parse("-1234.5", CultureInfo.InvariantCulture), PreciseNumber.Parse("-1.234,50\u00A0€", NumberStyles.Currency, Euros)); + Assert.AreEqual(PreciseNumber.Parse("1.5", CultureInfo.InvariantCulture), PreciseNumber.Parse("€\u00A01,5", NumberStyles.Currency, Euros)); + } + + [TestMethod] + [DataRow("(1.5)", NumberStyles.Float, "Invariant")] + [DataRow("1.5-", NumberStyles.Float, "Invariant")] + [DataRow("$1.5", NumberStyles.Number, "Dollars")] + [DataRow("1,5 €", NumberStyles.Number, "Euros")] + [DataRow("(-1.5)", NumberStyles.Any, "Invariant")] + [DataRow("-1.5-", NumberStyles.Any, "Invariant")] + [DataRow("$1.5$", NumberStyles.Any, "Dollars")] + [DataRow("()", NumberStyles.Any, "Invariant")] + [DataRow("$", NumberStyles.Any, "Dollars")] + [DataRow("-", NumberStyles.Any, "Invariant")] + public void TestParseRejectsWhatDecimalRejects(string text, NumberStyles style, string format) + { + NumberFormatInfo numberFormat = Format(format); + Assert.IsFalse(decimal.TryParse(text, style, numberFormat, out _), $"decimal accepts \"{text}\" under {style}, so the case is wrong"); + + Assert.IsFalse(PreciseNumber.TryParse(text, style, numberFormat, out _), $"TryParse(\"{text}\", {style}) accepted it"); + Assert.ThrowsExactly(() => PreciseNumber.Parse(text, style, numberFormat), $"Parse(\"{text}\", {style}) accepted it"); + } +} diff --git a/PreciseNumber/PreciseNumber.cs b/PreciseNumber/PreciseNumber.cs index 060127d..28fe460 100644 --- a/PreciseNumber/PreciseNumber.cs +++ b/PreciseNumber/PreciseNumber.cs @@ -48,6 +48,10 @@ public readonly partial record struct PreciseNumber /// The message carried by the that parsing throws. /// private const string InvalidFormatMessage = "Input string was not in a correct format."; + + /// The spaces a culture may put between a currency symbol and the number, as de-DE does in 1.234,50 €. + private static readonly char[] CurrencySymbolSpaces = [' ', '\u00A0', '\u202F']; + private const string UnsupportedParseStylesMessage = "The number styles AllowHexSpecifier and AllowBinarySpecifier are not supported by PreciseNumber."; #if NET8_0_OR_GREATER @@ -1225,19 +1229,20 @@ public static PreciseNumber Parse(ReadOnlySpan s, NumberStyles style, IFor throw new FormatException(InvalidFormatMessage); } + s = StripAffixes(s, style, numberFormat, out bool isNegative, out bool hasCurrencySymbol); + if (s.Length == 1 && s[0] == '0') { return Zero; } - ReadOnlySpan decimalSeparator = numberFormat.NumberDecimalSeparator; - ReadOnlySpan groupSeparator = numberFormat.NumberGroupSeparator; + // Once a currency symbol has been read, the number is written the way the culture writes money. + ReadOnlySpan decimalSeparator = hasCurrencySymbol ? numberFormat.CurrencyDecimalSeparator : numberFormat.NumberDecimalSeparator; + ReadOnlySpan groupSeparator = hasCurrencySymbol ? numberFormat.CurrencyGroupSeparator : numberFormat.NumberGroupSeparator; bool allowThousands = style.HasFlag(NumberStyles.AllowThousands) && !groupSeparator.IsEmpty; bool allowDecimalPoint = style.HasFlag(NumberStyles.AllowDecimalPoint); bool allowExponent = style.HasFlag(NumberStyles.AllowExponent); - int startIndex = ReadLeadingSign(s, style, numberFormat, out bool isNegative); - // Collect the digits first and hand them to BigInteger in one go. Accumulating with // significand = significand * 10 + digit costs a full BigInteger multiply per character. char[]? rentedDigits = s.Length > MaxStackAllocChars ? ArrayPool.Shared.Rent(s.Length) : null; @@ -1252,7 +1257,7 @@ public static PreciseNumber Parse(ReadOnlySpan s, NumberStyles style, IFor ReadOnlySpan exponentText = []; bool hasExponent = false; - for (int i = startIndex; i < s.Length; i++) + for (int i = 0; i < s.Length; i++) { char c = s[i]; if (allowDecimalPoint && StartsWithSeparator(s[i..], decimalSeparator)) @@ -1407,6 +1412,117 @@ private static ReadOnlySpan TrimAllowedWhite(ReadOnlySpan s, NumberS return s; } + /// + /// Removes what allows around the digits: enclosing parentheses, a leading + /// or trailing sign, and a leading or trailing currency symbol, returning the digits that remain. + /// + /// + /// Mirrors the BCL parsers. Parentheses mean negative and may not be combined with a sign. The sign + /// and the currency symbol may come in either order on either side, as -$1.5, $-1.5 + /// and 1,5 €- do, and a space or no-break space may separate the symbol from the number, + /// since cultures such as de-DE write 1.234,50 €. At most one sign and one symbol are read. + /// + /// Thrown when the parentheses enclose a sign. + private static ReadOnlySpan StripAffixes(ReadOnlySpan s, NumberStyles style, NumberFormatInfo numberFormat, out bool isNegative, out bool hasCurrencySymbol) + { + bool hasParentheses = false; + if (style.HasFlag(NumberStyles.AllowParentheses) && s.Length >= 2 && s[0] == '(' && s[^1] == ')') + { + hasParentheses = true; + s = s[1..^1]; + } + + ReadOnlySpan currencySymbol = style.HasFlag(NumberStyles.AllowCurrencySymbol) ? numberFormat.CurrencySymbol : []; + bool hasSign = false; + isNegative = false; + hasCurrencySymbol = false; + + // Leading: a sign and a currency symbol, in either order. + for (int pass = 0; pass < 2 && !s.IsEmpty; pass++) + { + if (!hasSign) + { + int signLength = ReadLeadingSign(s, style, numberFormat, out bool leadingNegative); + if (signLength > 0) + { + hasSign = true; + isNegative = leadingNegative; + s = s[signLength..]; + continue; + } + } + + if (!hasCurrencySymbol && !currencySymbol.IsEmpty && s.StartsWith(currencySymbol, StringComparison.Ordinal)) + { + hasCurrencySymbol = true; + s = s[currencySymbol.Length..].TrimStart(CurrencySymbolSpaces); + continue; + } + + break; + } + + // Trailing: the same two, read from the end. + for (int pass = 0; pass < 2 && !s.IsEmpty; pass++) + { + if (!hasCurrencySymbol && !currencySymbol.IsEmpty && s.EndsWith(currencySymbol, StringComparison.Ordinal)) + { + hasCurrencySymbol = true; + s = s[..^currencySymbol.Length].TrimEnd(CurrencySymbolSpaces); + continue; + } + + if (!hasSign && style.HasFlag(NumberStyles.AllowTrailingSign)) + { + int signLength = ReadTrailingSign(s, numberFormat, out bool trailingNegative); + if (signLength > 0) + { + hasSign = true; + isNegative = trailingNegative; + s = s[..^signLength]; + continue; + } + } + + break; + } + + if (hasParentheses) + { + if (hasSign) + { + throw new FormatException(InvalidFormatMessage); + } + + isNegative = true; + } + + return s; + } + + /// + /// Reads a trailing sign spelled as spells it, or the ASCII hyphen, + /// returning its length, or zero when does not end with one. + /// + private static int ReadTrailingSign(ReadOnlySpan s, NumberFormatInfo numberFormat, out bool isNegative) + { + isNegative = true; + if (!string.IsNullOrEmpty(numberFormat.NegativeSign) && s.EndsWith(numberFormat.NegativeSign, StringComparison.Ordinal)) + { + return numberFormat.NegativeSign.Length; + } + + if (s[^1] == '-') + { + return 1; + } + + isNegative = false; + return !string.IsNullOrEmpty(numberFormat.PositiveSign) && s.EndsWith(numberFormat.PositiveSign, StringComparison.Ordinal) + ? numberFormat.PositiveSign.Length + : 0; + } + /// /// Reads a leading sign spelled as spells it, returning the index /// of the first character after it. The ASCII hyphen stays accepted whatever the culture spells From f5e49e83db613c71cb34304b497bc2f88a161f48 Mon Sep 17 00:00:00 2001 From: matt-edmondson Date: Sun, 1 Nov 2026 04:41:26 +0000 Subject: [PATCH 2/2] Split StripAffixes into leading and trailing helpers SonarCloud S3776 counted its cognitive complexity at 27 against a limit of 15. Behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW4sAxuwubJA58cavro --- PreciseNumber/PreciseNumber.cs | 89 ++++++++++++++++++---------------- 1 file changed, 48 insertions(+), 41 deletions(-) diff --git a/PreciseNumber/PreciseNumber.cs b/PreciseNumber/PreciseNumber.cs index 28fe460..81564fe 100644 --- a/PreciseNumber/PreciseNumber.cs +++ b/PreciseNumber/PreciseNumber.cs @@ -1425,10 +1425,9 @@ private static ReadOnlySpan TrimAllowedWhite(ReadOnlySpan s, NumberS /// Thrown when the parentheses enclose a sign. private static ReadOnlySpan StripAffixes(ReadOnlySpan s, NumberStyles style, NumberFormatInfo numberFormat, out bool isNegative, out bool hasCurrencySymbol) { - bool hasParentheses = false; - if (style.HasFlag(NumberStyles.AllowParentheses) && s.Length >= 2 && s[0] == '(' && s[^1] == ')') + bool hasParentheses = style.HasFlag(NumberStyles.AllowParentheses) && s.Length >= 2 && s[0] == '(' && s[^1] == ')'; + if (hasParentheses) { - hasParentheses = true; s = s[1..^1]; } @@ -1437,32 +1436,55 @@ private static ReadOnlySpan StripAffixes(ReadOnlySpan s, NumberStyle isNegative = false; hasCurrencySymbol = false; - // Leading: a sign and a currency symbol, in either order. - for (int pass = 0; pass < 2 && !s.IsEmpty; pass++) + StripLeadingAffixes(ref s, style, numberFormat, currencySymbol, ref hasSign, ref isNegative, ref hasCurrencySymbol); + StripTrailingAffixes(ref s, style, numberFormat, currencySymbol, ref hasSign, ref isNegative, ref hasCurrencySymbol); + + if (hasParentheses) { - if (!hasSign) + if (hasSign) { - int signLength = ReadLeadingSign(s, style, numberFormat, out bool leadingNegative); - if (signLength > 0) - { - hasSign = true; - isNegative = leadingNegative; - s = s[signLength..]; - continue; - } + throw new FormatException(InvalidFormatMessage); } - if (!hasCurrencySymbol && !currencySymbol.IsEmpty && s.StartsWith(currencySymbol, StringComparison.Ordinal)) + isNegative = true; + } + + return s; + } + + /// + /// Removes a leading sign and a leading currency symbol, in either order, for . + /// + private static void StripLeadingAffixes(ref ReadOnlySpan s, NumberStyles style, NumberFormatInfo numberFormat, ReadOnlySpan currencySymbol, ref bool hasSign, ref bool isNegative, ref bool hasCurrencySymbol) + { + for (int pass = 0; pass < 2 && !s.IsEmpty; pass++) + { + bool leadingNegative = false; + int signLength = hasSign ? 0 : ReadLeadingSign(s, style, numberFormat, out leadingNegative); + if (signLength > 0) + { + hasSign = true; + isNegative = leadingNegative; + s = s[signLength..]; + } + else if (!hasCurrencySymbol && !currencySymbol.IsEmpty && s.StartsWith(currencySymbol, StringComparison.Ordinal)) { hasCurrencySymbol = true; s = s[currencySymbol.Length..].TrimStart(CurrencySymbolSpaces); - continue; } - - break; + else + { + break; + } } + } - // Trailing: the same two, read from the end. + /// + /// Removes a trailing currency symbol and a trailing sign, in either order, for . + /// + private static void StripTrailingAffixes(ref ReadOnlySpan s, NumberStyles style, NumberFormatInfo numberFormat, ReadOnlySpan currencySymbol, ref bool hasSign, ref bool isNegative, ref bool hasCurrencySymbol) + { + bool allowTrailingSign = style.HasFlag(NumberStyles.AllowTrailingSign); for (int pass = 0; pass < 2 && !s.IsEmpty; pass++) { if (!hasCurrencySymbol && !currencySymbol.IsEmpty && s.EndsWith(currencySymbol, StringComparison.Ordinal)) @@ -1472,32 +1494,17 @@ private static ReadOnlySpan StripAffixes(ReadOnlySpan s, NumberStyle continue; } - if (!hasSign && style.HasFlag(NumberStyles.AllowTrailingSign)) + bool trailingNegative = false; + int signLength = hasSign || !allowTrailingSign ? 0 : ReadTrailingSign(s, numberFormat, out trailingNegative); + if (signLength == 0) { - int signLength = ReadTrailingSign(s, numberFormat, out bool trailingNegative); - if (signLength > 0) - { - hasSign = true; - isNegative = trailingNegative; - s = s[..^signLength]; - continue; - } + break; } - break; + hasSign = true; + isNegative = trailingNegative; + s = s[..^signLength]; } - - if (hasParentheses) - { - if (hasSign) - { - throw new FormatException(InvalidFormatMessage); - } - - isNegative = true; - } - - return s; } ///