Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 20, 2025

This PR fixes an issue where floating point arguments in [Arguments] attributes were incorrectly parsed as multiple parameters instead of single values, causing ArgumentException: Expected exactly 1 argument, but got 2 errors.

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.1 to be formatted as "1,1"
  • The formatted string could then be mistakenly parsed as two separate arguments ["1", "1"]
  • Tests would fail with "Expected exactly 1 argument, but got 2"

Example of the Issue

[Test]
[Arguments(1.1)]
[Arguments(0.1)] 
[Arguments(1e-1)]
public void ParameterTest(double number)
{
    // This would fail in de-DE culture with:
    // ArgumentException: Expected exactly 1 argument, but got 2
}

Solution

Updated three critical formatting methods to use CultureInfo.InvariantCulture for all numeric types:

  1. TestNameFormatter.FormatArgumentValue() - Ensures test names display consistently
  2. TypedConstantFormatter.FormatPrimitive() - Ensures source generator output is culture-independent
  3. TypedConstantParser.FormatPrimitive() - Ensures argument parsing is culture-independent

The 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:

  • d for double values (e.g., 1.1d)
  • f for float values (e.g., 1.1f)
  • m for decimal values (e.g., 1.23m)
  • L for long values (e.g., 123L)
  • UL for ulong values (e.g., 123UL)
  • U for uint values (e.g., 123U)

Testing

Verified the fix works correctly across multiple cultures:

  • English (US): 1.1"1.1d"
  • German (DE): 1.1"1.1d" ✅ (previously "1,1")
  • French (FR): 1.1"1.1d" ✅ (previously "1,1")

All ArgumentsAttribute instances 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.

Copilot AI and others added 4 commits August 20, 2025 11:36
…ents

Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>
Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>
…or numeric formatting

Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>
…to culture-specific formatting

Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] [Arguments] creates wrong number of parameters with double floating point parameters Fix floating point arguments being parsed as multiple parameters due to culture-specific formatting Aug 20, 2025
@Copilot Copilot AI requested a review from thomhurst August 20, 2025 11:58
Copilot finished work on behalf of thomhurst August 20, 2025 11:58
@thomhurst thomhurst marked this pull request as ready for review August 20, 2025 12:26
@thomhurst
Copy link
Owner

@copilot We should still append the numeric representer in the emitted code. E.g. f for float, d for double

Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Aug 20, 2025

@copilot We should still append the numeric representer in the emitted code. E.g. f for float, d for double

Added numeric suffixes to the emitted code: d for double, f for float, m for decimal, L for long, UL for ulong, and U for uint. Updated both TypedConstantFormatter and TypedConstantParser to ensure consistency. Source generator test snapshots have been updated to reflect the changes. (4085248)

Copilot finished work on behalf of thomhurst August 20, 2025 13:32
@Copilot Copilot AI temporarily deployed to Pull Requests August 20, 2025 13:33 Inactive
@Copilot Copilot AI temporarily deployed to Pull Requests August 20, 2025 13:33 Inactive
@Copilot Copilot AI temporarily deployed to Pull Requests August 20, 2025 13:33 Inactive
@thomhurst thomhurst enabled auto-merge (squash) August 20, 2025 13:44
@thomhurst thomhurst merged commit 00fa751 into main Aug 20, 2025
8 of 9 checks passed
@thomhurst thomhurst deleted the copilot/fix-2961 branch August 20, 2025 13:55
This was referenced Aug 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Arguments] creates wrong number of parameters with double floating point parameters
2 participants