fixed the attribute() function when passing a variable for the arguments

fabpot · Aug 1, 2014 · c03a7a3 · c03a7a3
1 parent 72aa82b
commit c03a7a3
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 16 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,6 @@
 * 1.16.1 (2014-XX-XX)
 
- * n/a
+ * fixed the attribute() function when passing a variable for the arguments
 
 * 1.16.0 (2014-07-05)
 

diff --git a/lib/Twig/ExpressionParser.php b/lib/Twig/ExpressionParser.php
@@ -318,7 +318,7 @@ public function getFunctionNode($name, $line)
                     throw new Twig_Error_Syntax('The "attribute" function takes at least two arguments (the variable and the attributes)', $line, $this->parser->getFilename());
                 }
 
-                return new Twig_Node_Expression_GetAttr($args->getNode(0), $args->getNode(1), count($args) > 2 ? $args->getNode(2) : new Twig_Node_Expression_Array(array(), $line), Twig_Template::ANY_CALL, $line);
+                return new Twig_Node_Expression_GetAttr($args->getNode(0), $args->getNode(1), count($args) > 2 ? $args->getNode(2) : null, Twig_Template::ANY_CALL, $line);
             default:
                 if (null !== $alias = $this->parser->getImportedSymbol('function', $name)) {
                     $arguments = new Twig_Node_Expression_Array(array(), $line);

diff --git a/lib/Twig/Node/Expression/GetAttr.php b/lib/Twig/Node/Expression/GetAttr.php
@@ -11,7 +11,7 @@
  */
 class Twig_Node_Expression_GetAttr extends Twig_Node_Expression
 {
-    public function __construct(Twig_Node_Expression $node, Twig_Node_Expression $attribute, Twig_Node_Expression_Array $arguments, $type, $lineno)
+    public function __construct(Twig_Node_Expression $node, Twig_Node_Expression $attribute, Twig_Node_Expression $arguments = null, $type, $lineno)
     {
         parent::__construct(array('node' => $node, 'attribute' => $attribute, 'arguments' => $arguments), array('type' => $type, 'is_defined_test' => false, 'ignore_strict_check' => false, 'disable_c_ext' => false), $lineno);
     }
@@ -32,20 +32,30 @@ public function compile(Twig_Compiler $compiler)
 
         $compiler->raw(', ')->subcompile($this->getNode('attribute'));
 
-        if (count($this->getNode('arguments')) || Twig_Template::ANY_CALL !== $this->getAttribute('type') || $this->getAttribute('is_defined_test') || $this->getAttribute('ignore_strict_check')) {
-            $compiler->raw(', ')->subcompile($this->getNode('arguments'));
-
-            if (Twig_Template::ANY_CALL !== $this->getAttribute('type') || $this->getAttribute('is_defined_test') || $this->getAttribute('ignore_strict_check')) {
-                $compiler->raw(', ')->repr($this->getAttribute('type'));
+        // only generate optional arguments when needed (to make generated code more readable)
+        $needFourth = $this->getAttribute('ignore_strict_check');
+        $needThird = $needFourth || $this->getAttribute('is_defined_test');
+        $needSecond = $needThird || Twig_Template::ANY_CALL !== $this->getAttribute('type');
+        $needFirst = $needSecond || null !== $this->getNode('arguments');
+
+        if ($needFirst) {
+            if (null !== $this->getNode('arguments')) {
+                $compiler->raw(', ')->subcompile($this->getNode('arguments'));
+            } else {
+                $compiler->raw(', array()');
             }
+        }
 
-            if ($this->getAttribute('is_defined_test') || $this->getAttribute('ignore_strict_check')) {
-                $compiler->raw(', '.($this->getAttribute('is_defined_test') ? 'true' : 'false'));
-            }
+        if ($needSecond) {
+            $compiler->raw(', ')->repr($this->getAttribute('type'));
+        }
 
-            if ($this->getAttribute('ignore_strict_check')) {
-                $compiler->raw(', '.($this->getAttribute('ignore_strict_check') ? 'true' : 'false'));
-            }
+        if ($needThird) {
+            $compiler->raw(', ')->repr($this->getAttribute('is_defined_test'));
+        }
+
+        if ($needFourth) {
+            $compiler->raw(', ')->repr($this->getAttribute('ignore_strict_check'));
         }
 
         $compiler->raw(')');

diff --git a/test/Twig/Tests/Fixtures/functions/attribute.test b/test/Twig/Tests/Fixtures/functions/attribute.test
@@ -4,13 +4,15 @@
 {{ attribute(obj, method) }}
 {{ attribute(array, item) }}
 {{ attribute(obj, "bar", ["a", "b"]) }}
+{{ attribute(obj, "bar", arguments) }}
 {{ attribute(obj, method) is defined ? 'ok' : 'ko' }}
 {{ attribute(obj, nonmethod) is defined ? 'ok' : 'ko' }}
 --DATA--
-return array('obj' => new TwigTestFoo(), 'method' => 'foo', 'array' => array('foo' => 'bar'), 'item' => 'foo', 'nonmethod' => 'xxx')
+return array('obj' => new TwigTestFoo(), 'method' => 'foo', 'array' => array('foo' => 'bar'), 'item' => 'foo', 'nonmethod' => 'xxx', 'arguments' => array('a', 'b'))
 --EXPECT--
 foo
 bar
 bar_a-b
+bar_a-b
 ok
 ko
diff --git a/test/Twig/Tests/Node/Expression/GetAttrTest.php b/test/Twig/Tests/Node/Expression/GetAttrTest.php
@@ -46,7 +46,7 @@ public function getTests()
         $attr = new Twig_Node_Expression_Constant('bar', 1);
         $args = new Twig_Node_Expression_Array(array(), 1);
         $node = new Twig_Node_Expression_GetAttr($expr, $attr, $args, Twig_Template::ANY_CALL, 1);
-        $tests[] = array($node, sprintf('%s%s, "bar")', $this->getAttributeGetter(), $this->getVariableGetter('foo')));
+        $tests[] = array($node, sprintf('%s%s, "bar", array())', $this->getAttributeGetter(), $this->getVariableGetter('foo')));
 
         $node = new Twig_Node_Expression_GetAttr($expr, $attr, $args, Twig_Template::ARRAY_CALL, 1);
         $tests[] = array($node, sprintf('%s%s, "bar", array(), "array")', $this->getAttributeGetter(), $this->getVariableGetter('foo')));