feature #2123 deprecated the ability to store non Node instances in N…

…ode::$nodes (fabpot)

This PR was merged into the 1.x branch.

Discussion
----------

deprecated the ability to store non Node instances in Node::$nodes

Commits
-------

03f18b4 enforced Node:: to only contain node instances

CHANGELOG

@@ -1,5 +1,6 @@
 * 1.25.0 (2016-XX-XX)
 
+ * deprecated the ability to store non Node instances in Node::$nodes
  * deprecated Twig_Environment::getLexer(), Twig_Environment::getParser(), Twig_Environment::getCompiler()
  * deprecated Twig_Compiler::getFilename()
 

lib/Twig/Node.php

@@ -37,6 +37,11 @@ class Twig_Node implements Twig_NodeInterface
      */
     public function __construct(array $nodes = array(), array $attributes = array(), $lineno = 0, $tag = null)
     {
+        foreach ($nodes as $name => $node) {
+            if (!$node instanceof Twig_NodeInterface) {
+                @trigger_error(sprintf('Using "%s" for the value of node "%s" of "%s" is deprecated since version 1.25 and will be removed in 2.0.', is_object($node) ? get_class($node) : null === $node ? 'null' : gettype($node), $name, get_class($this)), E_USER_DEPRECATED);
+            }
+        }
         $this->nodes = $nodes;
         $this->attributes = $attributes;
         $this->lineno = $lineno;

lib/Twig/Node/Expression/Call.php

@@ -71,7 +71,7 @@ protected function compileArguments(Twig_Compiler $compiler)
             $first = false;
         }
 
-        if ($this->hasNode('arguments') && null !== $this->getNode('arguments')) {
+        if ($this->hasNode('arguments')) {
             $callable = $this->hasAttribute('callable') ? $this->getAttribute('callable') : null;
 
             $arguments = $this->getArguments($callable, $this->getNode('arguments'));

lib/Twig/Node/Expression/GetAttr.php

@@ -13,7 +13,12 @@ class Twig_Node_Expression_GetAttr extends Twig_Node_Expression
 {
     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);
+        $nodes = array('node' => $node, 'attribute' => $attribute);
+        if (null !== $arguments) {
+            $nodes['arguments'] = $arguments;
+        }
+
+        parent::__construct($nodes, array('type' => $type, 'is_defined_test' => false, 'ignore_strict_check' => false, 'disable_c_ext' => false), $lineno);
     }
 
     public function compile(Twig_Compiler $compiler)
@@ -36,10 +41,10 @@ public function compile(Twig_Compiler $compiler)
         $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');
+        $needFirst = $needSecond || $this->hasNode('arguments');
 
         if ($needFirst) {
-            if (null !== $this->getNode('arguments')) {
+            if ($this->hasNode('arguments')) {
                 $compiler->raw(', ')->subcompile($this->getNode('arguments'));
             } else {
                 $compiler->raw(', array()');

lib/Twig/Node/Expression/Test.php

@@ -12,7 +12,12 @@ class Twig_Node_Expression_Test extends Twig_Node_Expression_Call
 {
     public function __construct(Twig_NodeInterface $node, $name, Twig_NodeInterface $arguments = null, $lineno)
     {
-        parent::__construct(array('node' => $node, 'arguments' => $arguments), array('name' => $name), $lineno);
+        $nodes = array('node' => $node);
+        if (null !== $arguments) {
+            $nodes['arguments'] = $arguments;
+        }
+
+        parent::__construct($nodes, array('name' => $name), $lineno);
     }
 
     public function compile(Twig_Compiler $compiler)

lib/Twig/Node/For.php

@@ -27,7 +27,12 @@ public function __construct(Twig_Node_Expression_AssignName $keyTarget, Twig_Nod
             $body = new Twig_Node_If(new Twig_Node(array($ifexpr, $body)), null, $lineno, $tag);
         }
 
-        parent::__construct(array('key_target' => $keyTarget, 'value_target' => $valueTarget, 'seq' => $seq, 'body' => $body, 'else' => $else), array('with_loop' => true, 'ifexpr' => null !== $ifexpr), $lineno, $tag);
+        $nodes = array('key_target' => $keyTarget, 'value_target' => $valueTarget, 'seq' => $seq, 'body' => $body);
+        if (null !== $else) {
+            $nodes['else'] = $else;
+        }
+
+        parent::__construct($nodes, array('with_loop' => true, 'ifexpr' => null !== $ifexpr), $lineno, $tag);
     }
 
     public function compile(Twig_Compiler $compiler)
@@ -40,7 +45,7 @@ public function compile(Twig_Compiler $compiler)
             ->raw(");\n")
         ;
 
-        if (null !== $this->getNode('else')) {
+        if ($this->hasNode('else')) {
             $compiler->write("\$context['_iterated'] = false;\n");
         }
 
@@ -69,7 +74,7 @@ public function compile(Twig_Compiler $compiler)
             }
         }
 
-        $this->loop->setAttribute('else', null !== $this->getNode('else'));
+        $this->loop->setAttribute('else', $this->hasNode('else'));
         $this->loop->setAttribute('with_loop', $this->getAttribute('with_loop'));
         $this->loop->setAttribute('ifexpr', $this->getAttribute('ifexpr'));
 
@@ -85,7 +90,7 @@ public function compile(Twig_Compiler $compiler)
             ->write("}\n")
         ;
 
-        if (null !== $this->getNode('else')) {
+        if ($this->hasNode('else')) {
             $compiler
                 ->write("if (!\$context['_iterated']) {\n")
                 ->indent()

lib/Twig/Node/If.php

@@ -19,7 +19,12 @@ class Twig_Node_If extends Twig_Node
 {
     public function __construct(Twig_NodeInterface $tests, Twig_NodeInterface $else = null, $lineno, $tag = null)
     {
-        parent::__construct(array('tests' => $tests, 'else' => $else), array(), $lineno, $tag);
+        $nodes = array('tests' => $tests);
+        if (null !== $else) {
+            $nodes['else'] = $else;
+        }
+
+        parent::__construct($nodes, array(), $lineno, $tag);
     }
 
     public function compile(Twig_Compiler $compiler)
@@ -45,7 +50,7 @@ public function compile(Twig_Compiler $compiler)
             ;
         }
 
-        if ($this->hasNode('else') && null !== $this->getNode('else')) {
+        if ($this->hasNode('else')) {
             $compiler
                 ->outdent()
                 ->write("} else {\n")

lib/Twig/Node/Include.php

@@ -19,7 +19,12 @@ class Twig_Node_Include extends Twig_Node implements Twig_NodeOutputInterface
 {
     public function __construct(Twig_Node_Expression $expr, Twig_Node_Expression $variables = null, $only = false, $ignoreMissing = false, $lineno, $tag = null)
     {
-        parent::__construct(array('expr' => $expr, 'variables' => $variables), array('only' => (bool) $only, 'ignore_missing' => (bool) $ignoreMissing), $lineno, $tag);
+        $nodes = array('expr' => $expr);
+        if (null !== $variables) {
+            $nodes['variables'] = $variables;
+        }
+
+        parent::__construct($nodes, array('only' => (bool) $only, 'ignore_missing' => (bool) $ignoreMissing), $lineno, $tag);
     }
 
     public function compile(Twig_Compiler $compiler)
@@ -68,7 +73,7 @@ protected function addGetTemplate(Twig_Compiler $compiler)
 
     protected function addTemplateArguments(Twig_Compiler $compiler)
     {
-        if (null === $this->getNode('variables')) {
+        if (!$this->hasNode('variables')) {
             $compiler->raw(false === $this->getAttribute('only') ? '$context' : 'array()');
         } elseif (false === $this->getAttribute('only')) {
             $compiler

lib/Twig/Node/Module.php

@@ -23,9 +23,7 @@ class Twig_Node_Module extends Twig_Node
 {
     public function __construct(Twig_NodeInterface $body, Twig_Node_Expression $parent = null, Twig_NodeInterface $blocks, Twig_NodeInterface $macros, Twig_NodeInterface $traits, $embeddedTemplates, $filename)
     {
-        // embedded templates are set as attributes so that they are only visited once by the visitors
-        parent::__construct(array(
-            'parent' => $parent,
+        $nodes = array(
             'body' => $body,
             'blocks' => $blocks,
             'macros' => $macros,
@@ -35,7 +33,13 @@ public function __construct(Twig_NodeInterface $body, Twig_Node_Expression $pare
             'constructor_start' => new Twig_Node(),
             'constructor_end' => new Twig_Node(),
             'class_end' => new Twig_Node(),
-        ), array(
+        );
+        if (null !== $parent) {
+            $nodes['parent'] = $parent;
+        }
+
+        // embedded templates are set as attributes so that they are only visited once by the visitors
+        parent::__construct($nodes, array(
             'filename' => $filename,
             'index' => null,
             'embedded_templates' => $embeddedTemplates,
@@ -67,7 +71,7 @@ protected function compileTemplate(Twig_Compiler $compiler)
         if (
             count($this->getNode('blocks'))
             || count($this->getNode('traits'))
-            || null === $this->getNode('parent')
+            || !$this->hasNode('parent')
             || $this->getNode('parent') instanceof Twig_Node_Expression_Constant
             || count($this->getNode('constructor_start'))
             || count($this->getNode('constructor_end'))
@@ -94,9 +98,10 @@ protected function compileTemplate(Twig_Compiler $compiler)
 
     protected function compileGetParent(Twig_Compiler $compiler)
     {
-        if (null === $parent = $this->getNode('parent')) {
+        if (!$this->hasNode('parent')) {
             return;
         }
+        $parent = $this->getNode('parent');
 
         $compiler
             ->write("protected function doGetParent(array \$context)\n", "{\n")
@@ -114,7 +119,7 @@ protected function compileGetParent(Twig_Compiler $compiler)
                 ->raw(', ')
                 ->repr($this->getAttribute('filename'))
                 ->raw(', ')
-                ->repr($this->getNode('parent')->getLine())
+                ->repr($parent->getLine())
                 ->raw(')')
             ;
         }
@@ -149,17 +154,17 @@ protected function compileConstructor(Twig_Compiler $compiler)
         ;
 
         // parent
-        if (null === $parent = $this->getNode('parent')) {
+        if (!$this->hasNode('parent')) {
             $compiler->write("\$this->parent = false;\n\n");
-        } elseif ($parent instanceof Twig_Node_Expression_Constant) {
+        } elseif (($parent = $this->getNode('parent')) && $parent instanceof Twig_Node_Expression_Constant) {
             $compiler
                 ->addDebugInfo($parent)
                 ->write('$this->parent = $this->loadTemplate(')
                 ->subcompile($parent)
                 ->raw(', ')
                 ->repr($this->getAttribute('filename'))
                 ->raw(', ')
-                ->repr($this->getNode('parent')->getLine())
+                ->repr($parent->getLine())
                 ->raw(");\n")
             ;
         }
@@ -277,7 +282,8 @@ protected function compileDisplay(Twig_Compiler $compiler)
             ->subcompile($this->getNode('body'))
         ;
 
-        if (null !== $parent = $this->getNode('parent')) {
+        if ($this->hasNode('parent')) {
+            $parent = $this->getNode('parent');
             $compiler->addDebugInfo($parent);
             if ($parent instanceof Twig_Node_Expression_Constant) {
                 $compiler->write('$this->parent');
@@ -330,7 +336,7 @@ protected function compileIsTraitable(Twig_Compiler $compiler)
         //
         // Put another way, a template can be used as a trait if it
         // only contains blocks and use statements.
-        $traitable = null === $this->getNode('parent') && 0 === count($this->getNode('macros'));
+        $traitable = !$this->hasNode('parent') && 0 === count($this->getNode('macros'));
         if ($traitable) {
             if ($this->getNode('body') instanceof Twig_Node_Body) {
                 $nodes = $this->getNode('body')->getNode(0);

test/Twig/Tests/Node/ForTest.php

@@ -28,7 +28,7 @@ public function testConstructor()
         $this->assertTrue($node->getAttribute('ifexpr'));
         $this->assertEquals('Twig_Node_If', get_class($node->getNode('body')));
         $this->assertEquals($body, $node->getNode('body')->getNode('tests')->getNode(1)->getNode(0));
-        $this->assertNull($node->getNode('else'));
+        $this->assertFalse($node->hasNode('else'));
 
         $else = new Twig_Node_Print(new Twig_Node_Expression_Name('foo', 1), 1);
         $node = new Twig_Node_For($keyTarget, $valueTarget, $seq, $ifexpr, $body, $else, 1);

test/Twig/Tests/Node/IfTest.php

@@ -21,7 +21,7 @@ public function testConstructor()
         $node = new Twig_Node_If($t, $else, 1);
 
         $this->assertEquals($t, $node->getNode('tests'));
-        $this->assertNull($node->getNode('else'));
+        $this->assertFalse($node->hasNode('else'));
 
         $else = new Twig_Node_Print(new Twig_Node_Expression_Name('bar', 1), 1);
         $node = new Twig_Node_If($t, $else, 1);

test/Twig/Tests/Node/IncludeTest.php

@@ -16,7 +16,7 @@ public function testConstructor()
         $expr = new Twig_Node_Expression_Constant('foo.twig', 1);
         $node = new Twig_Node_Include($expr, null, false, false, 1);
 
-        $this->assertNull($node->getNode('variables'));
+        $this->assertFalse($node->hasNode('variables'));
         $this->assertEquals($expr, $node->getNode('expr'));
         $this->assertFalse($node->getAttribute('only'));