[Form] Fixed variable passing from outer to inner blocks of the same FormView instance

src/Symfony/Bridge/Twig/Node/SearchAndRenderBlockNode.php

@@ -32,22 +32,44 @@ class SearchAndRenderBlockNode extends \Twig_Node_Expression_Function
             $compiler->raw(', \'' . $blockNameSuffix . '\'');
 
             if (isset($arguments[1])) {
-                $compiler->raw(', ');
-
-                // The "label" function allows one extra argument here, the label
                 if ('label' === $blockNameSuffix) {
-                    if (isset($arguments[2])) {
+                    // The "label" function expects the label in the second argument.
-                        $compiler->subcompile($arguments[2]);
+                    // The array of variables is given in the third argument
-                        $compiler->raw(' + ');
+                    $lineno = $arguments[1]->getLine();
+                    $variables = new \Twig_Node_Expression_Array(array(), $lineno);
+                    $givenVariables = isset($arguments[2]) ? $arguments[2] : $variables;
+                    $labelKey = new \Twig_Node_Expression_Constant('label', $lineno);
+                    $found = false;
+
+                    // If the label is listed in the variables, the label given
+                    // in the arguments should take precedence in the following form:
+                    // labelInArgs|default(labelInAttr)
+                    foreach ($givenVariables->getKeyValuePairs() as $pair) {
+                        if ((string) $labelKey === (string) $pair['key']) {
+                            $pair['value'] = new \Twig_Node_Expression_Filter_Default(
+                                $arguments[1],
+                                new \Twig_Node_Expression_Constant('default', $lineno),
+                                new \Twig_Node(array($pair['value']), array(), $lineno),
+                                $lineno
+                            );
+                            $found = true;
+                        }
+
+                        $variables->addElement($pair['value'], $pair['key']);
                     }
 
-                    // Add the label to the variable array
+                    // If the label does not exist in the variables, simply add it
-                    $compiler->raw('array(\'label\' => ');
+                    if (!$found) {
-                    $compiler->subcompile($arguments[1]);
+                        $variables->addElement($arguments[1], $labelKey);
-                    $compiler->raw(')');
+                    }
                 } else {
-                    $compiler->subcompile($arguments[1]);
+                    // All other functions than "label" expect the variables
+                    // in the second argument
+                    $variables = $arguments[1];
                 }
+
+                $compiler->raw(', ');
+                $compiler->subcompile($variables);
             }
         }
 

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

@@ -249,7 +249,7 @@
 {% block form_row %}
 {% spaceless %}
     <div>
-        {{ form_label(form, label|default(null)) }}
+        {{ form_label(form) }}
         {{ form_errors(form) }}
         {{ form_widget(form) }}
     </div>

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

@@ -4,7 +4,7 @@
 {% spaceless %}
     <tr>
         <td>
-            {{ form_label(form, label|default(null)) }}
+            {{ form_label(form) }}
         </td>
         <td>
             {{ form_errors(form) }}

src/Symfony/Bridge/Twig/Tests/Node/SearchAndRenderBlockNodeTest.php

@@ -0,0 +1,187 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bridge\Twig\Tests\Node;
+
+use Symfony\Bridge\Twig\Tests\TestCase;
+use Symfony\Bridge\Twig\Node\SearchAndRenderBlockNode;
+
+class SearchAndRenderBlockNodeTest extends TestCase
+{
+    protected function setUp()
+    {
+        parent::setUp();
+
+        if (version_compare(\Twig_Environment::VERSION, '1.5.0', '<')) {
+            $this->markTestSkipped('Requires Twig version to be at least 1.5.0.');
+        }
+    }
+
+    public function testCompileWidget()
+    {
+        $arguments = new \Twig_Node(array(
+            new \Twig_Node_Expression_Name('form', 0),
+        ));
+
+        $node = new SearchAndRenderBlockNode('form_widget', $arguments, 0);
+
+        $compiler = new \Twig_Compiler(new \Twig_Environment());
+
+        $this->assertEquals(
+            sprintf(
+                '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'widget\')',
+                $this->getVariableGetter('form')
+             ),
+            trim($compiler->compile($node)->getSource())
+        );
+    }
+
+    public function testCompileWidgetWithVariables()
+    {
+        $arguments = new \Twig_Node(array(
+            new \Twig_Node_Expression_Name('form', 0),
+            new \Twig_Node_Expression_Array(array(
+                new \Twig_Node_Expression_Constant('foo', 0),
+                new \Twig_Node_Expression_Constant('bar', 0),
+            ), 0),
+        ));
+
+        $node = new SearchAndRenderBlockNode('form_widget', $arguments, 0);
+
+        $compiler = new \Twig_Compiler(new \Twig_Environment());
+
+        $this->assertEquals(
+            sprintf(
+                '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'widget\', array("foo" => "bar"))',
+                $this->getVariableGetter('form')
+            ),
+            trim($compiler->compile($node)->getSource())
+        );
+    }
+
+    public function testCompileLabelWithLabel()
+    {
+        $arguments = new \Twig_Node(array(
+            new \Twig_Node_Expression_Name('form', 0),
+            new \Twig_Node_Expression_Constant('my label', 0),
+        ));
+
+        $node = new SearchAndRenderBlockNode('form_label', $arguments, 0);
+
+        $compiler = new \Twig_Compiler(new \Twig_Environment());
+
+        $this->assertEquals(
+            sprintf(
+                '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("label" => "my label"))',
+                $this->getVariableGetter('form')
+            ),
+            trim($compiler->compile($node)->getSource())
+        );
+    }
+
+    public function testCompileLabelWithNullLabel()
+    {
+        $arguments = new \Twig_Node(array(
+            new \Twig_Node_Expression_Name('form', 0),
+            new \Twig_Node_Expression_Constant(null, 0),
+        ));
+
+        $node = new SearchAndRenderBlockNode('form_label', $arguments, 0);
+
+        $compiler = new \Twig_Compiler(new \Twig_Environment());
+
+        $this->assertEquals(
+            sprintf(
+                '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("label" => null))',
+                $this->getVariableGetter('form')
+            ),
+            trim($compiler->compile($node)->getSource())
+        );
+    }
+
+    public function testCompileLabelWithDefaultLabel()
+    {
+        $arguments = new \Twig_Node(array(
+            new \Twig_Node_Expression_Name('form', 0),
+        ));
+
+        $node = new SearchAndRenderBlockNode('form_label', $arguments, 0);
+
+        $compiler = new \Twig_Compiler(new \Twig_Environment());
+
+        $this->assertEquals(
+            sprintf(
+                '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\')',
+                $this->getVariableGetter('form')
+            ),
+            trim($compiler->compile($node)->getSource())
+        );
+    }
+
+    public function testCompileLabelWithAttributes()
+    {
+        $arguments = new \Twig_Node(array(
+            new \Twig_Node_Expression_Name('form', 0),
+            new \Twig_Node_Expression_Constant(null, 0),
+            new \Twig_Node_Expression_Array(array(
+                new \Twig_Node_Expression_Constant('foo', 0),
+                new \Twig_Node_Expression_Constant('bar', 0),
+            ), 0),
+        ));
+
+        $node = new SearchAndRenderBlockNode('form_label', $arguments, 0);
+
+        $compiler = new \Twig_Compiler(new \Twig_Environment());
+
+        $this->assertEquals(
+            sprintf(
+                '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("foo" => "bar", "label" => null))',
+                $this->getVariableGetter('form')
+            ),
+            trim($compiler->compile($node)->getSource())
+        );
+    }
+
+    public function testCompileLabelWithLabelAndAttributes()
+    {
+        $arguments = new \Twig_Node(array(
+            new \Twig_Node_Expression_Name('form', 0),
+            new \Twig_Node_Expression_Constant('value in argument', 0),
+            new \Twig_Node_Expression_Array(array(
+                new \Twig_Node_Expression_Constant('foo', 0),
+                new \Twig_Node_Expression_Constant('bar', 0),
+                new \Twig_Node_Expression_Constant('label', 0),
+                new \Twig_Node_Expression_Constant('value in attributes', 0),
+            ), 0),
+        ));
+
+        $node = new SearchAndRenderBlockNode('form_label', $arguments, 0);
+
+        $compiler = new \Twig_Compiler(new \Twig_Environment());
+
+        $this->assertEquals(
+            sprintf(
+                '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("foo" => "bar", "label" => _twig_default_filter("value in argument", "value in attributes")))',
+                $this->getVariableGetter('form')
+            ),
+            trim($compiler->compile($node)->getSource())
+        );
+    }
+
+    protected function getVariableGetter($name)
+    {
+        if (version_compare(phpversion(), '5.4.0RC1', '>=')) {
+            return sprintf('(isset($context["%s"]) ? $context["%s"] : null)', $name, $name);
+        }
+
+        return sprintf('$this->getContext($context, "%s")', $name);
+    }
+}

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

@@ -1,5 +1,5 @@
 <div>
-    <?php echo $view['form']->label($form, isset($label) ? $label : null) ?>
+    <?php echo $view['form']->label($form) ?>
     <?php echo $view['form']->errors($form) ?>
     <?php echo $view['form']->widget($form) ?>
 </div>

src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_row.html.php

@@ -1,6 +1,6 @@
 <tr>
     <td>
-        <?php echo $view['form']->label($form, isset($label) ? $label : null) ?>
+        <?php echo $view['form']->label($form) ?>
     </td>
     <td>
         <?php echo $view['form']->errors($form) ?>

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

@@ -125,7 +125,7 @@ class FormHelper extends Helper
      */
     public function label(FormView $view, $label = null, array $variables = array())
     {
-        if ($label !== null) {
+        if (null !== $label) {
             $variables += array('label' => $label);
         }
 

src/Symfony/Component/Form/FormRenderer.php

@@ -22,6 +22,8 @@ use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface;
  */
 class FormRenderer implements FormRendererInterface
 {
+    const CACHE_KEY_VAR = 'full_block_name';
+
     /**
      * @var FormRendererEngineInterface
      */
@@ -42,11 +44,6 @@ class FormRenderer implements FormRendererInterface
      */
     private $hierarchyLevelMap = array();
 
-    /**
-     * @var array
-     */
-    private $variableMap = array();
-
     /**
      * @var array
      */
@@ -95,7 +92,8 @@ class FormRenderer implements FormRendererInterface
             throw new FormException('This method should only be called while rendering a form element.');
         }
 
-        $scopeVariables = end($this->variableStack);
+        $viewCacheKey = $view->vars[self::CACHE_KEY_VAR];
+        $scopeVariables = end($this->variableStack[$viewCacheKey]);
 
         $resource = $this->engine->getResourceForBlockName($view, $blockName);
 
@@ -117,13 +115,13 @@ class FormRenderer implements FormRendererInterface
         // cannot be overwritten
         $variables = array_replace($scopeVariables, $variables);
 
-        $this->variableStack[] = $variables;
+        $this->variableStack[$viewCacheKey][] = $variables;
 
         // Do the rendering
         $html = $this->engine->renderBlock($view, $resource, $blockName, $variables);
 
         // Clear the stack
-        array_pop($this->variableStack);
+        array_pop($this->variableStack[$viewCacheKey]);
 
         return $html;
     }
@@ -140,7 +138,8 @@ class FormRenderer implements FormRendererInterface
         }
 
         // The cache key for storing the variables and types
-        $mapKey = $uniqueBlockName = $view->vars['full_block_name'] . '_' . $blockNameSuffix;
+        $viewCacheKey = $view->vars[self::CACHE_KEY_VAR];
+        $viewAndSuffixCacheKey = $viewCacheKey . $blockNameSuffix;
 
         // In templates, we have to deal with two kinds of block hierarchies:
         //
@@ -169,7 +168,7 @@ class FormRenderer implements FormRendererInterface
         // widget() function again to render the block for the parent type.
         //
         // The second kind is implemented in the following blocks.
-        if (!isset($this->blockNameHierarchyMap[$mapKey])) {
+        if (!isset($this->blockNameHierarchyMap[$viewAndSuffixCacheKey])) {
             // INITIAL CALL
             // Calculate the hierarchy of template blocks and start on
             // the bottom level of the hierarchy (= "_<id>_<section>" block)
@@ -177,21 +176,33 @@ class FormRenderer implements FormRendererInterface
             foreach ($view->vars['types'] as $type) {
                 $blockNameHierarchy[] = $type . '_' . $blockNameSuffix;
             }
-            $blockNameHierarchy[] = $uniqueBlockName;
+            $blockNameHierarchy[] = $view->vars['full_block_name'] . '_' . $blockNameSuffix;
             $hierarchyLevel = count($blockNameHierarchy) - 1;
 
-            // The default variable scope contains all view variables, merged with
+            $hierarchyInit = true;
-            // the variables passed explicitly to the helper
-            $scopeVariables = $view->vars;
         } else {
             // RECURSIVE CALL
             // If a block recursively calls renderSection() again, resume rendering
             // using the parent type in the hierarchy.
-            $blockNameHierarchy = $this->blockNameHierarchyMap[$mapKey];
+            $blockNameHierarchy = $this->blockNameHierarchyMap[$viewAndSuffixCacheKey];
-            $hierarchyLevel = $this->hierarchyLevelMap[$mapKey] - 1;
+            $hierarchyLevel = $this->hierarchyLevelMap[$viewAndSuffixCacheKey] - 1;
 
+            $hierarchyInit = false;
+        }
+
+        // The variables are cached globally for a view (instead of for the
+        // current suffix)
+        if (!isset($this->variableStack[$viewCacheKey])) {
+            // The default variable scope contains all view variables, merged with
+            // the variables passed explicitly to the helper
+            $scopeVariables = $view->vars;
+
+            $varInit = true;
+        } else {
             // Reuse the current scope and merge it with the explicitly passed variables
-            $scopeVariables = $this->variableMap[$mapKey];
+            $scopeVariables = end($this->variableStack[$viewCacheKey]);
+
+            $varInit = false;
         }
 
         // Load the resource where this block can be found
@@ -235,28 +246,29 @@ class FormRenderer implements FormRendererInterface
         // We need to store these values in maps (associative arrays) because within a
         // call to widget() another call to widget() can be made, but for a different view
         // object. These nested calls should not override each other.
-        $this->blockNameHierarchyMap[$mapKey] = $blockNameHierarchy;
+        $this->blockNameHierarchyMap[$viewAndSuffixCacheKey] = $blockNameHierarchy;
-        $this->hierarchyLevelMap[$mapKey] = $hierarchyLevel;
+        $this->hierarchyLevelMap[$viewAndSuffixCacheKey] = $hierarchyLevel;
-        $this->variableMap[$mapKey] = $variables;
 
-        // We also need to store the view and the variables so that we can render custom
+        // We also need to store the variables for the view so that we can render other
-        // blocks with renderBlock() using the same themes and variables as in the outer
+        // blocks for the same view using the same variables as in the outer block.
-        // block.
+        $this->variableStack[$viewCacheKey][] = $variables;
-        //
-        // A stack is sufficient for this purpose, because renderBlock() always accesses
-        // the immediate next outer scope, which is always stored at the end of the stack.
-        $this->variableStack[] = $variables;
 
         // Do the rendering
         $html = $this->engine->renderBlock($view, $resource, $blockName, $variables);
 
         // Clear the stack
-        array_pop($this->variableStack);
+        array_pop($this->variableStack[$viewCacheKey]);
 
-        // Clear the maps
+        // Clear the caches if they were filled for the first time within
-        unset($this->blockNameHierarchyMap[$mapKey]);
+        // this function call
-        unset($this->hierarchyLevelMap[$mapKey]);
+        if ($hierarchyInit) {
-        unset($this->variableMap[$mapKey]);
+            unset($this->blockNameHierarchyMap[$viewAndSuffixCacheKey]);
+            unset($this->hierarchyLevelMap[$viewAndSuffixCacheKey]);
+        }
+
+        if ($varInit) {
+            unset($this->variableStack[$viewCacheKey]);
+        }
 
         if ($renderOnlyOnce) {
             $view->setRendered();

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

@@ -38,13 +38,17 @@ abstract class AbstractDivLayoutTest extends AbstractLayoutTest
     public function testRowOverrideVariables()
     {
         $view = $this->factory->createNamed('name', 'text')->createView();
-        $html = $this->renderRow($view, array('label' => 'foo&bar'));
+        $html = $this->renderRow($view, array(
+            'attr' => array('class' => 'my&class'),
+            'label' => 'foo&bar',
+            'label_attr' => array('class' => 'my&label&class'),
+        ));
 
         $this->assertMatchesXpath($html,
 '/div
     [
-        ./label[@for="name"][.="[trans]foo&bar[/trans]"]
+        ./label[@for="name"][@class="my&label&class required"][.="[trans]foo&bar[/trans]"]
-        /following-sibling::input[@id="name"]
+        /following-sibling::input[@id="name"][@class="my&class"]
     ]
 '
         );

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

@@ -225,7 +225,7 @@ abstract class AbstractLayoutTest extends FormIntegrationTestCase
         );
     }
 
-    public function testLabelWithCustomOptionsPassedDirectly()
+    public function testLabelWithCustomAttributesPassedDirectly()
     {
         $form = $this->factory->createNamed('name', 'text');
         $html = $this->renderLabel($form->createView(), null, array(
@@ -242,7 +242,7 @@ abstract class AbstractLayoutTest extends FormIntegrationTestCase
         );
     }
 
-    public function testLabelWithCustomTextAndCustomOptionsPassedDirectly()
+    public function testLabelWithCustomTextAndCustomAttributesPassedDirectly()
     {
         $form = $this->factory->createNamed('name', 'text');
         $html = $this->renderLabel($form->createView(), 'Custom label', array(
@@ -260,6 +260,27 @@ abstract class AbstractLayoutTest extends FormIntegrationTestCase
         );
     }
 
+    // https://github.com/symfony/symfony/issues/5029
+    public function testLabelWithCustomTextAsOptionAndCustomAttributesPassedDirectly()
+    {
+        $form = $this->factory->createNamed('name', 'text', null, array(
+            'label' => 'Custom label',
+        ));
+        $html = $this->renderLabel($form->createView(), null, array(
+            'label_attr' => array(
+                'class' => 'my&class'
+            ),
+        ));
+
+        $this->assertMatchesXpath($html,
+            '/label
+    [@for="name"]
+    [@class="my&class required"]
+    [.="[trans]Custom label[/trans]"]
+'
+        );
+    }
+
     public function testErrors()
     {
         $form = $this->factory->createNamed('name', 'text');