From 03f18b44f4508bbf145c430fd8d036dd4dcc019b Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 19 Sep 2016 11:45:16 -0700 Subject: [PATCH] enforced Node:: to only contain node instances --- CHANGELOG | 1 + lib/Twig/Node.php | 5 +++++ lib/Twig/Node/Expression/Call.php | 2 +- lib/Twig/Node/Expression/GetAttr.php | 11 +++++++--- lib/Twig/Node/Expression/Test.php | 7 ++++++- lib/Twig/Node/For.php | 13 ++++++++---- lib/Twig/Node/If.php | 9 +++++++-- lib/Twig/Node/Include.php | 9 +++++++-- lib/Twig/Node/Module.php | 30 +++++++++++++++++----------- test/Twig/Tests/Node/ForTest.php | 2 +- test/Twig/Tests/Node/IfTest.php | 2 +- test/Twig/Tests/Node/IncludeTest.php | 2 +- 12 files changed, 65 insertions(+), 28 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 2fee97aa2..61d8ed195 100644 --- a/CHANGELOG +++ b/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() diff --git a/lib/Twig/Node.php b/lib/Twig/Node.php index 7688a59fe..d48a179a0 100644 --- a/lib/Twig/Node.php +++ b/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; diff --git a/lib/Twig/Node/Expression/Call.php b/lib/Twig/Node/Expression/Call.php index bae080c9a..c1aeba9bd 100644 --- a/lib/Twig/Node/Expression/Call.php +++ b/lib/Twig/Node/Expression/Call.php @@ -71,7 +71,7 @@ abstract class Twig_Node_Expression_Call extends Twig_Node_Expression $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')); diff --git a/lib/Twig/Node/Expression/GetAttr.php b/lib/Twig/Node/Expression/GetAttr.php index 6ce61111c..9a8d38160 100644 --- a/lib/Twig/Node/Expression/GetAttr.php +++ b/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 @@ class Twig_Node_Expression_GetAttr extends Twig_Node_Expression $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()'); diff --git a/lib/Twig/Node/Expression/Test.php b/lib/Twig/Node/Expression/Test.php index c0358c8bf..125d381d8 100644 --- a/lib/Twig/Node/Expression/Test.php +++ b/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) diff --git a/lib/Twig/Node/For.php b/lib/Twig/Node/For.php index 2d450932c..5498a88d2 100644 --- a/lib/Twig/Node/For.php +++ b/lib/Twig/Node/For.php @@ -27,7 +27,12 @@ class Twig_Node_For extends Twig_Node $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 @@ class Twig_Node_For extends Twig_Node ->raw(");\n") ; - if (null !== $this->getNode('else')) { + if ($this->hasNode('else')) { $compiler->write("\$context['_iterated'] = false;\n"); } @@ -69,7 +74,7 @@ class Twig_Node_For extends Twig_Node } } - $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 @@ class Twig_Node_For extends Twig_Node ->write("}\n") ; - if (null !== $this->getNode('else')) { + if ($this->hasNode('else')) { $compiler ->write("if (!\$context['_iterated']) {\n") ->indent() diff --git a/lib/Twig/Node/If.php b/lib/Twig/Node/If.php index caff93682..0aa9e7074 100644 --- a/lib/Twig/Node/If.php +++ b/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 @@ class Twig_Node_If extends Twig_Node ; } - if ($this->hasNode('else') && null !== $this->getNode('else')) { + if ($this->hasNode('else')) { $compiler ->outdent() ->write("} else {\n") diff --git a/lib/Twig/Node/Include.php b/lib/Twig/Node/Include.php index 01c5580c5..bc80dc4f1 100644 --- a/lib/Twig/Node/Include.php +++ b/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 @@ class Twig_Node_Include extends Twig_Node implements Twig_NodeOutputInterface 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 diff --git a/lib/Twig/Node/Module.php b/lib/Twig/Node/Module.php index 19990347a..c50d0e5ba 100644 --- a/lib/Twig/Node/Module.php +++ b/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 @@ class Twig_Node_Module extends Twig_Node '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 @@ class Twig_Node_Module extends Twig_Node 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 @@ class Twig_Node_Module extends Twig_Node 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 @@ class Twig_Node_Module extends Twig_Node ->raw(', ') ->repr($this->getAttribute('filename')) ->raw(', ') - ->repr($this->getNode('parent')->getLine()) + ->repr($parent->getLine()) ->raw(')') ; } @@ -149,9 +154,9 @@ class Twig_Node_Module extends Twig_Node ; // 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(') @@ -159,7 +164,7 @@ class Twig_Node_Module extends Twig_Node ->raw(', ') ->repr($this->getAttribute('filename')) ->raw(', ') - ->repr($this->getNode('parent')->getLine()) + ->repr($parent->getLine()) ->raw(");\n") ; } @@ -277,7 +282,8 @@ class Twig_Node_Module extends Twig_Node ->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 @@ class Twig_Node_Module extends Twig_Node // // 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); diff --git a/test/Twig/Tests/Node/ForTest.php b/test/Twig/Tests/Node/ForTest.php index b2c6fa42f..2bf4c7b43 100644 --- a/test/Twig/Tests/Node/ForTest.php +++ b/test/Twig/Tests/Node/ForTest.php @@ -28,7 +28,7 @@ class Twig_Tests_Node_ForTest extends Twig_Test_NodeTestCase $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); diff --git a/test/Twig/Tests/Node/IfTest.php b/test/Twig/Tests/Node/IfTest.php index e47dd6540..4ab0e4cc7 100644 --- a/test/Twig/Tests/Node/IfTest.php +++ b/test/Twig/Tests/Node/IfTest.php @@ -21,7 +21,7 @@ class Twig_Tests_Node_IfTest extends Twig_Test_NodeTestCase $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); diff --git a/test/Twig/Tests/Node/IncludeTest.php b/test/Twig/Tests/Node/IncludeTest.php index 6fe5c17bf..d801f3387 100644 --- a/test/Twig/Tests/Node/IncludeTest.php +++ b/test/Twig/Tests/Node/IncludeTest.php @@ -16,7 +16,7 @@ class Twig_Tests_Node_IncludeTest extends Twig_Test_NodeTestCase $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'));