Repository: twigphp/Twig (php)

Issue:
yield used when use_yield = false
Just getting around to updating Craft CMS’s Twig requirement to 3.10 from 3.8.

Some Craft-provided Twig nodes are breaking because they rely on output buffering to capture subcompiled node output, for example:

https://github.com/craftcms/cms/blob/c0a690ae58c795de61edb725460f87d9ddeae292/src/web/twig/nodes/RegisterResourceNode.php#L39-L42

It looks like those could be fixed by utilizing `CaptureNode` instead of output buffering, however it will impact third party plugins as well, so we can’t release this as-is, at least until the next major version.

Shouldn’t `PrintNode` et al. be taking `Environment::useYield()` into account before yielding their output, rather than echoing it? So everything continues to work as before up until Twig is instantiated with `use_yield = true`.

Pull request:
Deprecate OptimizerNodeVisitor::OPTIMIZE_TEXT_NODES
Refs #4146

While trying to debug #4146, I realized that this optimization is not possible as we don't know how nodes are going to be used.

See the script provided in #4146 to reproduce the problem.

@brandonkelly Can you confirm that this fixes your problem? Maybe there is also an issue with yield vs echo, but the script provided in the issue runs fine for me after this PR.


Patch:
diff --git a/CHANGELOG b/CHANGELOG
index fa50ff1e..30e00839 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,6 @@
 # 3.12.0 (2024-XX-XX)
 
+ * Deprecate `OptimizerNodeVisitor::OPTIMIZE_TEXT_NODES`
  * Fix performance regression when `use_yield` is `false` (which is the default)
  * Improve compatibility when `use_yield` is `false` (as extensions still using `echo` will work as is)
  * Accept colons (`:`) in addition to equals (`=`) to separate argument names and values in named arguments
@@ -18,6 +19,7 @@
 
 # 3.11.0 (2024-08-08)
 
+ * Deprecate `OptimizerNodeVisitor::OPTIMIZE_RAW_FILTER`
  * Add `Twig\Cache\ChainCache` and `Twig\Cache\ReadOnlyFilesystemCache`
  * Add the possibility to deprecate attributes and nodes on `Node`
  * Add the possibility to add a package and a version to the `deprecated` tag
diff --git a/doc/deprecated.rst b/doc/deprecated.rst
index 2d2860cb..8320a229 100644
--- a/doc/deprecated.rst
+++ b/doc/deprecated.rst
@@ -141,6 +141,11 @@ Node Visitors
 * The ``Twig\NodeVisitor\AbstractNodeVisitor`` class is deprecated, implement the
   ``Twig\NodeVisitor\NodeVisitorInterface`` interface instead.
 
+* The ``Twig\NodeVisitor\OptimizerNodeVisitor::OPTIMIZE_RAW_FILTER`` and the
+  ``Twig\NodeVisitor\OptimizerNodeVisitor::OPTIMIZE_TEXT_NODES`` options are
+  deprecated as of Twig 3.12 and will be removed in Twig 4.0; they don't do
+  anything anymore.
+
 Parser
 ------
 
diff --git a/src/Extension/CoreExtension.php b/src/Extension/CoreExtension.php
index 73caf8a5..2cf889e7 100644
--- a/src/Extension/CoreExtension.php
+++ b/src/Extension/CoreExtension.php
@@ -817,6 +817,9 @@ final class CoreExtension extends AbstractExtension
         return $r;
     }
 
+    // The '_default' filter is used internally to avoid using the ternary operator
+    // which costs a lot for big contexts (before PHP 5.4). So, on average,
+    // a function call is cheaper.
     /**
      * @internal
      */
diff --git a/src/Lexer.php b/src/Lexer.php
index 28feaa2c..78b07156 100644
--- a/src/Lexer.php
+++ b/src/Lexer.php
@@ -418,7 +418,7 @@ class Lexer
                 $result .= $nextChar;
             } elseif ("'" === $nextChar || '"' === $nextChar) {
                 if ($nextChar !== $quoteType) {
-                    trigger_deprecation('twig/twig', '3.12', 'Character "%s" at position %d should not be escaped; the "\" character is ignored in Twig v3 but will not be in v4. Please remove the extra "\" character.', $nextChar, $i + 1);
+                    trigger_deprecation('twig/twig', '3.12', 'Character "%s" at position %d does not need to be escaped anymore.', $nextChar, $i + 1);
                 }
                 $result .= $nextChar;
             } elseif ('#' === $nextChar && $i + 1 < $length && '{' === $str[$i + 1]) {
@@ -437,7 +437,7 @@ class Lexer
                 }
                 $result .= \chr(octdec($octal));
             } else {
-                trigger_deprecation('twig/twig', '3.12', 'Character "%s" at position %d should not be escaped; the "\" character is ignored in Twig v3 but will not be in v4. Please remove the extra "\" character.', $nextChar, $i + 1);
+                trigger_deprecation('twig/twig', '3.12', \sprintf('Character "%s" at position %d does not need to be escaped anymore.', $nextChar, $i + 1));
                 $result .= $nextChar;
             }
 
diff --git a/src/Loader/ChainLoader.php b/src/Loader/ChainLoader.php
index 90f798db..163c029f 100644
--- a/src/Loader/ChainLoader.php
+++ b/src/Loader/ChainLoader.php
@@ -21,33 +21,22 @@ use Twig\Source;
  */
 final class ChainLoader implements LoaderInterface
 {
-    /**
-     * @var \Traversable<LoaderInterface>|LoaderInterface[]
-     */
-    private $loaders;
-
-    /**
-     * @var array<string, bool>
-     */
     private $hasSourceCache = [];
+    private $loaders = [];
 
     /**
-     * @param iterable<LoaderInterface> $loaders
+     * @param LoaderInterface[] $loaders
      */
-    public function __construct(iterable $loaders = [])
+    public function __construct(array $loaders = [])
     {
-        $this->loaders = $loaders;
+        foreach ($loaders as $loader) {
+            $this->addLoader($loader);
+        }
     }
 
     public function addLoader(LoaderInterface $loader): void
     {
-        $current = $this->loaders;
-
-        $this->loaders = (static function () use ($current, $loader): \Generator {
-            yield from $current;
-            yield $loader;
-        })();
-
+        $this->loaders[] = $loader;
         $this->hasSourceCache = [];
     }
 
@@ -56,18 +45,13 @@ final class ChainLoader implements LoaderInterface
      */
     public function getLoaders(): array
     {
-        if (!\is_array($this->loaders)) {
-            $this->loaders = iterator_to_array($this->loaders, false);
-        }
-
         return $this->loaders;
     }
 
     public function getSourceContext(string $name): Source
     {
         $exceptions = [];
-
-        foreach ($this->getLoaders() as $loader) {
+        foreach ($this->loaders as $loader) {
             if (!$loader->exists($name)) {
                 continue;
             }
@@ -88,7 +72,7 @@ final class ChainLoader implements LoaderInterface
             return $this->hasSourceCache[$name];
         }
 
-        foreach ($this->getLoaders() as $loader) {
+        foreach ($this->loaders as $loader) {
             if ($loader->exists($name)) {
                 return $this->hasSourceCache[$name] = true;
             }
@@ -100,8 +84,7 @@ final class ChainLoader implements LoaderInterface
     public function getCacheKey(string $name): string
     {
         $exceptions = [];
-
-        foreach ($this->getLoaders() as $loader) {
+        foreach ($this->loaders as $loader) {
             if (!$loader->exists($name)) {
                 continue;
             }
@@ -119,8 +102,7 @@ final class ChainLoader implements LoaderInterface
     public function isFresh(string $name, int $time): bool
     {
         $exceptions = [];
-
-        foreach ($this->getLoaders() as $loader) {
+        foreach ($this->loaders as $loader) {
             if (!$loader->exists($name)) {
                 continue;
             }
diff --git a/src/NodeVisitor/OptimizerNodeVisitor.php b/src/NodeVisitor/OptimizerNodeVisitor.php
index 55f5d6eb..0d2dc02d 100644
--- a/src/NodeVisitor/OptimizerNodeVisitor.php
+++ b/src/NodeVisitor/OptimizerNodeVisitor.php
@@ -62,6 +62,10 @@ final class OptimizerNodeVisitor implements NodeVisitorInterface
             trigger_deprecation('twig/twig', '3.11', 'The "Twig\NodeVisitor\OptimizerNodeVisitor::OPTIMIZE_RAW_FILTER" option is deprecated and does nothing.');
         }
 
+        if (-1 !== $optimizers && self::OPTIMIZE_TEXT_NODES === (self::OPTIMIZE_TEXT_NODES & $optimizers)) {
+            trigger_deprecation('twig/twig', '3.12', 'The "Twig\NodeVisitor\OptimizerNodeVisitor::OPTIMIZE_TEXT_NODES" option is deprecated and does nothing.');
+        }
+
         $this->optimizers = $optimizers;
     }
 
@@ -82,42 +86,6 @@ final class OptimizerNodeVisitor implements NodeVisitorInterface
 
         $node = $this->optimizePrintNode($node);
 
-        if (self::OPTIMIZE_TEXT_NODES === (self::OPTIMIZE_TEXT_NODES & $this->optimizers)) {
-            $node = $this->mergeTextNodeCalls($node);
-        }
-
-        return $node;
-    }
-
-    private function mergeTextNodeCalls(Node $node): Node
-    {
-        $text = '';
-        $names = [];
-        foreach ($node as $k => $n) {
-            if (!$n instanceof TextNode) {
-                return $node;
-            }
-
-            $text .= $n->getAttribute('data');
-            $names[] = $k;
-        }
-
-        if (!$text) {
-            return $node;
-        }
-
-        if (Node::class === \get_class($node)) {
-            return new TextNode($text, $node->getTemplateLine());
-        }
-
-        foreach ($names as $i => $name) {
-            if (0 === $i) {
-                $node->setNode($name, new TextNode($text, $node->getTemplateLine()));
-            } else {
-                $node->removeNode($name);
-            }
-        }
-
         return $node;
     }
 
diff --git a/src/Util/ReflectionCallable.php b/src/Util/ReflectionCallable.php
index 9b183f14..b4a70939 100644
--- a/src/Util/ReflectionCallable.php
+++ b/src/Util/ReflectionCallable.php
@@ -21,7 +21,7 @@ use Twig\TwigCallableInterface;
 final class ReflectionCallable
 {
     private $reflector;
-    private $callable;
+    private $callable = null;
     private $name;
 
     public function __construct(


Test patch:
diff --git a/tests/LexerTest.php b/tests/LexerTest.php
index f07a0868..87dfd2e8 100644
--- a/tests/LexerTest.php
+++ b/tests/LexerTest.php
@@ -252,17 +252,17 @@ class LexerTest extends TestCase
         yield '{{ \'App\Test\' }} => AppTest' => [
             '{{ \'App\\Test\' }}',
             'AppTest',
-            'Since twig/twig 3.12: Character "T" at position 5 should not be escaped; the "\" character is ignored in Twig v3 but will not be in v4. Please remove the extra "\" character.',
+            "Since twig/twig 3.12: Character \"T\" at position 5 does not need to be escaped anymore.",
         ];
         yield '{{ "foo \\\' bar" }} => foo \' bar' => [
             '{{ "foo \\\' bar" }}',
             'foo \' bar',
-            'Since twig/twig 3.12: Character "\'" at position 6 should not be escaped; the "\" character is ignored in Twig v3 but will not be in v4. Please remove the extra "\" character.',
+            "Since twig/twig 3.12: Character \"'\" at position 6 does not need to be escaped anymore.",
         ];
         yield '{{ \'foo \" bar\' }} => foo " bar' => [
             '{{ \'foo \\" bar\' }}',
             'foo " bar',
-            'Since twig/twig 3.12: Character """ at position 6 should not be escaped; the "\" character is ignored in Twig v3 but will not be in v4. Please remove the extra "\" character.',
+            'Since twig/twig 3.12: Character """ at position 6 does not need to be escaped anymore.',
         ];
     }
 
diff --git a/tests/Loader/ChainTest.php b/tests/Loader/ChainTest.php
index 52d6d4c7..faaaebe3 100644
--- a/tests/Loader/ChainTest.php
+++ b/tests/Loader/ChainTest.php
@@ -72,30 +72,10 @@ class ChainTest extends TestCase
 
     public function testAddLoader()
     {
-        $fooLoader = new ArrayLoader(['foo' => 'foo:code']);
-        $barLoader = new ArrayLoader(['bar' => 'bar:code']);
-        $bazLoader = new ArrayLoader(['baz' => 'baz:code']);
-        $quxLoader = new ArrayLoader(['qux' => 'qux:code']);
-
-        $loader = new ChainLoader((static function () use ($fooLoader, $barLoader): \Generator {
-            yield $fooLoader;
-            yield $barLoader;
-        })());
-
-        $loader->addLoader($bazLoader);
-        $loader->addLoader($quxLoader);
-
-        $this->assertEquals('foo:code', $loader->getSourceContext('foo')->getCode());
-        $this->assertEquals('bar:code', $loader->getSourceContext('bar')->getCode());
-        $this->assertEquals('baz:code', $loader->getSourceContext('baz')->getCode());
-        $this->assertEquals('qux:code', $loader->getSourceContext('qux')->getCode());
-
-        $this->assertEquals([
-            $fooLoader,
-            $barLoader,
-            $bazLoader,
-            $quxLoader,
-        ], $loader->getLoaders());
+        $loader = new ChainLoader();
+        $loader->addLoader(new ArrayLoader(['foo' => 'bar']));
+
+        $this->assertEquals('bar', $loader->getSourceContext('foo')->getCode());
     }
 
     public function testExists()
diff --git a/tests/NodeVisitor/OptimizerTest.php b/tests/NodeVisitor/OptimizerTest.php
index b670d894..12dc1214 100644
--- a/tests/NodeVisitor/OptimizerTest.php
+++ b/tests/NodeVisitor/OptimizerTest.php
@@ -27,10 +27,8 @@ class OptimizerTest extends TestCase
     public function testConstructor()
     {
         $this->expectNotToPerformAssertions();
-        new OptimizerNodeVisitor(
-            OptimizerNodeVisitor::OPTIMIZE_FOR
-            | OptimizerNodeVisitor::OPTIMIZE_TEXT_NODES
-        );
+
+        new OptimizerNodeVisitor(OptimizerNodeVisitor::OPTIMIZE_FOR);
     }
 
     public function testRenderBlockOptimizer()