symfony · fabpot · Jul 21, 2012 · Jul 16, 2012 · Jul 16, 2012 · Jul 16, 2012
diff --git a/UPGRADE-2.1.md b/UPGRADE-2.1.md
@@ -224,30 +224,23 @@
     `buildViewBottomUp()` in `FormTypeInterface` and `FormTypeExtensionInterface`.
     Furthermore, `buildViewBottomUp()` was renamed to `finishView()`. At last,
     all methods in these types now receive instances of `FormBuilderInterface`
-    and `FormViewInterface` where they received instances of `FormBuilder` and
-    `FormView` before. You need to change the method signatures in your
-     form types and extensions as shown below.
+    where they received instances of `FormBuilder` before. You need to change the
+    method signatures in your form types and extensions as shown below.
 
     Before:
 
     ```
     use Symfony\Component\Form\FormBuilder;
-    use Symfony\Component\Form\FormView;
 
     public function buildForm(FormBuilder $builder, array $options)
-    public function buildView(FormView $view, FormInterface $form)
-    public function buildViewBottomUp(FormView $view, FormInterface $form)
     ```
 
     After:
 
     ```
     use Symfony\Component\Form\FormBuilderInterface;
-    use Symfony\Component\Form\FormViewInterface;
 
     public function buildForm(FormBuilderInterface $builder, array $options)
-    public function buildView(FormViewInterface $view, FormInterface $form, array $options)
-    public function finishView(FormViewInterface $view, FormInterface $form, array $options)
     ```
 
   * The method `createBuilder` was removed from `FormTypeInterface` for performance
@@ -383,41 +376,6 @@
     If address is an object in this case, the code given in "Before"
     works without changes.
 
-  * The methods in class `FormView` were renamed to match the naming used in
-    `Form` and `FormBuilder`. The following list shows the old names on the
-    left and the new names on the right:
-
-      * `set`: `setVar`
-      * `has`: `hasVar`
-      * `get`: `getVar`
-      * `all`: `getVars`
-      * `addChild`: `add`
-      * `getChild`: `get`
-      * `getChildren`: `all`
-      * `removeChild`: `remove`
-      * `hasChild`: `has`
-
-    The new method `addVars` was added to make the definition of multiple
-    variables at once more convenient.
-
-    The method `hasChildren` was deprecated. You should use `count` instead.
-
-    Before:
-
-    ```
-    $view->set('help', 'A text longer than six characters');
-    $view->set('error_class', 'max_length_error');
-    ```
-
-    After:
-
-    ```
-    $view->addVars(array(
-        'help'        => 'A text longer than six characters',
-        'error_class' => 'max_length_error',
-    ));
-    ```
-
   * Form and field names must now start with a letter, digit or underscore
     and only contain letters, digits, underscores, hyphens and colons.
 
@@ -1069,6 +1027,67 @@
     <?php echo $view['form']->block('widget_attributes') ?>
     ```
 
+  * The following methods in class `FormView` were deprecated and will be
+    removed in Symfony 2.3:
+
+      * `set`
+      * `has`
+      * `get`
+      * `all`
+      * `getVars`
+      * `addChild`
+      * `getChild`
+      * `getChildren`
+      * `removeChild`
+      * `hasChild`
+      * `hasChildren`
+      * `getParent`
+      * `hasParent`
+      * `setParent`
+
+    You should access the public properties `vars`, `children` and `parent`
+    instead.
+
+    Before:
+
+    ```
+    $view->set('help', 'A text longer than six characters');
+    $view->set('error_class', 'max_length_error');
+    ```
+
+    After:
+
+    ```
+    $view->vars = array_replace($view->vars, array(
+        'help'        => 'A text longer than six characters',
+        'error_class' => 'max_length_error',
+    ));
+    ```
+
+    Before:
+
+    ```
+    echo $view->get('error_class');
+    ```
+
+    After:
+
+    ```
+    echo $view->vars['error_class'];
+    ```
+
+    Before:
+
+    ```
+    if ($view->hasChildren()) { ...
+    ```
+
+    After:
+
+    ```
+    if (count($view->children)) { ...
+    ```
+
 ### Validator
 
   * The methods `setMessage()`, `getMessageTemplate()` and

diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php
@@ -124,7 +124,7 @@ public function testSetDataToUninitializedEntityWithNonRequired()
             'property' => 'name'
         ));
 
-        $this->assertEquals(array(1 => new ChoiceView('1', 'Foo'), 2 => new ChoiceView('2', 'Bar')), $field->createView()->getVar('choices'));
+        $this->assertEquals(array(1 => new ChoiceView('1', 'Foo'), 2 => new ChoiceView('2', 'Bar')), $field->createView()->vars['choices']);
     }
 
     public function testSetDataToUninitializedEntityWithNonRequiredToString()
@@ -140,7 +140,7 @@ public function testSetDataToUninitializedEntityWithNonRequiredToString()
             'required' => false,
         ));
 
-        $this->assertEquals(array(1 => new ChoiceView('1', 'Foo'), 2 => new ChoiceView('2', 'Bar')), $field->createView()->getVar('choices'));
+        $this->assertEquals(array(1 => new ChoiceView('1', 'Foo'), 2 => new ChoiceView('2', 'Bar')), $field->createView()->vars['choices']);
     }
 
     public function testSetDataToUninitializedEntityWithNonRequiredQueryBuilder()
@@ -159,7 +159,7 @@ public function testSetDataToUninitializedEntityWithNonRequiredQueryBuilder()
             'query_builder' => $qb
         ));
 
-        $this->assertEquals(array(1 => new ChoiceView('1', 'Foo'), 2 => new ChoiceView('2', 'Bar')), $field->createView()->getVar('choices'));
+        $this->assertEquals(array(1 => new ChoiceView('1', 'Foo'), 2 => new ChoiceView('2', 'Bar')), $field->createView()->vars['choices']);
     }
 
     /**
@@ -503,7 +503,7 @@ public function testOverrideChoices()
 
         $field->bind('2');
 
-        $this->assertEquals(array(1 => new ChoiceView('1', 'Foo'), 2 => new ChoiceView('2', 'Bar')), $field->createView()->getVar('choices'));
+        $this->assertEquals(array(1 => new ChoiceView('1', 'Foo'), 2 => new ChoiceView('2', 'Bar')), $field->createView()->vars['choices']);
         $this->assertTrue($field->isSynchronized());
         $this->assertSame($entity2, $field->getData());
         $this->assertSame('2', $field->getClientData());
@@ -533,7 +533,7 @@ public function testGroupByChoices()
             'Group1' => array(1 => new ChoiceView('1', 'Foo'), 2 => new ChoiceView('2', 'Bar')),
             'Group2' => array(3 => new ChoiceView('3', 'Baz')),
             '4' => new ChoiceView('4', 'Boo!')
-        ), $field->createView()->getVar('choices'));
+        ), $field->createView()->vars['choices']);
     }
 
     public function testDisallowChoicesThatAreNotIncluded_choicesSingleIdentifier()

diff --git a/src/Symfony/Bridge/Twig/Extension/FormExtension.php b/src/Symfony/Bridge/Twig/Extension/FormExtension.php
@@ -13,7 +13,7 @@
 
 use Symfony\Bridge\Twig\TokenParser\FormThemeTokenParser;
 use Symfony\Bridge\Twig\Form\TwigRendererInterface;
-use Symfony\Component\Form\FormViewInterface;
+use Symfony\Component\Form\Extension\Core\View\ChoiceView;
 use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface;
 
 /**
@@ -62,15 +62,13 @@ public function getTokenParsers()
     public function getFunctions()
     {
         return array(
-            'form_enctype'             => new \Twig_Function_Method($this, 'renderer->renderEnctype', array('is_safe' => array('html'))),
-            'form_widget'              => new \Twig_Function_Method($this, 'renderer->renderWidget', array('is_safe' => array('html'))),
-            'form_errors'              => new \Twig_Function_Method($this, 'renderer->renderErrors', array('is_safe' => array('html'))),
-            'form_label'               => new \Twig_Function_Method($this, 'renderer->renderLabel', array('is_safe' => array('html'))),
-            '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'))),
+            'form_enctype' => new \Twig_Function_Method($this, 'renderer->renderEnctype', array('is_safe' => array('html'))),
+            'form_widget'  => new \Twig_Function_Method($this, 'renderer->renderWidget', array('is_safe' => array('html'))),
+            'form_errors'  => new \Twig_Function_Method($this, 'renderer->renderErrors', array('is_safe' => array('html'))),
+            'form_label'   => new \Twig_Function_Method($this, 'renderer->renderLabel', array('is_safe' => array('html'))),
+            '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'),
         );
     }
 
@@ -84,6 +82,48 @@ public function getFilters()
         );
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function getTests()
+    {
+        return array(
+            'selectedchoice' => new \Twig_Test_Method($this, 'isSelectedChoice'),
+        );
+    }
+
+    /**
+     * Returns whether a choice is selected for a given form value.
+     *
+     * Unfortunately Twig does not support an efficient way to execute the
+     * "is_selected" closure passed to the template by ChoiceType. It is faster
+     * to implement the logic here (around 65ms for a specific form).
+     *
+     * Directly implementing the logic here is also faster than doing so in
+     * ChoiceView (around 30ms).
+     *
+     * The worst option tested so far is to implement the logic in ChoiceView
+     * and access the ChoiceView method directly in the template. Doing so is
+     * around 220ms slower than doing the method call here in the filter. Twig
+     * seems to be much more efficient at executing filters than at executing
+     * methods of an object.
+     *
+     * @param ChoiceView   $choice        The choice to check.
+     * @param string|array $selectedValue The selected value to compare.
+     *
+     * @return Boolean Whether the choice is selected.
+     *
+     * @see ChoiceView::isSelected()
+     */
+    public function isSelectedChoice(ChoiceView $choice, $selectedValue)
+    {
+        if (is_array($selectedValue)) {
+            return false !== array_search($choice->value, $selectedValue, true);
+        }
+
+        return $choice->value === $selectedValue;
+    }
+
     /**
      * {@inheritdoc}
      */

diff --git a/src/Symfony/Bridge/Twig/Form/TwigRendererEngine.php b/src/Symfony/Bridge/Twig/Form/TwigRendererEngine.php
@@ -12,7 +12,7 @@
 namespace Symfony\Bridge\Twig\Form;
 
 use Symfony\Component\Form\AbstractRendererEngine;
-use Symfony\Component\Form\FormViewInterface;
+use Symfony\Component\Form\FormView;
 
 /**
  * @author Bernhard Schussek <bschussek@gmail.com>
@@ -40,9 +40,9 @@ public function setEnvironment(\Twig_Environment $environment)
     /**
      * {@inheritdoc}
      */
-    public function renderBlock(FormViewInterface $view, $resource, $block, array $variables = array())
+    public function renderBlock(FormView $view, $resource, $block, array $variables = array())
     {
-        $cacheKey = $view->getVar(self::CACHE_KEY_VAR);
+        $cacheKey = $view->vars[self::CACHE_KEY_VAR];
 
         $context = $this->environment->mergeGlobals($variables);
 
@@ -71,12 +71,12 @@ public function renderBlock(FormViewInterface $view, $resource, $block, array $v
      * @see getResourceForBlock()
      *
      * @param string            $cacheKey The cache key of the form view.
-     * @param FormViewInterface $view     The form view for finding the applying themes.
+     * @param FormView $view     The form view for finding the applying themes.
      * @param string            $block    The name of the block to load.
      *
      * @return Boolean True if the resource could be loaded, false otherwise.
      */
-    protected function loadResourceForBlock($cacheKey, FormViewInterface $view, $block)
+    protected function loadResourceForBlock($cacheKey, FormView $view, $block)
     {
         // The caller guarantees that $this->resources[$cacheKey][$block] is
         // not set, but it doesn't have to check whether $this->resources[$cacheKey]
@@ -105,19 +105,19 @@ protected function loadResourceForBlock($cacheKey, FormViewInterface $view, $blo
         }
 
         // Check the default themes once we reach the root view without success
-        if (!$view->hasParent()) {
+        if (!$view->parent) {
             for ($i = count($this->defaultThemes) - 1; $i >= 0; --$i) {
                 $this->loadResourcesFromTheme($cacheKey, $this->defaultThemes[$i]);
                 // CONTINUE LOADING (see doc comment)
             }
         }
 
         // Proceed with the themes of the parent view
-        if ($view->hasParent()) {
-            $parentCacheKey = $view->getParent()->getVar(self::CACHE_KEY_VAR);
+        if ($view->parent) {
+            $parentCacheKey = $view->parent->vars[self::CACHE_KEY_VAR];
 
             if (!isset($this->resources[$parentCacheKey])) {
-                $this->loadResourceForBlock($parentCacheKey, $view->getParent(), $block);
+                $this->loadResourceForBlock($parentCacheKey, $view->parent, $block);
             }
 
             // EAGER CACHE POPULATION (see doc comment)

diff --git a/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig b/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 %}
-        {% if _form_is_choice_group(choice) %}
-            <optgroup label="{{ index|trans({}, translation_domain) }}">
-                {% for nested_choice in choice %}
-                    <option value="{{ nested_choice.value }}"{% if _form_is_choice_selected(form, nested_choice) %} selected="selected"{% endif %}>{{ nested_choice.label|trans({}, translation_domain) }}</option>
-                {% endfor %}
+    {% for group_label, choice in options %}
+        {% if choice is iterable %}
+            <optgroup label="{{ group_label|trans({}, translation_domain) }}">
+                {% set options = choice %}
+                {{ block('choice_widget_options') }}
             </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 selectedchoice(value) %} selected="selected"{% endif %}>{{ choice.label|trans({}, translation_domain) }}</option>
         {% endif %}
     {% endfor %}
 {% endspaceless %}

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php
@@ -18,6 +18,7 @@
 use Symfony\Bridge\Twig\Tests\Extension\Fixtures\StubTranslator;
 use Symfony\Bridge\Twig\Tests\Extension\Fixtures\StubFilesystemLoader;
 use Symfony\Component\Form\FormView;
+use Symfony\Component\Form\Extension\Core\View\ChoiceView;
 use Symfony\Component\Form\Tests\AbstractDivLayoutTest;
 
 class FormExtensionDivLayoutTest extends AbstractDivLayoutTest
@@ -105,6 +106,39 @@ public function testThemeBlockInheritanceUsingExtend()
         );
     }
 
+    public function isSelectedChoiceProvider()
+    {
+        // The commented cases should not be necessary anymore, because the
+        // choice lists should assure that both values passed here are always
+        // strings
+        return array(
+//             array(true, 0, 0),
+            array(true, '0', '0'),
+            array(true, '1', '1'),
+//             array(true, false, 0),
+//             array(true, true, 1),
+            array(true, '', ''),
+//             array(true, null, ''),
+            array(true, '1.23', '1.23'),
+            array(true, 'foo', 'foo'),
+            array(true, 'foo10', 'foo10'),
+            array(true, 'foo', array(1, 'foo', 'foo10')),
+
+            array(false, 10, array(1, 'foo', 'foo10')),
+            array(false, 0, array(1, 'foo', 'foo10')),
+        );
+    }
+
+    /**
+     * @dataProvider isSelectedChoiceProvider
+     */
+    public function testIsChoiceSelected($expected, $choice, $value)
+    {
+        $choice = new ChoiceView($choice, $choice . ' label');
+
+        $this->assertSame($expected, $this->extension->isSelectedChoice($choice, $value));
+    }
+
     protected function renderEnctype(FormView $view)
     {
         return (string) $this->extension->renderer->renderEnctype($view);