-
-
Notifications
You must be signed in to change notification settings - Fork 95
Fix floating point arguments being parsed as multiple parameters due to culture-specific formatting #2962
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ents Co-authored-by: thomhurst <[email protected]>
Co-authored-by: thomhurst <[email protected]>
…or numeric formatting Co-authored-by: thomhurst <[email protected]>
…to culture-specific formatting Co-authored-by: thomhurst <[email protected]>
|
@copilot We should still append the numeric representer in the emitted code. E.g. |
Co-authored-by: thomhurst <[email protected]>
Added numeric suffixes to the emitted code: |
This PR fixes an issue where floating point arguments in
[Arguments]attributes were incorrectly parsed as multiple parameters instead of single values, causingArgumentException: Expected exactly 1 argument, but got 2errors.Root Cause
The issue occurred when floating point numbers were formatted using culture-specific decimal separators. In cultures like German (
de-DE) and French (fr-FR), the decimal separator is a comma (,) instead of a period (.). This caused:1.1to be formatted as"1,1"["1", "1"]Example of the Issue
Solution
Updated three critical formatting methods to use
CultureInfo.InvariantCulturefor all numeric types:TestNameFormatter.FormatArgumentValue()- Ensures test names display consistentlyTypedConstantFormatter.FormatPrimitive()- Ensures source generator output is culture-independentTypedConstantParser.FormatPrimitive()- Ensures argument parsing is culture-independentThe fix ensures that floating point numbers are always formatted with period (
.) as the decimal separator, regardless of the user's locale settings.Additionally, the emitted code now includes proper numeric type suffixes for clarity and correctness:
dfor double values (e.g.,1.1d)ffor float values (e.g.,1.1f)mfor decimal values (e.g.,1.23m)Lfor long values (e.g.,123L)ULfor ulong values (e.g.,123UL)Ufor uint values (e.g.,123U)Testing
Verified the fix works correctly across multiple cultures:
1.1→"1.1d"✅1.1→"1.1d"✅ (previously"1,1")1.1→"1.1d"✅ (previously"1,1")All
ArgumentsAttributeinstances now correctly maintain exactly one argument value regardless of culture, and the emitted code includes proper numeric type suffixes.Fixes #2961.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.