Add support for thousands separators in [bigint] casting #25396

AbishekPonmudi · 2025-04-21T16:20:23Z

PR Summary

This PR resolves a parsing issue in PowerShell where numeric strings containing thousands separators (e.g., 1,000) fail to convert into [bigint], resulting in an InvalidArgument exception. The root cause is the absence of NumberStyles.AllowThousands and lack of formatting cleanup before conversion. This update ensures consistent and locale-aware conversion behavior for large numeric strings.

PR Context

The issue occurs when users attempt to cast strings like '1,000' to [bigint], and PowerShell does not account for grouping symbols defined by culture. This leads to type conversion errors in scripts or user input relying on formatted numbers.

Solution

Updates made to the ConvertFrom() method in LanguagePrimitives.cs include:

Casting sourceValue using (string) instead of as string for type safety.
Dynamically handling culture-specific formatting to ensure group separators are correctly removed.
Including NumberStyles.AllowThousands for accurate parsing of BigInteger.

Error Message Before Fix

InvalidArgument: Cannot convert value "1,000" to type "System.Numerics.BigInteger". Error: "The value could not be parsed."

After Fix

PS> [bigint] '1,000'
'1000'

Why This Fix?

Prevents casting errors when handling numeric strings with thousands separators.
Improves PowerShell’s type conversion reliability.
Ensures compatibility with standard numeric formatting.

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
This PR is ready to merge. If this PR is a work in progress, please open this as a Draft Pull Request and mark it as Ready to Review when it is ready to merge.
Breaking changes
- None
User-facing changes
- Not Applicable
Testing - New and feature
- added a new test (https://github.com/PowerShell/PowerShell/blob/master/.github/CONTRIBUTING.md#before-submitting)
  - Added unit tests to verify comma-separated numeric string conversion.

Related Issues

FIXED ----

Fixes [bigint] casting breaks with strings containing thousands separators (commas) #25368

…casting errors.

src/System.Management.Automation/engine/LanguagePrimitives.cs

…asting errors… (PowerShell#25396)

…casting errors (PowerShell#25396) # Issue Summary PowerShell fails to convert numeric strings with thousands separators (e.g., commas) to `[bigint]`, causing casting errors. This occurs because the `ConvertFrom()` method in `LanguagePrimitives.cs` doesn’t handle formatted numbers. ## Cause - The `ConvertTo()` method doesn’t remove thousands separators (e.g., `,` in en-US), leading to `BigInteger.Parse()` failures. - This affects scripts handling large numbers with separators. ## Solution Modify `ConvertFrom()` to: - Detect the culture-specific thousands separator. - Remove it before conversion.

iSazonov · 2025-04-22T07:57:14Z

In related issue there is a reference to code where exception is raised

PowerShell/src/System.Management.Automation/engine/LanguagePrimitives.cs

Lines 2946 to 2955 in dd1fb91

    
           if (resultType == typeof(BigInteger)) 
        
           { 
        
               // Fallback for BigInteger: manual parsing using any common format. 
        
               NumberStyles style = NumberStyles.AllowLeadingSign 
        
                   | NumberStyles.AllowDecimalPoint 
        
                   | NumberStyles.AllowExponent 
        
                   | NumberStyles.AllowHexSpecifier; 
        
               return BigInteger.Parse(strToConvert, style, NumberFormatInfo.InvariantInfo); 
        
           }

I guess it is right place for a fix.
Also I don't think we should do any normalizations since BigInteger.(Try)Parser() can do all work.

AbishekPonmudi · 2025-04-22T08:54:49Z

Hey @iSazonov,

Thanks for the heads-up! I went ahead and tweaked LanguagePrimitives.cs (lines 2957–2966) to fix that BigInteger.Parse() issue with thousands separators. I ditched NumberStyles.AllowHexSpecifier, swapped BigInteger.Parse() for BigInteger.TryParse() to keep things smooth, and made sure it handles stuff like "1,000" → 1000 and "5,432,100" → 5432100 correctly.

if (resultType == typeof(BigInteger)) 
{ 
    // Adjust NumberStyles to remove AllowHexSpecifier for standard numeric parsing
    NumberStyles style = NumberStyles.AllowLeadingSign 
        | NumberStyles.AllowDecimalPoint 
        | NumberStyles.AllowExponent; // Removed AllowHexSpecifier  

    BigInteger parsedValue;
    if (BigInteger.TryParse(strToConvert, style, NumberFormatInfo.InvariantInfo, out parsedValue))
    {
        return parsedValue;
    }
    throw new PSInvalidCastException("Failed to convert string to BigInteger.");
}

…ted numeric strings (PowerShell#25396)

src/System.Management.Automation/engine/LanguagePrimitives.cs

…eparators (PowerShell#25396)

AbishekPonmudi

Thanks for the review, @iSazonov! I’ve updated LanguagePrimitives.cs to remove the unnecessary comment, replaced BigInteger.TryParse() with BigInteger.Parse(), and adjusted NumberStyles to allow thousands separators. Let me know if any further refinements are needed!

iSazonov · 2025-04-22T16:31:22Z

@AbishekPonmudi Please add new tests to cover the code. (The tests should use current culture NumberGroupSeparator .)

src/System.Management.Automation/engine/LanguagePrimitives.cs

…casting errors (PowerShell#25396)

…ng error (Powershell PowerShell#25396)

AbishekPonmudi

Thanks for the review, @iSazonov! I’ve updated the code to remove manual separator normalization and now rely on BigInteger.Parse() with NumberStyles.AllowThousands to handle formatted numeric strings directly. Let me know if any further refinements are needed!

…casting errors (PowerShell#25396)

iSazonov · 2025-04-26T16:09:45Z

@AbishekPonmudi Please add tests to cover all scenarios you are trying to fix.

AbishekPonmudi · 2025-04-26T17:58:33Z

@AbishekPonmudi Please add tests to cover all scenarios you are trying to fix.

I ran manual tests for different locales using the file BigIntegerCultureHandling.ps1 as mentioned in the visual reference. I verified that PowerShell correctly handles thousands of separators, everything seems to be working fine based on these tests.
Please let me know if there are any issues or if any further refinements are needed :)

iSazonov · 2025-04-27T07:30:17Z

@AbishekPonmudi You should add tests to the PR.

…ultures (PowerShell#25396)

AbishekPonmudi · 2025-04-27T12:31:10Z

Hi @iSazonov,

I have submitted a separate Pull Request (#25463) that includes the necessary unit tests for BigInteger parsing across different cultures. These tests verify correct handling of thousands separators using PowerShell’s current culture settings.

Please refer to the test PR here: Tests for BigInteger parsing across cultures (#25463).

Let me know if any changes or refinements are needed to do better with the expected behavior....

iSazonov · 2025-04-28T03:30:12Z

@AbishekPonmudi If the PR fix #25368 we need tests in the PR which cover scenarios from that issue.

…casting error (PowerShell#25396)

mklement0 · 2025-05-07T19:30:44Z

test/powershell/engine/BigIntegerCultureHandling.test.ps1

+Describe 'PowerShell Type Conversion - BigInteger Parsing' -Tag 'CI' {
+
+    It 'Can convert formatted numbers using PowerShell type system' {
+        [System.Globalization.CultureInfo]::CurrentCulture = [System.Globalization.CultureInfo]::GetCultureInfo("en-US")


Since the culture angle is now covered by the de-DE test, I suggest removing this.

mklement0 · 2025-05-07T19:32:35Z

test/powershell/engine/BigIntegerCultureHandling.test.ps1

+        $convertedValue | Should -Be 1000
+    }
+
+    It 'Handles large comma-separated numbers that previously failed' {


Quibble: perhaps better: `Handles large numbers with thousands separators that previously failed'

mklement0 · 2025-05-07T19:34:40Z

test/powershell/engine/BigIntegerCultureHandling.test.ps1

+        $convertedValue | Should -Be 1000000
+    }
+
+    It 'Parses number using de-DE culture but falls back to invariant behavior' {


Quibble: Since the current culture never comes into play, I wouldn't say "Parses number using de-DE". Perhaps better: Parses a number string using the invariant culture, irrespective of the current culture

Thanks for the feedback @mklement0 , I have removed the line as suggested, and updated the "Handles large comma-separated numbers" test to "Handles large numbers with thousands separators" for clarity. Also, I have reworded the "de-DE" test to better reflect that it uses invariant culture, irrespective of the current culture.

Get me again ! if anything needed to better :)

mklement0

@iSazonov, everything LGTM now.

iSazonov · 2025-05-08T07:29:23Z

test/powershell/engine/BigIntegerCultureHandling.test.ps1

+# Copyright (c) Microsoft Corporation.
+# Licensed under the MIT License.
+
+Describe 'PowerShell Type Conversion - BigInteger Parsing' -Tag 'CI' {


Please all tests in LanguagePrimitive.Tests.ps1

iSazonov · 2025-05-08T09:44:35Z

test/powershell/engine/Api/LanguagePrimitive.Tests.ps1

+    It 'Can convert formatted numbers using PowerShell type system' {
+        [System.Globalization.CultureInfo]::CurrentCulture = [System.Globalization.CultureInfo]::GetCultureInfo("en-US")
+        $formattedNumber = "1,000"
+        $convertedValue = [System.Management.Automation.LanguagePrimitives]::ConvertTo($formattedNumber, [bigint])
+        $convertedValue | Should -Be 1000
+    }


The test was removed before.

AbishekPonmudi · 2025-05-08T11:19:06Z

@iSazonov Is everything is resolved.

microsoft-github-policy-service · 2025-05-08T12:10:55Z

📣 Hey @@AbishekPonmudi, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

Microsoft Forms

iSazonov · 2025-05-08T12:11:52Z

@AbishekPonmudi Thanks for your contribution! Please update the PR description to exactly reflect the change.

iSazonov · 2025-05-08T12:12:27Z

@mklement0 Thanks for review!

AbishekPonmudi · 2025-05-08T12:15:42Z

@iSazonov Thank you for reviewing my contribution! I appreciate your feedback. I have updated the PR description to reflect the implemented changes.

Fix: Remove commas from numeric strings before conversion to prevent …

b5304ae

…casting errors.

iSazonov reviewed Apr 21, 2025

View reviewed changes

src/System.Management.Automation/engine/LanguagePrimitives.cs Outdated Show resolved Hide resolved

Merge branch 'master' into patch-3

9946284

AbishekPonmudi added a commit to AbishekPonmudi/PowerShell that referenced this pull request Apr 22, 2025

Fix Remove commas from numeric strings before conversion to prevent c…

9360964

…asting errors… (PowerShell#25396)

AbishekPonmudi requested a review from iSazonov April 22, 2025 07:01

Fix: Remove AllowHexSpecifier in BigInteger parsing to support format…

82f8b7b

…ted numeric strings (PowerShell#25396)

iSazonov reviewed Apr 22, 2025

View reviewed changes

src/System.Management.Automation/engine/LanguagePrimitives.cs Outdated Show resolved Hide resolved

src/System.Management.Automation/engine/LanguagePrimitives.cs Outdated Show resolved Hide resolved

src/System.Management.Automation/engine/LanguagePrimitives.cs Outdated Show resolved Hide resolved

AbishekPonmudi added 2 commits April 22, 2025 19:05

Fix: Update BigInteger parsing to use Parse() and support thousands s…

a66921a

…eparators (PowerShell#25396)

Fix: Update BigInteger parsing to use Parse() and support thousands s…

861177e

…eparators (PowerShell#25396)

AbishekPonmudi commented Apr 22, 2025

View reviewed changes

Merge branch 'master' into patch-3

8b0bf4e

iSazonov reviewed Apr 25, 2025

View reviewed changes

src/System.Management.Automation/engine/LanguagePrimitives.cs Outdated Show resolved Hide resolved

AbishekPonmudi added 2 commits April 25, 2025 10:53

Fix: Remove commas from numeric strings before conversion to prevent …

ad2bde6

…casting errors (PowerShell#25396)

Remove commas from numeric strings before conversion to prevent casti…

c2c1725

…ng error (Powershell PowerShell#25396)

AbishekPonmudi commented Apr 26, 2025

View reviewed changes

Fix: Remove commas from numeric strings before conversion to prevent …

fd9cdc0

…casting errors (PowerShell#25396)

AbishekPonmudi added a commit to AbishekPonmudi/PowerShell that referenced this pull request Apr 27, 2025

Added tests for BigInteger parsing with thousands separators across c…

bc7df3d

…ultures (PowerShell#25396)

AbishekPonmudi added a commit to AbishekPonmudi/PowerShell that referenced this pull request Apr 27, 2025

Added tests for BigInteger parsing with thousands separators across c…

be627d8

…ultures (PowerShell#25396)

AbishekPonmudi mentioned this pull request Apr 27, 2025

Tests for BigInteger parsing across cultures (#25396) #25463

Closed

AbishekPonmudi added a commit to AbishekPonmudi/PowerShell that referenced this pull request Apr 28, 2025

Fix: Remove commas from numeric strings before conversion to prevent …

d320e4b

…casting error (PowerShell#25396)

Merge branch 'PowerShell:master' into patch-3

d76fcd3

AbishekPonmudi added 2 commits May 8, 2025 00:42

Tests for BigInteger Parsing Across Cultures (PowerShell#25396)

b5c28a3

Merge branch 'master' into patch-3

0fd3199

AbishekPonmudi requested a review from mklement0 May 7, 2025 19:29

mklement0 reviewed May 7, 2025

View reviewed changes

AbishekPonmudi added 2 commits May 8, 2025 01:10

Tests for BigInteger Parsing Across Cultures (PowerShell#25396)

b9ca9fc

Tests for BigInteger Parsing Across Cultures (PowerShell#25396)

010f04d

AbishekPonmudi requested a review from mklement0 May 7, 2025 19:50

mklement0 approved these changes May 7, 2025

View reviewed changes

AbishekPonmudi requested a review from iSazonov May 7, 2025 19:55

iSazonov reviewed May 8, 2025

View reviewed changes

AbishekPonmudi requested a review from iSazonov May 8, 2025 07:40

AbishekPonmudi added 3 commits May 8, 2025 13:21

Add Tests for BigInteger Parsing Across Cultures (PowerShell#25396)

e2bc432

Delete test/powershell/engine/BigIntegerCultureHandling.test.ps1

c4a08ff

Add Tests for BigInteger Parsing Across Cultures (PowerShell#25396)

97ec872

iSazonov reviewed May 8, 2025

View reviewed changes

Add Tests for BigInteger Parsing Across Cultures (PowerShell#25396)

c89ed59

AbishekPonmudi requested a review from iSazonov May 8, 2025 09:53

This comment was marked as outdated.

Sign in to view

PowerShell deleted a comment from azure-pipelines bot May 8, 2025

iSazonov approved these changes May 8, 2025

View reviewed changes

iSazonov merged commit 1b786d2 into PowerShell:master May 8, 2025
36 checks passed

AbishekPonmudi deleted the patch-3 branch May 9, 2025 12:47

Add support for thousands separators in [bigint] casting #25396

Add support for thousands separators in [bigint] casting #25396

Uh oh!

Conversation

AbishekPonmudi commented Apr 21, 2025 • edited by iSazonov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

Solution

Error Message Before Fix

After Fix

Why This Fix?

PR Checklist

Related Issues

FIXED ----

Uh oh!

Uh oh!

iSazonov commented Apr 22, 2025

Uh oh!

AbishekPonmudi commented Apr 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AbishekPonmudi left a comment

Choose a reason for hiding this comment

Uh oh!

iSazonov commented Apr 22, 2025

Uh oh!

Uh oh!

AbishekPonmudi left a comment

Choose a reason for hiding this comment

Uh oh!

iSazonov commented Apr 26, 2025

Uh oh!

AbishekPonmudi commented Apr 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iSazonov commented Apr 27, 2025

Uh oh!

AbishekPonmudi commented Apr 27, 2025

Uh oh!

iSazonov commented Apr 28, 2025

Uh oh!

mklement0 May 7, 2025

Choose a reason for hiding this comment

Uh oh!

mklement0 May 7, 2025

Choose a reason for hiding this comment

Uh oh!

mklement0 May 7, 2025

Choose a reason for hiding this comment

Uh oh!

AbishekPonmudi May 7, 2025

Choose a reason for hiding this comment

Uh oh!

mklement0 left a comment

Choose a reason for hiding this comment

Uh oh!

iSazonov May 8, 2025

Choose a reason for hiding this comment

Uh oh!

iSazonov May 8, 2025

Choose a reason for hiding this comment

Uh oh!

AbishekPonmudi commented May 8, 2025

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

Uh oh!

microsoft-github-policy-service bot commented May 8, 2025 • edited by unfurl-links bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iSazonov commented May 8, 2025

Uh oh!

iSazonov commented May 8, 2025

Uh oh!

AbishekPonmudi commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AbishekPonmudi commented Apr 21, 2025 •

edited by iSazonov

Loading

AbishekPonmudi commented Apr 26, 2025 •

edited

Loading

microsoft-github-policy-service bot commented May 8, 2025 •

edited by unfurl-links bot

Loading

AbishekPonmudi commented May 8, 2025 •

edited

Loading