bug #20378 [Form] Fixed show float values as choice value in ChoiceType (yceruto)

This PR was merged into the 2.7 branch.

Discussion
----------

[Form] Fixed show float values as choice value in ChoiceType

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #13817
| License       | MIT
| Doc PR        | -

There is a closed [issue][1] related to this before Symfony 2.7 (when choice value still was the key from the `array`). This issue already happened in 2.7+ but now inside Symfony core.

Information
-----------

[This method][3] checks whether the given choices can be cast to strings without generating duplicates (3ab8189080 in #16705):

```php
private function castableToString(array $choices, array &$cache = array())
{
    foreach ($choices as $choice) {
        if (is_array($choice)) {
            if (!$this->castableToString($choice, $cache)) {
                return false;
            }

            continue;
        } elseif (!is_scalar($choice)) {
            return false;
        } elseif (isset($cache[$choice])) {  // <---- red breakpoint
            return false;
        }

        $cache[$choice] = true; // <---- red breakpoint
    }

    return true;
}
```

So it should to keep [scalar values (integer, float, string or boolean)][2] as choice values always (unless have duplicates values).

The Problem
-----------

But in this situation it doesn't happen:

```php
$form = $this->createFormBuilder()
    ->add('foo', ChoiceType::class, [
        'choices' => [
            'Min' => 0.5,
            'Mid' => 1.0,
            'Max' => 1.5,
        ]
    ])
    ->getForm();
```

**Output:**

```html
<select id="form_foo" name="form[foo]">
  <option value="0">Min</option>
  <option value="1">Mid</option>
  <option value="2">Max</option>
</select>
```

[**Why?**][5]

If the key of the array is a float number, it's interpreted as integer automatically:

```php
// ...

$cache[$choice] = true;

// when $choice = 0.5: $cache = [0 => true]
// when $choice = 1.0: $cache = [0 => true, 1 => true]
```

Then, when `$choice = 1.5` [this sentence][4] `isset($cache[1.5])` returns `true` because really checks `isset($cache[1])` and this key already exists, so `castableToString()` returns `false` (detected as duplicate) and the choices values are generated incrementing integers as values.

The PR's Effect
-------------

**Before:**

```html
<select id="form_foo" name="form[foo]">
  <option value="0">Min</option>
  <option value="1">Mid</option>
  <option value="2">Max</option>
</select>
```

**After:**

```html
<select id="form_foo" name="form[foo]">
  <option value="0.5">Min</option>
  <option value="1">Mid</option>
  <option value="1.5">Max</option>
</select>
```

  [1]: https://github.com/symfony/symfony/issues/13817
  [2]: http://php.net/manual/en/function.is-scalar.php
  [3]: https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php#L228
  [4]: https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php#L239
  [5]: https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php#L243

Commits
-------

3564228 [Form] Fix show float values as choices values in ChoiceType
This commit is contained in:
Fabien Potencier 2016-11-06 08:03:54 -08:00
commit 94d9bfbc1d
2 changed files with 16 additions and 3 deletions

View File

@ -236,7 +236,11 @@ class ArrayChoiceList implements ChoiceListInterface
continue;
} elseif (!is_scalar($choice)) {
return false;
} elseif (isset($cache[$choice])) {
}
$choice = false === $choice ? '0' : (string) $choice;
if (isset($cache[$choice])) {
return false;
}

View File

@ -34,12 +34,12 @@ class ArrayChoiceListTest extends AbstractChoiceListTest
protected function getChoices()
{
return array(0, 1, '1', 'a', false, true, $this->object, null);
return array(0, 1, 1.5, '1', 'a', false, true, $this->object, null);
}
protected function getValues()
{
return array('0', '1', '2', '3', '4', '5', '6', '7');
return array('0', '1', '2', '3', '4', '5', '6', '7', '8');
}
/**
@ -162,4 +162,13 @@ class ArrayChoiceListTest extends AbstractChoiceListTest
$this->assertSame(array(0 => true), $choiceList->getChoicesForValues(array('1')));
$this->assertSame(array(0 => false), $choiceList->getChoicesForValues(array('0')));
}
public function testGetChoicesForValuesWithContainingEmptyStringAndFloats()
{
$choiceList = new ArrayChoiceList(array('Empty String' => '', '1/3' => 0.3, '1/2' => 0.5));
$this->assertSame(array(0 => ''), $choiceList->getChoicesForValues(array('')));
$this->assertSame(array(0 => 0.3), $choiceList->getChoicesForValues(array('0.3')));
$this->assertSame(array(0 => 0.5), $choiceList->getChoicesForValues(array('0.5')));
}
}