{{ message }}
Repository navigation
Add new NumberStyles option to stop parsing on invalid character - #130210
Merged
Merged
Conversation
) ## Description Added comprehensive test coverage for the `NumberStyles.AllowTrailingInvalidCharacters` mode across all numeric types. This style allows Parse/TryParse APIs to parse up to the first invalid character and return the number of characters consumed, similar to Utf8Parser behavior. ## Changes **Test Coverage Added:** - All integer types: Int32, Int64, Int16, SByte, UInt32, UInt64, UInt16, Byte - All floating point types: Double, Single, Half, Decimal - Arbitrary precision: BigInteger **Test Scenarios:** - Basic parsing with trailing invalid characters (e.g., `"123abc"` → value: 123, consumed: 3) - Combined with NumberStyles: Integer, HexNumber, BinaryNumber, Currency, Float - Edge cases: max/min values, empty strings, overflow, special values (Infinity, NaN) - All API surface: string, ReadOnlySpan\<char\>, ReadOnlySpan\<byte\> overloads - Both success and failure paths with proper validation of characters/bytes consumed Tests follow existing patterns in System.Runtime.Tests and mirror the ValidateParser approach used in System.Memory Utf8Parser tests. ## Testing Tests build successfully. Full baseline build required for test execution would exceed time constraints. <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tannergooding <10487869+tannergooding@users.noreply.github.com>
- Reset elementsConsumed to 0 on the integer and decimal overflow paths so a failed parse never reports a nonzero consumed count. - Restrict floating-point special-value (Infinity/NaN) matching to the non-hex path and honor AllowTrailingInvalidCharacters via prefix matching, reporting leading whitespace plus the matched symbol as consumed. - Apply the same special-value consistency fix to the IEEE-754 decimal path (Decimal32/64/128) and share the matching logic via a helper. - Add AllowTrailingInvalidCharacters to the hex and binary allowed styles. - Remove redundant string null guards from TryParse overloads and rely on ReadOnlySpan null-tolerance; relax internal asserts to tolerate empty spans. - Restore the UTF-8 BOM inadvertently stripped from Double/Single and the IEEE-754 decimal source files. - Add and correct AllowTrailingInvalidCharacters tests, using CultureInfo.InvariantCulture for culture-dependent special values. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for parsing numeric prefixes (stopping at the first invalid character) via a new NumberStyles.AllowTrailingInvalidCharacters flag, and introduces TryParse overloads that report how many UTF-16 chars / UTF-8 bytes were consumed. The change updates the CoreLib parsing pipelines and extends coverage across built-in numeric types plus BigInteger and Complex, with broad new tests.
Changes:
- Add
NumberStyles.AllowTrailingInvalidCharactersand plumb “consumed count” (charsConsumed/bytesConsumed) through numeric parsing implementations. - Add/override new
TryParse(..., out int consumed)APIs across numeric types (CoreLib, Numerics) and update ref assemblies accordingly. - Add extensive tests validating consumed counts and behavior for trailing invalid characters (including UTF-8 overloads in many cases).
Show a summary per file
Copilot's findings
- Files reviewed: 56/56 changed files
- Comments generated: 5
…acters - Decimal.cs: the new consumed-count TryParse overloads now use ValidateParseStyleDecimal instead of ValidateParseStyleFloatingPoint, matching the rest of decimal parsing (which rejects hex/binary specifiers). - INumberBase.cs: the default consumed-count TryParse implementations no longer strip NumberStyles.AllowTrailingInvalidCharacters before delegating. Per the approved design, the style is passed through unmodified so a type that does not understand the flag throws as expected, rather than silently succeeding with a masked style. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This was referenced Jul 5, 2026
…ests - Allow NumberStyles.AllowTrailingInvalidCharacters to be combined with HexFloat. ValidateParseStyleFloatingPoint previously rejected HexFloat | AllowTrailingInvalidCharacters (via the `style & ~HexFloat` mask), which made the AllowTrailingInvalidCharacters handling already present in TryParseHexFloatingPoint unreachable. Widen the mask to permit the flag, matching how ValidateParseStyleInteger already permits HexNumber | AllowTrailingInvalidCharacters and BinaryNumber | AllowTrailingInvalidCharacters. - Update the AllowBinarySpecifier XML docs to note that AllowTrailingInvalidCharacters may also be combined with it, consistent with the updated validation behavior. - Add UTF-8 (ReadOnlySpan<byte>) failure-path assertions (bytesConsumed == 0) to Parse_AllowTrailingInvalidCharacters_Invalid in the test files that were missing them (Double, Single already had it; Half, Decimal, Decimal32/64/128, and Int64 did not), locking down the new consumed-count contract for the UTF-8 overloads. - Add double hex-float regression cases for HexFloat | AllowTrailingInvalidCharacters, including one that stops on a trailing invalid character after consuming trailing whitespace. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…F-8 invalid assertion - Use CultureInfo.InvariantCulture instead of null provider for floating-point/decimal Parse_AllowTrailingInvalidCharacters test data (Double/Single/Half/Decimal) so cases with a decimal point/exponent are deterministic across cultures. - Assert the UTF-8 (bytesConsumed == 0) failure contract in BigInteger's Parse_AllowTrailingInvalidCharacters_Invalid test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
PranavSenthilnathan
approved these changes
Jul 8, 2026
PranavSenthilnathan
left a comment
Member
There was a problem hiding this comment.
Copilot beat me to a few of the comments in previous iterations 😄 The rest of the comments I have are minor
…, expand special-value tests - Remove [MethodImpl(NoInlining)] from ConsumeTrailingNulls; it is small, centralized, and always called on the trailing path. - Restructure TryParseFloat so the special-value (Infinity/NaN) handling runs for both the hex and non-hex paths. Hex floating-point now supports the Infinity/NaN symbols and the misleading "not supported" comment is corrected. - Add the surrounding-whitespace special-value (Infinity/NaN) test cases to Single/Half/Decimal32/Decimal64 to match Double/Decimal128. - Add hex-float Infinity/NaN test cases (valid and trailing-invalid) to Double/Single/Half and remove the now-obsolete invalid entries that asserted NaN/Infinity were rejected under HexFloat. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Matching the Infinity/NaN symbols via SpanStartsWith(..., OrdinalIgnoreCase) routed the UTF-8 path through StartsWithIgnoreCaseUtf8_Scalar, which trips a Debug.Assert(length == 0) when the trailing 1-3 byte chunk matches case- insensitively but not exactly (e.g. UTF-8 "NAN" vs the "NaN" symbol). The UTF-16 tests only used exact-case symbols, so this surfaced via case-variant UTF-8 input such as the System.Text.Json named-literal parsing. Match the symbol against an equal-length slice using EqualsOrdinalIgnoreCase (the well-tested equality path, as the original code did) which is semantically identical to a case-insensitive prefix match here. Also remove the now-obsolete "NaN/Infinity not valid for HexFloat" invalid entries from BFloat16Tests and add the corresponding valid hex Infinity/NaN cases, matching Double/Single/Half. Add mixed-case special-value cases to DoubleTests to exercise the UTF-8 case-insensitive path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Member
Author
PranavSenthilnathan
approved these changes
Jul 8, 2026
eiriktsarpalis
pushed a commit
that referenced
this pull request
Jul 15, 2026
…0210) This is a continuation of #123944, now that I had a chance to finish getting copilot to respond to the feedback, which was namely just tests. The original commit (product code) is still the original which was done by hand. This resolves #87171 --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: tannergooding <10487869+tannergooding@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

This is a continuation of #123944, now that I had a chance to finish getting copilot to respond to the feedback, which was namely just tests. The original commit (product code) is still the original which was done by hand.
This resolves #87171