Update the parsing/formatting logic to allow more than 99 digits

[tannergooding]

As per #46440 (comment), rather than adding new overloads API review opted to take the breaking change in how standard vs custom numeric format strings are differentiated.

As per the new rules, standard numeric format strings would be identified by an ASCII alphabetical character followed by a sequence of ASCII digits. If the number of digits doesn't fit into a System.Int32, it will throw.

This is a breaking change in that today 42.ToString("G999") is interpreted as a "custom numeric format string" and will print G999 as will other values such as 42.ToString("G100") printing G142. Moving forward, these would be interpreted as "standard numeric format strings" and would be treated as the G format specifier with the respective number of digits, thus printing 42.

Likewise, today 42.ToString("H999") is interpreted as a "custom numeric format string" and will print H999, while 42.ToString("H99") is interpreted as a "standard numeric format string" and since H is not supported, it throws. Moving forward, both will throw since H is not a supported "standard numeric format string". This leaves it open to be used in the future if necessary.

Format strings, like the following, would continue to be interpreted as "custom numeric format strings":

• start with any character that is not an ASCII alphabetical character (such as $ or è)
• start with an ASCII alphabetical character but which are not followed by an ASCII digit (such as A$)
• start with an ASCII alphabetical character, followed by an ASCII digit sequence, and then any character that is not an ASCII digit character (such as A55A).

For users who want to maintain the previous behavior (that is where 42.ToString("G999") was intentionally meant to return G999) they can explicitly single quote the first character (that is 42.ToString("'G'999")). This will work on .NET Framework, .NET Core, and .NET 5+

The Definition of Done for this includes (but is not limited to):

• Commenting on this issue with a test matrix of method pairs for round-tripping values through formatting/parsing
• Commenting on this issue with the full list of APIs that will be affected with new behavior
• Sharing that information with partner teams for advance notice before merging into master
• Creating a breaking change document issue
• Updating existing documentation for the affected APIs to reflect the changes

[ghost]

Tagging subscribers to this area: @tannergooding, @pgovind
See info in area-owners.md if you want to be subscribed.

Issue Details

---

As per #46440 (comment), rather than adding new overloads API review opted to take the breaking change in how standard vs custom numeric format strings are differentiated.

As per the new rules, standard numeric format strings would be identified by an ASCII alphabetical character followed by a sequence of digits. If the number of digits doesn't fit into a System.Int32, it will throw.

This is a breaking change in that today 42.ToString("G999") is interpreted as a "custom numeric format string" and will print G999 as will other values such as 42.ToString("G100") printing G142. Moving forward, these would be interpreted as "standard numeric format strings" and would be treated as the G format specifier with the respective number of digits, thus printing 42.

Likewise, today 42.ToString("H999") is interpreted as a "custom numeric format string" and will print H999, while 42.ToString("H99") is interpreted as a "standard numeric format string" and since H is not supported, it throws. Moving forward, both will throw since H is not a supported "standard numeric format string". This leaves it open to be used in the future if necessary.

Format strings that start with a non ASCII alphabetical character (such as $) or which start with an ASCII alphabetical character but which are not followed by a digit (such as A$) or which have start have an ASCII alphabetical character, followed by a digit sequence, and then any non ASCII digit character (such as A55A) would continue to be interpreted as "custom numeric format strings".

For users who want to maintain the previous behavior (that is where 42.ToString("G999") was intentionally meant to return G999) can maintain that behavior in a compatible way by explicitly single quoting the first character (that is 42.ToString("'G'999")).

Author:	tannergooding
Assignees:	-
Labels:	area-System.Numerics, untriaged
Milestone:	-

[tannergooding]

CC. @terrajobst (also the api-review-team alias seems to be missing)

[tannergooding]

CC. @dotnet/fxdc

[terrajobst]

CC. @terrajobst (also the api-review-team alias seems to be missing)

There isn't one, we use FXDC on GitHub, or maybe I don't understand what you mean.

[pgovind]

I have a fix locally. The only change needed was to remove the if (n > 10) break condition and a checked call: n = checked(n * 10 + (format[i++] - '0'));. The changes had to go into BigNumber.cs and Number.Formatting.cs.

Commenting on this issue with the full list of APIs that will be affected with new behavior

BigInteger.ToString(string format)
BigInteger.ToString(string format, IFormatProvider provider) // This should really be the same as the previous one but just being thorough here
BigInteger.TryFormat()
// Integral Types
int/uint/byte/sbyte/short/ushort/long/ulong.ToString(string format)
int/uint/byte/sbyte/short/ushort/long/ulong.ToString(string format, IFormatProvider provider)
int/uint/byte/sbyte/short/ushort/long/ulong.TryFormat()
// Floating Types
half/single/double/decimal.ToString(string format)
half/single/double/decimal.ToString(string format, IFormatProvider provider)
half/single/double/decimal.TryFormat()

Commenting on this issue with a test matrix of method pairs for round-tripping values through formatting/parsing

We're essentially only fixing the max 99 digits of precision here. So the existing tests for all these APIs need to be augmented with 2 cases: For each standard format specifier, add a test case with precision > 99. Add a test case with precision > intMax.
Also add test cases for unsupported format specifiers with precision > 99. These should now throw

// Since GH formatting is rudimentary
foreach (var formatter in [DecimalFormatter, CurrencyFormatter, ExponentialFormatter, FixedFormatter, NumberFormatter, PercentFormatter, HexFormatter]) {
    BigNumber:
         Test ToString with precision > 99 and precision > int.MaxValue
          Test TryFormat
    Integral Types:
         int/uint/byte/sbyte/short/ushort/long/ulong.ToString - precision > 99 and precision > int.MaxValue
    Non-Integral Types:
         half/single/double/decimal.ToString - precision > 99 and precision > int.MaxValue
         half/single/double/decimal.TryFormat
}

I also need to check that there are existing tests to cover:

• start with any character that is not an ASCII alphabetical character (such as $ or è)
• start with an ASCII alphabetical character but which are not followed by an ASCII digit (such as A$)
• start with an ASCII alphabetical character, followed by an ASCII digit sequence, and then any character that is not an ASCII digit character (such as A55A).

[pgovind]

Sharing that information with partner teams for advance notice before merging into master

Any idea who these partner teams might be?

[tannergooding]

At the very least, @mjsabby from Bing. @danmosemsft might know if we have a good way to broadcast these changes internally

[danmoseley]

I don't know of partners that are pulling directly from master at the moment (I believe Bing has paused for a little bit). My suggestion is to just use the breaking change process -- follow the issue template in the docs repo. @PriyaPurkayastha can confirm whether we need to do more. I think the Preview 1 snap happened, so we have a month or so.

[PriyaPurkayastha]

As mentioned by Dan for now, please create a breaking change issue using https:/dotnet/docs/issues/new?assignees=&labels=&template=dotnet-breaking-change.md&title=
We have a dotnetbcn alias to announce breaking changes but I need to work with all the engineering managers to identify the partner teams that need to be on that alias. The plan is to start sending out the breaking change issue links on that alias once we have had a chance to populate it with the right folks.

[pgovind]

Ah cool, thanks! Btw, I think Preview 1 snap is Monday Jan 25?

[danmoseley]

Ah yes I just saw the email. Thanks.