[Form] Tweaked the generation of option tags for performance (PHP +200ms, Twig +50ms)

src/Symfony/Bridge/Twig/Extension/FormExtension.php

@@ -69,8 +69,6 @@ class FormExtension extends \Twig_Extension
             'form_row'                 => new \Twig_Function_Method($this, 'renderer->renderRow', array('is_safe' => array('html'))),
             'form_rest'                => new \Twig_Function_Method($this, 'renderer->renderRest', array('is_safe' => array('html'))),
             'csrf_token'               => new \Twig_Function_Method($this, 'renderer->renderCsrfToken'),
-            '_form_is_choice_group'    => new \Twig_Function_Method($this, 'renderer->isChoiceGroup', array('is_safe' => array('html'))),
-            '_form_is_choice_selected' => new \Twig_Function_Method($this, 'renderer->isChoiceSelected', array('is_safe' => array('html'))),
         );
     }
 
@@ -80,7 +78,9 @@ class FormExtension extends \Twig_Extension
     public function getFilters()
     {
         return array(
-            'humanize' => new \Twig_Filter_Method($this, 'renderer->humanize'),
+            'humanize'           => new \Twig_Filter_Method($this, 'renderer->humanize'),
+            'is_choice_group'    => new \Twig_Filter_Function('is_array', array('is_safe' => array('html'))),
+            'is_choice_selected' => new \Twig_Filter_Method($this, 'renderer->isChoiceSelected', array('is_safe' => array('html'))),
         );
     }
 

src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig

@@ -86,15 +86,14 @@
 
 {% block choice_widget_options %}
 {% spaceless %}
-    {% for index, choice in options %}
+    {% for group_label, choice in options %}
-        {% if _form_is_choice_group(choice) %}
+        {% if choice|is_choice_group %}
-            <optgroup label="{{ index|trans({}, translation_domain) }}">
+            <optgroup label="{{ group_label|trans({}, translation_domain) }}">
-                {% for nested_choice in choice %}
+                {% set options = choice %}
-                    <option value="{{ nested_choice.value }}"{% if _form_is_choice_selected(form, nested_choice) %} selected="selected"{% endif %}>{{ nested_choice.label|trans({}, translation_domain) }}</option>
+                {{ block('choice_widget_options') }}
-                {% endfor %}
             </optgroup>
         {% else %}
-            <option value="{{ choice.value }}"{% if _form_is_choice_selected(form, choice) %} selected="selected"{% endif %}>{{ choice.label|trans({}, translation_domain) }}</option>
+            <option value="{{ choice.value }}"{% if choice|is_choice_selected(value) %} selected="selected"{% endif %}>{{ choice.label|trans({}, translation_domain) }}</option>
         {% endif %}
     {% endfor %}
 {% endspaceless %}

src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_collapsed.html.php

@@ -4,10 +4,10 @@
 >
     <?php if (null !== $empty_value): ?><option value=""><?php echo $view->escape($view['translator']->trans($empty_value, array(), $translation_domain)) ?></option><?php endif; ?>
     <?php if (count($preferred_choices) > 0): ?>
-        <?php echo $view['form']->block('choice_widget_options', array('options' => $preferred_choices)) ?>
+        <?php echo $view['form']->block('choice_widget_options', array('choices' => $preferred_choices)) ?>
         <?php if (count($choices) > 0 && null !== $separator): ?>
             <option disabled="disabled"><?php echo $separator ?></option>
         <?php endif ?>
     <?php endif ?>
-    <?php echo $view['form']->block('choice_widget_options', array('options' => $choices)) ?>
+    <?php echo $view['form']->block('choice_widget_options', array('choices' => $choices)) ?>
 </select>

src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_options.html.php

@@ -1,11 +1,9 @@
-<?php foreach ($options as $index => $choice): ?>
+<?php foreach ($choices as $index => $choice): ?>
-    <?php if ($view['form']->isChoiceGroup($choice)): ?>
+    <?php if (is_array($choice)): ?>
         <optgroup label="<?php echo $view->escape($view['translator']->trans($index, array(), $translation_domain)) ?>">
-            <?php foreach ($choice as $nested_choice): ?>
+            <?php echo $view['form']->block('choice_widget_options', array('choices' => $choice)) ?>
-                <option value="<?php echo $view->escape($nested_choice->value) ?>"<?php if ($view['form']->isChoiceSelected($form, $nested_choice)): ?> selected="selected"<?php endif?>><?php echo $view->escape($view['translator']->trans($nested_choice->label, array(), $translation_domain)) ?></option>
-            <?php endforeach ?>
         </optgroup>
     <?php else: ?>
-        <option value="<?php echo $view->escape($choice->value) ?>"<?php if ($view['form']->isChoiceSelected($form, $choice)): ?> selected="selected"<?php endif?>><?php echo $view->escape($view['translator']->trans($choice->label, array(), $translation_domain)) ?></option>
+        <option value="<?php echo $view->escape($choice->value) ?>"<?php if ($view['form']->isChoiceSelected($choice, $value)): ?> selected="selected"<?php endif?>><?php echo $view->escape($view['translator']->trans($choice->label, array(), $translation_domain)) ?></option>
     <?php endif ?>
 <?php endforeach ?>

src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php

@@ -49,14 +49,9 @@ class FormHelper extends Helper
         return 'form';
     }
 
-    public function isChoiceGroup($label)
+    public function isChoiceSelected(ChoiceView $choice, $selectedValue)
     {
-        return $this->renderer->isChoiceGroup($label);
+        return $this->renderer->isChoiceSelected($choice, $selectedValue);
-    }
-
-    public function isChoiceSelected(FormView $view, ChoiceView $choice)
-    {
-        return $this->renderer->isChoiceSelected($view, $choice);
     }
 
     /**
@@ -176,7 +171,7 @@ class FormHelper extends Helper
      */
     public function renderBlock($block, array $variables = array())
     {
-        return $this->block($block, $variables);
+        return $this->renderer->renderBlock($block, $variables);
     }
 
     /**

src/Symfony/Component/Form/Extension/Core/ChoiceList/ChoiceList.php

@@ -240,27 +240,19 @@ class ChoiceList implements ChoiceListInterface
      *                                  view objects.
      * @param array $bucketForRemaining The bucket where to store the
      *                                  non-preferred view objects.
-     * @param array $choices          The list of choices.
+     * @param array $choices            The list of choices.
-     * @param array $labels           The labels corresponding to the choices.
+     * @param array $labels             The labels corresponding to the choices.
-     * @param array $preferredChoices The preferred choices.
+     * @param array $preferredChoices   The preferred choices.
      *
      * @throws UnexpectedTypeException If the structure of the $labels array
      *                                 does not match the structure of the
      *                                 $choices array.
      */
-    protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, $choices, $labels, array $preferredChoices)
+    protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, array $choices, array $labels, array $preferredChoices)
     {
-        if (!is_array($choices) && !$choices instanceof \Traversable) {
-            throw new UnexpectedTypeException($choices, 'array or \Traversable');
-        }
-
         // Add choices to the nested buckets
         foreach ($choices as $group => $choice) {
             if (is_array($choice)) {
-                if (!is_array($labels)) {
-                    throw new UnexpectedTypeException($labels, 'array');
-                }
-
                 // Don't do the work if the array is empty
                 if (count($choice) > 0) {
                     $this->addChoiceGroup(
@@ -292,11 +284,11 @@ class ChoiceList implements ChoiceListInterface
      *                                  view objects.
      * @param array $bucketForRemaining The bucket where to store the
      *                                  non-preferred view objects.
-     * @param array $choices          The list of choices in the group.
+     * @param array $choices            The list of choices in the group.
-     * @param array $labels           The labels corresponding to the choices in the group.
+     * @param array $labels             The labels corresponding to the choices in the group.
-     * @param array $preferredChoices The preferred choices.
+     * @param array $preferredChoices   The preferred choices.
      */
-    protected function addChoiceGroup($group, &$bucketForPreferred, &$bucketForRemaining, $choices, $labels, array $preferredChoices)
+    protected function addChoiceGroup($group, &$bucketForPreferred, &$bucketForRemaining, array $choices, array $labels, array $preferredChoices)
     {
         // If this is a choice group, create a new level in the choice
         // key hierarchy
@@ -323,13 +315,15 @@ class ChoiceList implements ChoiceListInterface
     /**
      * Adds a new choice.
      *
-     * @param array $bucketForPreferred The bucket where to store the preferred
+     * @param array  $bucketForPreferred The bucket where to store the preferred
-     *                                  view objects.
+     *                                   view objects.
-     * @param array $bucketForRemaining The bucket where to store the
+     * @param array  $bucketForRemaining The bucket where to store the
-     *                                  non-preferred view objects.
+     *                                   non-preferred view objects.
-     * @param mixed  $choice           The choice to add.
+     * @param mixed  $choice             The choice to add.
-     * @param string $label            The label for the choice.
+     * @param string $label              The label for the choice.
-     * @param array  $preferredChoices The preferred choices.
+     * @param array  $preferredChoices   The preferred choices.
+     *
+     * @throws InvalidConfigurationException If no valid value or index could be created.
      */
     protected function addChoice(&$bucketForPreferred, &$bucketForRemaining, $choice, $label, array $preferredChoices)
     {
@@ -366,8 +360,10 @@ class ChoiceList implements ChoiceListInterface
      *
      * @param mixed $choice           The choice to test.
      * @param array $preferredChoices An array of preferred choices.
+     *
+     * @return Boolean Whether the choice is preferred.
      */
-    protected function isPreferred($choice, $preferredChoices)
+    protected function isPreferred($choice, array $preferredChoices)
     {
         return false !== array_search($choice, $preferredChoices, true);
     }

src/Symfony/Component/Form/Extension/Core/ChoiceList/SimpleChoiceList.php

@@ -89,7 +89,7 @@ class SimpleChoiceList extends ChoiceList
      *
      * @see parent::addChoices
      */
-    protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, $choices, $labels, array $preferredChoices)
+    protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, array $choices, array $labels, array $preferredChoices)
     {
         // Add choices to the nested buckets
         foreach ($choices as $choice => $label) {
@@ -126,8 +126,10 @@ class SimpleChoiceList extends ChoiceList
      *
      * @param mixed $choice           The choice to test.
      * @param array $preferredChoices An array of preferred choices.
+     *
+     * @return Boolean Whether the choice is preferred.
      */
-    protected function isPreferred($choice, $preferredChoices)
+    protected function isPreferred($choice, array $preferredChoices)
     {
         // Optimize performance over the default implementation
         return isset($preferredChoices[$choice]);

src/Symfony/Component/Form/FormRenderer.php

@@ -175,24 +175,15 @@ class FormRenderer implements FormRendererInterface
     /**
      * {@inheritdoc}
      */
-    public function isChoiceGroup($choice)
+    public function isChoiceSelected(ChoiceView $choice, $selectedValue)
     {
-        return is_array($choice) || $choice instanceof \Traversable;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function isChoiceSelected(FormView $view, ChoiceView $choice)
-    {
-        $value = $view->vars['value'];
         $choiceValue = $choice->value;
 
-        if (is_array($value)) {
+        if (is_array($selectedValue)) {
-            return false !== array_search($choiceValue, $value, true);
+            return false !== array_search($choiceValue, $selectedValue, true);
         }
 
-        return $choiceValue === $value;
+        return $choiceValue === $selectedValue;
     }
 
     /**

src/Symfony/Component/Form/FormRendererInterface.php

@@ -146,24 +146,15 @@ interface FormRendererInterface
      */
     public function renderCsrfToken($intention);
 
-    /**
-     * Returns whether the given choice is a group.
-     *
-     * @param mixed $choice A choice
-     *
-     * @return Boolean Whether the choice is a group
-     */
-    public function isChoiceGroup($choice);
-
     /**
      * Returns whether the given choice is selected.
      *
-     * @param FormView $view   The view of the choice field
+     * @param ChoiceView   $choice        The choice to check.
-     * @param ChoiceView        $choice The choice to check
+     * @param string|array $selectedValue The selected value(s).
      *
      * @return Boolean Whether the choice is selected
      */
-    public function isChoiceSelected(FormView $view, ChoiceView $choice);
+    public function isChoiceSelected(ChoiceView $choice, $selectedValue);
 
     /**
      * Makes a technical name human readable.

src/Symfony/Component/Form/Tests/FormRendererTest.php

@@ -40,34 +40,6 @@ class FormRendererTest extends \PHPUnit_Framework_TestCase
         $this->renderer = new FormRenderer($this->engine, $this->csrfProvider);
     }
 
-    public function isChoiceGroupProvider()
-    {
-        return array(
-            array(false, 0),
-            array(false, '0'),
-            array(false, '1'),
-            array(false, 1),
-            array(false, ''),
-            array(false, null),
-            array(false, true),
-
-            array(true, array()),
-        );
-    }
-
-    /**
-     * @dataProvider isChoiceGroupProvider
-     */
-    public function testIsChoiceGroup($expected, $value)
-    {
-        $this->assertSame($expected, $this->renderer->isChoiceGroup($value));
-    }
-
-    public function testIsChoiceGroupPart2()
-    {
-        $this->assertTrue($this->renderer->isChoiceGroup(new \SplFixedArray(1)));
-    }
-
     public function isChoiceSelectedProvider()
     {
         // The commented cases should not be necessary anymore, because the
@@ -96,10 +68,8 @@ class FormRendererTest extends \PHPUnit_Framework_TestCase
      */
     public function testIsChoiceSelected($expected, $choice, $value)
     {
-        $view = new FormView();
-        $view->vars['value'] = $value;
         $choice = new ChoiceView($choice, $choice . ' label');
 
-        $this->assertSame($expected, $this->renderer->isChoiceSelected($view, $choice));
+        $this->assertSame($expected, $this->renderer->isChoiceSelected($choice, $value));
     }
 }