Use InvariantCulture for parameter value formatting#2354
Merged
alexeyzimarev merged 1 commit intodevfrom Feb 26, 2026
Merged
Conversation
… values to strings Previously, AddParameter<T>, AddQueryParameter<T>, AddUrlSegment<T>, AddHeader<T>, AddOrUpdateParameter<T>, AddOrUpdateHeader<T>, and Parameter.CreateParameter all used ToString() which respects the current culture. This caused issues on non-English locales where e.g. 1.234 would be formatted as "1,234" with a comma decimal separator. All generic overloads now default to InvariantCulture and accept an optional CultureInfo parameter for callers who need locale-specific formatting. Fixes #2270 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Review Summary by QodoUse InvariantCulture for parameter value formatting across all generic overloads
WalkthroughsDescription• Use CultureInfo.InvariantCulture by default for all generic parameter formatting methods • Add optional CultureInfo parameter to allow locale-specific formatting when needed • Implement ToStringValue() helper extension for consistent invariant culture conversion • Update ObjectParser and Parameter.CreateParameter to use invariant culture formatting Diagramflowchart LR
A["Parameter Methods<br/>AddParameter, AddOrUpdateParameter<br/>AddQueryParameter, AddUrlSegment<br/>AddHeader, AddOrUpdateHeader"] -->|"Use ToStringValue()"| B["ToStringValue Extension<br/>IFormattable.ToString<br/>CultureInfo.InvariantCulture"]
B -->|"Default Culture"| C["InvariantCulture<br/>Decimal: 1.234"]
B -->|"Optional Override"| D["Custom Culture<br/>Decimal: 1,234"]
E["ObjectParser.ParseValue<br/>Parameter.CreateParameter"] -->|"Use ToStringValue()"| B
File Changes1. src/RestSharp/Extensions/StringExtensions.cs
|
Contributor
Code Review by Qodo
1. Nullable struct becomes null
|
Deploying restsharp with
|
| Latest commit: |
6e7947e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7739fd96.restsharp.pages.dev |
| Branch Preview URL: | https://fix-invariant-culture-parame.restsharp.pages.dev |
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+152
to
+153
| internal static string? ToStringValue(this object? value, CultureInfo? culture = null) | ||
| => value is IFormattable f ? f.ToString(null, culture ?? CultureInfo.InvariantCulture) : value?.ToString(); |
Contributor
There was a problem hiding this comment.
1. Nullable struct becomes null 🐞 Bug ✓ Correctness
ToStringValue(object?) returns null for null input, and generic where T : struct overloads box Nullable<T> with no value to null. This can change query-string output from name= to name and can throw ArgumentNullException in AddHeader<T>/AddOrUpdateHeader<T> when called with nullable structs that have no value.
Agent Prompt
## Issue description
Generic parameter/header overloads (`where T : struct`) now call `ToStringValue(object?)`. When `T` is `Nullable<U>` and the value has no value, boxing results in `null`, so `ToStringValue` returns `null`.
This changes behavior:
- **Headers**: `AddHeader<T>`/`AddOrUpdateHeader<T>` call `Ensure.NotNull(...)` and will now throw for `T = int?` etc when null.
- **Query-string**: RestSharp renders `null` values as just the parameter name (no `=`), changing output compared to empty-string values.
## Issue Context
The new invariant-culture behavior is correct for non-null values, but nullable structs are an important edge case because they are still `struct` and thus accepted by these overloads.
## Fix Focus Areas
- src/RestSharp/Extensions/StringExtensions.cs[152-153]
- src/RestSharp/Request/RestRequestExtensions.Headers.cs[54-57]
- src/RestSharp/Request/RestRequestExtensions.Headers.cs[76-78]
- src/RestSharp/Request/RestRequestExtensions.cs[58-61]
- src/RestSharp/Request/RestRequestExtensions.cs[80-82]
- src/RestSharp/Request/RestRequestExtensions.Query.cs[44-46]
- src/RestSharp/Request/RestRequestExtensions.Url.cs[44-46]
- src/RestSharp/BuildUriExtensions.cs[75-88]
- test/RestSharp.Tests/InvariantCultureTests.cs[1-98]
## Implementation guidance
1. Add a struct-specific helper (or update the call sites) to preserve nullable semantics:
- If `T` is `Nullable<U>` and `value` is default (no value), return `string.Empty` (matching `Nullable<T>.ToString()` behavior).
- Otherwise format via invariant culture (`IFormattable.ToString(null, ...)`).
2. Update generic overloads to call the struct-specific helper to avoid boxing-to-null.
3. Add tests:
- `AddHeader("x", (int?)null)` should not throw and should set header value to empty string (or whatever behavior you decide, but make it consistent/documented).
- `AddQueryParameter("x", (int?)null)` + `BuildUriString` should verify whether output is `x=` vs `x` according to the intended behavior (and lock it in with a test).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Member
Author
There was a problem hiding this comment.
This is a false positive. The where T : struct constraint specifically excludes Nullable<T> — the C# compiler rejects it:
error CS0453: The type 'int?' must be a non-nullable value type in order to use it
as parameter 'T' in the generic type or method 'Test<T>(T)'
So AddParameter<int?>("name", null) won't compile. There's no way to pass a null value through these generic overloads.
Contributor
There was a problem hiding this comment.
Question:
This is a false positive. The where T : struct constraint specifically excludes Nullable<T> — the C# compiler rejects it: error CS0453: The type 'int?' must be a non-nullable value type in order to use it as parameter 'T' in the generic type or method 'Test<T>(T)' So AddParameter<int?>(name, null) won't compile. There's no way to pass a null value through these generic overloads.
Answer:
Add a C#-specific guardrail to the improve prompt so suggestions don’t assume where T : struct accepts Nullable<T> (and to require compile-checked reasoning before proposing changes).
In your .pr_agent.toml (or as an inline flag on /improve), set:
[pr_code_suggestions]
extra_instructions="""\
For C#:
- Do NOT assume `where T : struct` permits `Nullable<T>` (`int?`, etc.). The compiler rejects `T = Nullable<U>` under this constraint (CS0453).
- Before flagging a bug involving generic constraints, explicitly verify (in the explanation) whether the code can compile and the call site is legal.
- If the scenario requires `T = int?` through a `where T : struct` API, treat it as invalid/unreachable unless there is evidence of a different overload or non-generic path.
"""If you only want this behavior for one PR run, use:
/improve --pr_code_suggestions.extra_instructions="For C#: do not claim `where T : struct` accepts `Nullable<T>` (CS0453); verify compilation/legal call sites before suggesting fixes."
Relevant Sources:
- https://docs.qodo.ai/qodo-documentation/code-review/qodo-merge/tools/improve#extra-instructions-and-best-practices
- https://docs.qodo.ai/qodo-documentation/code-review/qodo-merge/tools/improve#extra-instructions
- https://docs.qodo.ai/qodo-documentation/code-review/qodo-merge/usage-guide/additional_configurations#extra-instructions
- https://docs.qodo.ai/qodo-documentation/code-review/qodo-merge/usage-guide/configuration_options#local-configuration-file
Test Results 42 files 42 suites 17m 55s ⏱️ For more details on these failures, see this check. Results for commit 6e7947e. |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
<T> where T : structoverloads (AddParameter<T>,AddOrUpdateParameter<T>,AddQueryParameter<T>,AddUrlSegment<T>,AddHeader<T>,AddOrUpdateHeader<T>) now format values usingCultureInfo.InvariantCultureby defaultParameter.CreateParameterandObjectParser.ParseValuealso use invariant cultureCultureInfo? cultureparameter is added to all generic overloads for callers who need locale-specific formatting (e.g..AddParameter("value", 1.234, culture: new CultureInfo("da-DK")))ToStringValue(CultureInfo?)helper inStringExtensionsFixes #2270
Test plan
🤖 Generated with Claude Code