Refactor decimal point handling in format method#1
Open
HerveDelvaux wants to merge 1 commit intomaster-1.xfrom
Open
Refactor decimal point handling in format method#1HerveDelvaux wants to merge 1 commit intomaster-1.xfrom
HerveDelvaux wants to merge 1 commit intomaster-1.xfrom
Conversation
### Context
`NumberFormatter::format()` supports an “auto” mode: when `$decimals` is `null`, it tries to preserve the number of decimals by converting the input to a string and counting the fractional digits.
The previous implementation used the current locale decimal separator (`localeconv()['decimal_point']`) to split the string and detect the fractional part.
### Problem
This approach is incorrect for floats when the locale uses a comma as decimal separator:
- In PHP, casting a **float** to string uses `.` as the decimal separator (e.g. `(string) 12.34` becomes `"12.34"`), regardless of the active locale.
- If the locale decimal separator is `,`, splitting `"12.34"` on `,` will never find the fractional part, and the code will incorrectly detect **0 decimals**.
Example (locale decimal point is `,`):
- `(string) 12.34` → `"12.34"`
- `explode(',', "12.34")` → `["12.34"]`
- detected decimals → `0` (wrong)
This can lead to incorrect formatting (e.g. rounding to an integer) when `$decimals` is not explicitly provided.
### What changed
- Locale-based decimal detection is now applied **only for non-float inputs**.
- For **floats**, the code always uses `.` to split the string representation, which is the only reliable decimal separator in this context.
### Why this is safe
- The function is designed to accept numeric values (`int|float`), so using the locale to “parse” floats is not only unreliable but also unnecessary.
- The patch keeps the original intent (preserving decimals when `$decimals` is `null`) while removing locale-dependent incorrect behavior for floats.
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
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.
Context
NumberFormatter::format()supports an “auto” mode: when$decimalsisnull, it tries to preserve the number of decimals by converting the input to a string and counting the fractional digits.The previous implementation used the current locale decimal separator (
localeconv()['decimal_point']) to split the string and detect the fractional part.Problem
This approach is incorrect for floats when the locale uses a comma as decimal separator:
.as the decimal separator (e.g.(string) 12.34becomes"12.34"), regardless of the active locale.,, splitting"12.34"on,will never find the fractional part, and the code will incorrectly detect 0 decimals.Example (locale decimal point is
,):(string) 12.34→"12.34"explode(',', "12.34")→["12.34"]0(wrong)This can lead to incorrect formatting (e.g. rounding to an integer) when
$decimalsis not explicitly provided.What changed
.to split the string representation, which is the only reliable decimal separator in this context.Why this is safe
int|float), so using the locale to “parse” floats is not only unreliable but also unnecessary.$decimalsisnull) while removing locale-dependent incorrect behavior for floats.