[Form] Fixed SearchAndRenderBlockNode to really ignore empty labels

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

@@ -33,43 +33,71 @@ class SearchAndRenderBlockNode extends \Twig_Node_Expression_Function
 
             if (isset($arguments[1])) {
                 if ('label' === $blockNameSuffix) {
-                    // The "label" function expects the label in the second argument.
+                    // The "label" function expects the label in the second and
-                    // The array of variables is given in the third argument
+                    // the variables in the third argument
-                    $lineno = $arguments[1]->getLine();
+                    $label = $arguments[1];
-                    $variables = new \Twig_Node_Expression_Array(array(), $lineno);
+                    $variables = isset($arguments[2]) ? $arguments[2] : null;
-                    $givenVariables = isset($arguments[2]) ? $arguments[2] : $variables;
+                    $lineno = $label->getLine();
-                    $labelKey = new \Twig_Node_Expression_Constant('label', $lineno);
-                    $found = false;
 
-                    // If the label is listed in the variables, the label given
+                    if ($label instanceof \Twig_Node_Expression_Constant) {
-                    // in the arguments should take precedence in the following form:
+                        // If the label argument is given as a constant, we can either
-                    // labelInArgs|default(labelInAttr)
+                        // strip it away if it is empty, or integrate it into the array
-                    foreach ($givenVariables->getKeyValuePairs() as $pair) {
+                        // of variables at compile time.
-                        if ((string) $labelKey === (string) $pair['key']) {
+                        $labelIsExpression = false;
-                            $pair['value'] = new \Twig_Node_Expression_Filter_Default(
+
-                                $arguments[1],
+                        // Only insert the label into the array if it is not empty
-                                new \Twig_Node_Expression_Constant('default', $lineno),
+                        if (!twig_test_empty($label->getAttribute('value'))) {
-                                new \Twig_Node(array($pair['value']), array(), $lineno),
+                            $originalVariables = $variables;
-                                $lineno
+                            $variables = new \Twig_Node_Expression_Array(array(), $lineno);
-                            );
+                            $labelKey = new \Twig_Node_Expression_Constant('label', $lineno);
-                            $found = true;
+
+                            if (null !== $originalVariables) {
+                                foreach ($originalVariables->getKeyValuePairs() as $pair) {
+                                    // Don't copy the original label attribute over if it exists
+                                    if ((string) $labelKey !== (string) $pair['key']) {
+                                        $variables->addElement($pair['value'], $pair['key']);
+                                    }
+                                }
+                            }
+
+                            // Insert the label argument into the array
+                            $variables->addElement($label, $labelKey);
                         }
-
+                    } else {
-                        $variables->addElement($pair['value'], $pair['key']);
+                        // The label argument is not a constant, but some kind of
-                    }
+                        // expression. This expression needs to be evaluated at runtime.
-
+                        // Depending on the result (whether it is null or not), the
-                    // If the label does not exist in the variables, simply add it
+                        // label in the arguments should take precedence over the label
-                    if (!$found) {
+                        // in the attributes or not.
-                        $variables->addElement($arguments[1], $labelKey);
+                        $labelIsExpression = true;
                     }
                 } else {
                     // All other functions than "label" expect the variables
                     // in the second argument
+                    $label = null;
                     $variables = $arguments[1];
+                    $labelIsExpression = false;
                 }
 
-                $compiler->raw(', ');
+                if (null !== $variables || $labelIsExpression) {
-                $compiler->subcompile($variables);
+                    $compiler->raw(', ');
+
+                    if (null !== $variables) {
+                        $compiler->subcompile($variables);
+                    }
+
+                    if ($labelIsExpression) {
+                        if (null !== $variables) {
+                            $compiler->raw(' + ');
+                        }
+
+                        // Check at runtime whether the label is empty.
+                        // If not, add it to the array at runtime.
+                        $compiler->raw('(twig_test_empty($_label_ = ');
+                        $compiler->subcompile($label);
+                        $compiler->raw(') ? array() : array("label" => $_label_))');
+                    }
+                }
             }
         }
 

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

@@ -98,9 +98,33 @@ class SearchAndRenderBlockNodeTest extends TestCase
 
         $compiler = new \Twig_Compiler(new \Twig_Environment());
 
+        // "label" => null must not be included in the output!
+        // Otherwise the default label is overwritten with null.
         $this->assertEquals(
             sprintf(
-                '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("label" => null))',
+                '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\')',
+                $this->getVariableGetter('form')
+            ),
+            trim($compiler->compile($node)->getSource())
+        );
+    }
+
+    public function testCompileLabelWithEmptyStringLabel()
+    {
+        $arguments = new \Twig_Node(array(
+            new \Twig_Node_Expression_Name('form', 0),
+            new \Twig_Node_Expression_Constant('', 0),
+        ));
+
+        $node = new SearchAndRenderBlockNode('form_label', $arguments, 0);
+
+        $compiler = new \Twig_Compiler(new \Twig_Environment());
+
+        // "label" => null must not be included in the output!
+        // Otherwise the default label is overwritten with null.
+        $this->assertEquals(
+            sprintf(
+                '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\')',
                 $this->getVariableGetter('form')
             ),
             trim($compiler->compile($node)->getSource())
@@ -141,9 +165,12 @@ class SearchAndRenderBlockNodeTest extends TestCase
 
         $compiler = new \Twig_Compiler(new \Twig_Environment());
 
+        // "label" => null must not be included in the output!
+        // Otherwise the default label is overwritten with null.
+        // https://github.com/symfony/symfony/issues/5029
         $this->assertEquals(
             sprintf(
-                '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("foo" => "bar", "label" => null))',
+                '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("foo" => "bar"))',
                 $this->getVariableGetter('form')
             ),
             trim($compiler->compile($node)->getSource())
@@ -169,7 +196,75 @@ class SearchAndRenderBlockNodeTest extends TestCase
 
         $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->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("foo" => "bar", "label" => "value in argument"))',
+                $this->getVariableGetter('form')
+            ),
+            trim($compiler->compile($node)->getSource())
+        );
+    }
+
+    public function testCompileLabelWithLabelThatEvaluatesToNull()
+    {
+        $arguments = new \Twig_Node(array(
+            new \Twig_Node_Expression_Name('form', 0),
+            new \Twig_Node_Expression_Conditional(
+                // if
+                new \Twig_Node_Expression_Constant(true, 0),
+                // then
+                new \Twig_Node_Expression_Constant(null, 0),
+                // else
+                new \Twig_Node_Expression_Constant(null, 0),
+                0
+            ),
+        ));
+
+        $node = new SearchAndRenderBlockNode('form_label', $arguments, 0);
+
+        $compiler = new \Twig_Compiler(new \Twig_Environment());
+
+        // "label" => null must not be included in the output!
+        // Otherwise the default label is overwritten with null.
+        // https://github.com/symfony/symfony/issues/5029
+        $this->assertEquals(
+            sprintf(
+                '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', (twig_test_empty($_label_ = ((true) ? (null) : (null))) ? array() : array("label" => $_label_)))',
+                $this->getVariableGetter('form')
+            ),
+            trim($compiler->compile($node)->getSource())
+        );
+    }
+
+    public function testCompileLabelWithLabelThatEvaluatesToNullAndAttributes()
+    {
+        $arguments = new \Twig_Node(array(
+            new \Twig_Node_Expression_Name('form', 0),
+            new \Twig_Node_Expression_Conditional(
+                // if
+                new \Twig_Node_Expression_Constant(true, 0),
+                // then
+                new \Twig_Node_Expression_Constant(null, 0),
+                // else
+                new \Twig_Node_Expression_Constant(null, 0),
+                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());
+
+        // "label" => null must not be included in the output!
+        // Otherwise the default label is overwritten with null.
+        // https://github.com/symfony/symfony/issues/5029
+        $this->assertEquals(
+            sprintf(
+                '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("foo" => "bar", "label" => "value in attributes") + (twig_test_empty($_label_ = ((true) ? (null) : (null))) ? array() : array("label" => $_label_)))',
                 $this->getVariableGetter('form')
             ),
             trim($compiler->compile($node)->getSource())