8000 The NumberToLocalizedStringTransformer mix int and float inconsistently. · Issue #35775 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

The NumberToLocalizedStringTransformer mix int and float inconsistently. #35775

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.

Sign up for GitHub

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

Jump to bottom
Closed
VincentLanglet opened this issue Feb 18, 2020 · 0 comments
Closed

The NumberToLocalizedStringTransformer mix int and float inconsistently. #35775

VincentLanglet opened this issue Feb 18, 2020 · 0 comments
Labels
Bug Form Status: Needs Review

Comments

@VincentLanglet
Copy link
Contributor

Symfony version(s) affected: 3.4, 4.4, 5.0, ...

Description
When using the NumberType, the reverse transform method does:

if (false !== strpos($value, $decSep)) {
    $type = \NumberFormatter::TYPE_DOUBLE;
} else {
    $type = PHP_INT_SIZE === 8 ? \NumberFormatter::TYPE_INT64 : \NumberFormatter::TYPE_INT32;
}

Which means,

  • If 1 is submitted, an int should be set.
  • If 1.0 is submitted, a float should be set.

But then, a private function round is called and does:

private function round($number)
    {
        if (null !== $this->scale && null !== $this->roundingMode) {
            // shift number to maintain the correct scale during rounding
            $roundingCoef = pow(10, $this->scale);
            // string representation to avoid rounding errors, similar to bcmul()
            $number = (string) ($number * $roundingCoef);

            switch ($this->roundingMode) {
                ...
                case self::ROUND_HALF_UP:
                    $number = round($number, 0, PHP_ROUND_HALF_UP);
                    break;
                ...
            }

            $number /= $roundingCoef;
        }

        return $number;
    }

Which means,

  • If 1 is submitted with scale null, an int will be set.
  • If 1 is submitted with scale 0, a float will be set.
  • If 1 is submitted with scale 1, a float will be set.
  • If 1.0 is submitted with scale null, a float will be set.
  • If 1.0 is submitted with scale 0, a float will be set.
  • If 1.0 is submitted with scale 1, a float will be set.

It's the same for IntegerToLocalizedStringTransformer since it extends NumberToLocalizedStringTransformer with scale 0.

Possible Solution
Update the private round function to cast the value to int if the scale is 0.
@VincentLanglet VincentLanglet mentioned this issue Feb 18, 2020
Merged
@xabbuh xabbuh added the Form label Feb 19, 2020
@fabpot fabpot closed this as completed Feb 29, 2020
fabpot added a commit that referenced this issue Feb 29, 2020
@fabpot
bug #35781 [Form] NumberToLocalizedStringTransformer return int if sc…
34c2e96 
…ale = 0 (VincentLanglet)

This PR was merged into the 3.4 branch.

Discussion
----------

[Form] NumberToLocalizedStringTransformer return int if scale = 0

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #35775
| License       | MIT
<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Never break backward compatibility (see https://symfony.com/bc).
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too.)
 - Features and deprecations must be submitted against branch master.
-->

Commits
-------

2993fc9 Return int if scale = 0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Form Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@fabpot @xabbuh @VincentLanglet @carsonbot
0