feature #4335 Deprecate instantiating Node directly, introduce EmptyN…

…ode and Nodes (fabpot) This PR was merged into the 3.x branch. Discussion ---------- Deprecate instantiating Node directly, introduce EmptyNode and Nodes Based on some comments from `@stof`: See #4292 (comment) See #4333 (comment) First interesting usage here: 65ee72a Commits ------- 8b27898 Deprecate using Node directly, introduce EmptyNode and Nodes
twigphp · Sep 27, 2024 · faf8471 · faf8471
2 parents a1daf1e + 8b27898
commit faf8471
Show file tree

Hide file tree

Showing 37 changed files with 230 additions and 130 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,5 +1,6 @@
 # 3.15.0 (2024-XX-XX)
 
+ * Deprecate instantiating `Node` directly. Use `EmptyNode` or `Nodes` instead.
  * Add support for inline comments
  * Add support for accessing class constants with the dot operator
  * Add `Profile::getStartTime()` and `Profile::getEndTime()`

diff --git a/doc/deprecated.rst b/doc/deprecated.rst
@@ -289,3 +289,10 @@ Functions/Filters/Tests
 
 * For variadic arguments, use snake-case for the argument name to ease the
   transition to 4.0.
+
+Node
+----
+
+* Instantiating ``Twig\Node\Node`` directly is deprecated as of Twig 3.15. Use
+  ``EmptyNode`` or ``Nodes`` instead depending on the use case. The
+  ``Twig\Node\Node`` class will be abstract in Twig 4.0.
diff --git a/src/ExpressionParser.php b/src/ExpressionParser.php
@@ -14,6 +14,7 @@
 
 use Twig\Attribute\FirstClassTwigCallableReady;
 use Twig\Error\SyntaxError;
+use Twig\Node\EmptyNode;
 use Twig\Node\Expression\AbstractExpression;
 use Twig\Node\Expression\ArrayExpression;
 use Twig\Node\Expression\ArrowFunctionExpression;
@@ -32,6 +33,7 @@
 use Twig\Node\Expression\Unary\PosUnary;
 use Twig\Node\Expression\Unary\SpreadUnary;
 use Twig\Node\Node;
+use Twig\Node\Nodes;
 
 /**
  * Parses expressions.
@@ -110,7 +112,7 @@ private function parseArrow()
             $names = [new AssignNameExpression($token->getValue(), $token->getLine())];
             $stream->expect(Token::ARROW_TYPE);
 
-            return new ArrowFunctionExpression($this->parseExpression(0), new Node($names), $line);
+            return new ArrowFunctionExpression($this->parseExpression(0), new Nodes($names), $line);
         }
 
         // first, determine if we are parsing an arrow function by finding => (long form)
@@ -151,7 +153,7 @@ private function parseArrow()
         $stream->expect(Token::PUNCTUATION_TYPE, ')');
         $stream->expect(Token::ARROW_TYPE);
 
-        return new ArrowFunctionExpression($this->parseExpression(0), new Node($names), $line);
+        return new ArrowFunctionExpression($this->parseExpression(0), new Nodes($names), $line);
     }
 
     private function getPrimary(): AbstractExpression
@@ -467,7 +469,7 @@ public function getFunctionNode($name, $line)
         $function = $this->getFunction($name, $line);
 
         if ($function->getParserCallable()) {
-            $fakeNode = new Node(lineno: $line);
+            $fakeNode = new EmptyNode($line);
             $fakeNode->setSourceContext($this->parser->getStream()->getSourceContext());
 
             return ($function->getParserCallable())($this->parser, $fakeNode, $args, $line);
@@ -556,7 +558,7 @@ public function parseSubscriptExpression($node)
                 }
 
                 $filter = $this->getFilter('slice', $token->getLine());
-                $arguments = new Node([$arg, $length]);
+                $arguments = new Nodes([$arg, $length]);
                 $filter = new ($filter->getNodeClass())($node, $filter, $arguments, $token->getLine());
 
                 $stream->expect(Token::PUNCTUATION_TYPE, ']');
@@ -587,7 +589,7 @@ public function parseFilterExpressionRaw($node)
             $token = $this->parser->getStream()->expect(Token::NAME_TYPE);
 
             if (!$this->parser->getStream()->test(Token::PUNCTUATION_TYPE, '(')) {
-                $arguments = new Node();
+                $arguments = new EmptyNode();
             } else {
                 $arguments = $this->parseArguments(true, false, true);
             }
@@ -691,7 +693,7 @@ public function parseArguments($namedArguments = false, $definition = false, $al
         }
         $stream->expect(Token::PUNCTUATION_TYPE, ')', 'A list of arguments must be closed by a parenthesis');
 
-        return new Node($args);
+        return new Nodes($args);
     }
 
     public function parseAssignmentExpression()
@@ -717,7 +719,7 @@ public function parseAssignmentExpression()
             }
         }
 
-        return new Node($targets);
+        return new Nodes($targets);
     }
 
     public function parseMultitargetExpression()
@@ -730,7 +732,7 @@ public function parseMultitargetExpression()
             }
         }
 
-        return new Node($targets);
+        return new Nodes($targets);
     }
 
     private function parseNotTestExpression(Node $node): NotUnary
@@ -747,7 +749,7 @@ private function parseTestExpression(Node $node): TestExpression
         if ($stream->test(Token::PUNCTUATION_TYPE, '(')) {
             $arguments = $this->parseArguments(true);
         } elseif ($test->hasOneMandatoryArgument()) {
-            $arguments = new Node([0 => $this->parsePrimaryExpression()]);
+            $arguments = new Nodes([0 => $this->parsePrimaryExpression()]);
         }
 
         if ('defined' === $test->getName() && $node instanceof NameExpression && null !== $alias = $this->parser->getImportedSymbol('function', $node->getAttribute('name'))) {

diff --git a/src/Node/EmptyNode.php b/src/Node/EmptyNode.php
@@ -0,0 +1,28 @@
+<?php
+
+/*
+ * This file is part of Twig.
+ *
+ * (c) Fabien Potencier
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Twig\Node;
+
+use Twig\Attribute\YieldReady;
+
+/**
+ * Represents an empty node.
+ *
+ * @author Fabien Potencier <[email protected]>
+ */
+#[YieldReady]
+final class EmptyNode extends Node
+{
+    public function __construct(int $lineno = 0)
+    {
+        parent::__construct([], [], $lineno);
+    }
+}
diff --git a/src/Node/Expression/Filter/DefaultFilter.php b/src/Node/Expression/Filter/DefaultFilter.php
@@ -14,6 +14,7 @@
 use Twig\Attribute\FirstClassTwigCallableReady;
 use Twig\Compiler;
 use Twig\Extension\CoreExtension;
+use Twig\Node\EmptyNode;
 use Twig\Node\Expression\ConditionalExpression;
 use Twig\Node\Expression\ConstantExpression;
 use Twig\Node\Expression\FilterExpression;
@@ -45,7 +46,7 @@ public function __construct(Node $node, TwigFilter|ConstantExpression $filter, N
         }
 
         if ('default' === $name && ($node instanceof NameExpression || $node instanceof GetAttrExpression)) {
-            $test = new DefinedTest(clone $node, new TwigTest('defined'), new Node(), $node->getTemplateLine());
+            $test = new DefinedTest(clone $node, new TwigTest('defined'), new EmptyNode(), $node->getTemplateLine());
             $false = \count($arguments) ? $arguments->getNode('0') : new ConstantExpression('', $node->getTemplateLine());
 
             $node = new ConditionalExpression($test, $default, $false, $node->getTemplateLine());

diff --git a/src/Node/Expression/Filter/RawFilter.php b/src/Node/Expression/Filter/RawFilter.php
@@ -13,6 +13,7 @@
 
 use Twig\Attribute\FirstClassTwigCallableReady;
 use Twig\Compiler;
+use Twig\Node\EmptyNode;
 use Twig\Node\Expression\ConstantExpression;
 use Twig\Node\Expression\FilterExpression;
 use Twig\Node\Node;
@@ -26,7 +27,7 @@ class RawFilter extends FilterExpression
     #[FirstClassTwigCallableReady]
     public function __construct(Node $node, TwigFilter|ConstantExpression|null $filter = null, ?Node $arguments = null, int $lineno = 0)
     {
-        parent::__construct($node, $filter ?: new TwigFilter('raw', null, ['is_safe' => ['all']]), $arguments ?: new Node(), $lineno ?: $node->getTemplateLine());
+        parent::__construct($node, $filter ?: new TwigFilter('raw', null, ['is_safe' => ['all']]), $arguments ?: new EmptyNode(), $lineno ?: $node->getTemplateLine());
     }
 
     public function compile(Compiler $compiler): void

diff --git a/src/Node/Expression/NullCoalesceExpression.php b/src/Node/Expression/NullCoalesceExpression.php
@@ -12,6 +12,7 @@
 namespace Twig\Node\Expression;
 
 use Twig\Compiler;
+use Twig\Node\EmptyNode;
 use Twig\Node\Expression\Binary\AndBinary;
 use Twig\Node\Expression\Test\DefinedTest;
 use Twig\Node\Expression\Test\NullTest;
@@ -23,12 +24,12 @@ class NullCoalesceExpression extends ConditionalExpression
 {
     public function __construct(Node $left, Node $right, int $lineno)
     {
-        $test = new DefinedTest(clone $left, new TwigTest('defined'), new Node(), $left->getTemplateLine());
+        $test = new DefinedTest(clone $left, new TwigTest('defined'), new EmptyNode(), $left->getTemplateLine());
         // for "block()", we don't need the null test as the return value is always a string
         if (!$left instanceof BlockReferenceExpression) {
             $test = new AndBinary(
                 $test,
-                new NotUnary(new NullTest($left, new TwigTest('null'), new Node(), $left->getTemplateLine()), $left->getTemplateLine()),
+                new NotUnary(new NullTest($left, new TwigTest('null'), new EmptyNode(), $left->getTemplateLine()), $left->getTemplateLine()),
                 $left->getTemplateLine()
             );
         }

diff --git a/src/Node/ForNode.php b/src/Node/ForNode.php
@@ -29,7 +29,7 @@ class ForNode extends Node
 
     public function __construct(AssignNameExpression $keyTarget, AssignNameExpression $valueTarget, AbstractExpression $seq, ?Node $ifexpr, Node $body, ?Node $else, int $lineno)
     {
-        $body = new Node([$body, $this->loop = new ForLoopNode($lineno)]);
+        $body = new Nodes([$body, $this->loop = new ForLoopNode($lineno)]);
 
         $nodes = ['key_target' => $keyTarget, 'value_target' => $valueTarget, 'seq' => $seq, 'body' => $body];
         if (null !== $else) {

diff --git a/src/Node/ModuleNode.php b/src/Node/ModuleNode.php
@@ -44,11 +44,11 @@ public function __construct(Node $body, ?AbstractExpression $parent, Node $block
             'blocks' => $blocks,
             'macros' => $macros,
             'traits' => $traits,
-            'display_start' => new Node(),
-            'display_end' => new Node(),
-            'constructor_start' => new Node(),
-            'constructor_end' => new Node(),
-            'class_end' => new Node(),
+            'display_start' => new EmptyNode(),
+            'display_end' => new EmptyNode(),
+            'constructor_start' => new EmptyNode(),
+            'constructor_end' => new EmptyNode(),
+            'class_end' => new EmptyNode(),
         ];
         if (null !== $parent) {
             $nodes['parent'] = $parent;
@@ -414,7 +414,7 @@ protected function compileIsTraitable(Compiler $compiler)
             }
 
             if (!\count($nodes)) {
-                $nodes = new Node([$nodes]);
+                $nodes = new Nodes([$nodes]);
             }
 
             foreach ($nodes as $node) {

diff --git a/src/Node/Node.php b/src/Node/Node.php
@@ -47,6 +47,10 @@ class Node implements \Countable, \IteratorAggregate
      */
     public function __construct(array $nodes = [], array $attributes = [], int $lineno = 0)
     {
+        if (self::class === static::class) {
+            trigger_deprecation('twig/twig', '3.15', \sprintf('Instantiating "%s" directly is deprecated; the class will become abstract in 4.0.', self::class));
+        }
+
         foreach ($nodes as $name => $node) {
             if (!$node instanceof self) {
                 throw new \InvalidArgumentException(\sprintf('Using "%s" for the value of node "%s" of "%s" is not supported. You must pass a \Twig\Node\Node instance.', \is_object($node) ? $node::class : (null === $node ? 'null' : \gettype($node)), $name, static::class));

diff --git a/src/Node/Nodes.php b/src/Node/Nodes.php
@@ -0,0 +1,28 @@
+<?php
+
+/*
+ * This file is part of Twig.
+ *
+ * (c) Fabien Potencier
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Twig\Node;
+
+use Twig\Attribute\YieldReady;
+
+/**
+ * Represents a list of nodes.
+ *
+ * @author Fabien Potencier <[email protected]>
+ */
+#[YieldReady]
+final class Nodes extends Node
+{
+    public function __construct(array $nodes = [], int $lineno = 0)
+    {
+        parent::__construct($nodes, [], $lineno);
+    }
+}
diff --git a/src/Node/SetNode.php b/src/Node/SetNode.php
@@ -33,7 +33,8 @@ public function __construct(bool $capture, Node $names, Node $values, int $linen
         $safe = false;
         if ($capture) {
             $safe = true;
-            if (Node::class === get_class($values) && !count($values)) {
+            // Node::class === get_class($values) should be removed in Twig 4.0
+            if (($values instanceof Nodes || Node::class === get_class($values)) && !count($values)) {
                 $values = new ConstantExpression('', $values->getTemplateLine());
                 $capture = false;
             } elseif ($values instanceof TextNode) {

diff --git a/src/NodeVisitor/EscaperNodeVisitor.php b/src/NodeVisitor/EscaperNodeVisitor.php
@@ -24,6 +24,7 @@
 use Twig\Node\ImportNode;
 use Twig\Node\ModuleNode;
 use Twig\Node\Node;
+use Twig\Node\Nodes;
 use Twig\Node\PrintNode;
 use Twig\NodeTraverser;
 
@@ -197,7 +198,7 @@ private function getEscaperFilter(Environment $env, string $type, Node $node): F
     {
         $line = $node->getTemplateLine();
         $filter = $env->getFilter('escape');
-        $args = new Node([new ConstantExpression($type, $line), new ConstantExpression(null, $line), new ConstantExpression(true, $line)]);
+        $args = new Nodes([new ConstantExpression($type, $line), new ConstantExpression(null, $line), new ConstantExpression(true, $line)]);
 
         return new FilterExpression($node, $filter, $args, $line);
     }

diff --git a/src/NodeVisitor/SandboxNodeVisitor.php b/src/NodeVisitor/SandboxNodeVisitor.php
@@ -23,6 +23,7 @@
 use Twig\Node\Expression\NameExpression;
 use Twig\Node\ModuleNode;
 use Twig\Node\Node;
+use Twig\Node\Nodes;
 use Twig\Node\PrintNode;
 use Twig\Node\SetNode;
 
@@ -105,8 +106,8 @@ public function leaveNode(Node $node, Environment $env): ?Node
         if ($node instanceof ModuleNode) {
             $this->inAModule = false;
 
-            $node->setNode('constructor_end', new Node([new CheckSecurityCallNode(), $node->getNode('constructor_end')]));
-            $node->setNode('class_end', new Node([new CheckSecurityNode($this->filters, $this->tags, $this->functions), $node->getNode('class_end')]));
+            $node->setNode('constructor_end', new Nodes([new CheckSecurityCallNode(), $node->getNode('constructor_end')]));
+            $node->setNode('class_end', new Nodes([new CheckSecurityNode($this->filters, $this->tags, $this->functions), $node->getNode('class_end')]));
         } elseif ($this->inAModule) {
             if ($node instanceof PrintNode || $node instanceof SetNode) {
                 $this->needsToStringWrap = false;

diff --git a/src/Parser.php b/src/Parser.php
@@ -16,12 +16,14 @@
 use Twig\Node\BlockNode;
 use Twig\Node\BlockReferenceNode;
 use Twig\Node\BodyNode;
+use Twig\Node\EmptyNode;
 use Twig\Node\Expression\AbstractExpression;
 use Twig\Node\MacroNode;
 use Twig\Node\ModuleNode;
 use Twig\Node\Node;
 use Twig\Node\NodeCaptureInterface;
 use Twig\Node\NodeOutputInterface;
+use Twig\Node\Nodes;
 use Twig\Node\PrintNode;
 use Twig\Node\TextNode;
 use Twig\TokenParser\TokenParserInterface;
@@ -83,7 +85,7 @@ public function parse(TokenStream $stream, $test = null, bool $dropNeedle = fals
             $body = $this->subparse($test, $dropNeedle);
 
             if (null !== $this->parent && null === $body = $this->filterBodyNodes($body)) {
-                $body = new Node();
+                $body = new EmptyNode();
             }
         } catch (SyntaxError $e) {
             if (!$e->getSourceContext()) {
@@ -97,7 +99,7 @@ public function parse(TokenStream $stream, $test = null, bool $dropNeedle = fals
             throw $e;
         }
 
-        $node = new ModuleNode(new BodyNode([$body]), $this->parent, new Node($this->blocks), new Node($this->macros), new Node($this->traits), $this->embeddedTemplates, $stream->getSourceContext());
+        $node = new ModuleNode(new BodyNode([$body]), $this->parent, new Nodes($this->blocks), new Nodes($this->macros), new Nodes($this->traits), $this->embeddedTemplates, $stream->getSourceContext());
 
         $traverser = new NodeTraverser($this->env, $this->visitors);
 
@@ -149,7 +151,7 @@ public function subparse($test, bool $dropNeedle = false): Node
                             return $rv[0];
                         }
 
-                        return new Node($rv, [], $lineno);
+                        return new Nodes($rv, $lineno);
                     }
 
                     if (!$subparser = $this->env->getTokenParser($token->getValue())) {
@@ -189,7 +191,7 @@ public function subparse($test, bool $dropNeedle = false): Node
             return $rv[0];
         }
 
-        return new Node($rv, [], $lineno);
+        return new Nodes($rv, $lineno);
     }
 
     public function getBlockStack(): array
@@ -371,7 +373,8 @@ private function filterBodyNodes(Node $node, bool $nested = false): ?Node
 
         // here, $nested means "being at the root level of a child template"
         // we need to discard the wrapping "Node" for the "body" node
-        $nested = $nested || Node::class !== \get_class($node);
+        // Node::class !== \get_class($node) should be removed in Twig 4.0
+        $nested = $nested || (Node::class !== \get_class($node) && !$node instanceof Nodes);
         foreach ($node as $k => $n) {
             if (null !== $n && null === $this->filterBodyNodes($n, $nested)) {
                 $node->removeNode($k);