feature #4335 Deprecate instantiating Node directly, introduce EmptyNode 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

Author: fabpot

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()`

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.

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'))) {

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);
+    }
+}

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());

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

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()
             );
         }

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) {

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) {

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));