feature #3950 Deprecate usage of ob_* functions in favor of yielding …

…(fabpot) This PR was squashed before being merged into the 3.x branch. Discussion ---------- Deprecate usage of ob_* functions in favor of yielding Still a work in progress, but everything works fine in both modes. Commits ------- a93723e Fix bug 0d3a283 Tweak code 14d3803 Add a deprecation when a Node uses echo/print 210e2d2 Fix tests 1591aa5 Optimize code 1469e6a Add support for templates that do not have output Nodes 72922a3 - 71e90fa Remove some tests 4fd62ab - a9c7307 Tweak code 1587940 Remove the new yield nodes 3b6cbf9 Remove usage of ob_* functions in favor of yielding
twigphp · Feb 5, 2024 · edf5808 · edf5808
2 parents 594c548 + a93723e
commit edf5808
Show file tree

Hide file tree

Showing 35 changed files with 634 additions and 147 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -14,7 +14,7 @@ permissions:
 
 jobs:
     tests:
-        name: "PHP ${{ matrix.php-version }}"
+        name: "PHP ${{ matrix.php-version }} (yield: ${{ matrix.use_yield }})"
 
         runs-on: 'ubuntu-latest'
 
@@ -31,6 +31,7 @@ jobs:
                     - '8.2'
                     - '8.3'
                 experimental: [false]
+                use_yield: [true, false]
 
         steps:
             - name: "Checkout code"
@@ -48,20 +49,25 @@ jobs:
 
             - run: composer install
 
+            - name: "Switch use_yield to true"
+              if: ${{ matrix.use_yield }}
+              run: |
+                  sed -i -e "s/'use_yield' => false/'use_yield' => true/" src/Environment.php
+
             - name: "Install PHPUnit"
               run: vendor/bin/simple-phpunit install
 
             - name: "PHPUnit version"
               run: vendor/bin/simple-phpunit --version
 
             - name: "Run tests"
-              run: vendor/bin/simple-phpunit
+              run: SYMFONY_DEPRECATIONS_HELPER=ignoreFile=./tests/ignore-use-yield-deprecations vendor/bin/simple-phpunit
 
     extension-tests:
         needs:
             - 'tests'
 
-        name: "${{ matrix.extension }} with PHP ${{ matrix.php-version }}"
+        name: "${{ matrix.extension }} PHP ${{ matrix.php-version }} (yield: ${{ matrix.use_yield }})"
 
         runs-on: 'ubuntu-latest'
 
@@ -78,15 +84,16 @@ jobs:
                     - '8.2'
                     - '8.3'
                 extension:
-                    - 'extra/cache-extra'
-                    - 'extra/cssinliner-extra'
-                    - 'extra/html-extra'
-                    - 'extra/inky-extra'
-                    - 'extra/intl-extra'
-                    - 'extra/markdown-extra'
-                    - 'extra/string-extra'
-                    - 'extra/twig-extra-bundle'
+                    - 'cache-extra'
+                    - 'cssinliner-extra'
+                    - 'html-extra'
+                    - 'inky-extra'
+                    - 'intl-extra'
+                    - 'markdown-extra'
+                    - 'string-extra'
+                    - 'twig-extra-bundle'
                 experimental: [false]
+                use_yield: [true, false]
 
         steps:
             - name: "Checkout code"
@@ -115,9 +122,17 @@ jobs:
               working-directory: ${{ matrix.extension}}
               run: composer install
 
+            - name: "Switch use_yield to true"
+              if: ${{ matrix.use_yield }}
+              run: |
+                  sed -i -e "s/'use_yield' => false/'use_yield' => true/" extra/${{ matrix.extension }}/vendor/twig/twig/src/Environment.php
+
             - name: "Run tests for ${{ matrix.extension}}"
               working-directory: ${{ matrix.extension}}
-              run: ../../vendor/bin/simple-phpunit
+
+            - name: "Run tests"
+              working-directory: extra/${{ matrix.extension }}
+              run: SYMFONY_DEPRECATIONS_HELPER=ignoreFile=../../tests/ignore-use-yield-deprecations ../../vendor/bin/simple-phpunit
 
     integration-tests:
         needs:

diff --git a/CHANGELOG b/CHANGELOG
@@ -1,5 +1,8 @@
-# 3.9.0 (2023-XX-XX)
+# 3.9.0 (2024-XX-XX)
 
+ * Add a new "yield" mode for output generation
+   The "use_yield" Environment option controls the output strategy: use "false" for "echo", "true" for "yield"
+   "yield" will be the only strategy supported in the next major version
  * Add return type for Symfony 7 compatibility
  * Fix premature loop exit in Security Policy lookup of allowed methods/properties
  * Deprecate all internal extension functions in favor of methods on the extension classes

diff --git a/extra/twig-extra-bundle/Tests/Fixture/Kernel.php b/extra/twig-extra-bundle/Tests/Fixture/Kernel.php
@@ -24,11 +24,17 @@ public function registerBundles(): iterable
 
     protected function configureContainer(ContainerBuilder $c, LoaderInterface $loader): void
     {
-        $c->loadFromExtension('framework', [
+        $config = [
             'secret' => 'S3CRET',
             'test' => true,
-        ]);
-
+            'router' => ['utf8' => true],
+            'http_method_override' => false,
+        ];
+        if (6 === Kernel::MAJOR_VERSION) {
+            $config['handle_all_throwables'] = true;
+            $config['php_errors']['log'] = true;
+        }
+        $c->loadFromExtension('framework', $config);
         $c->loadFromExtension('twig', [
             'default_path' => __DIR__.'/views',
         ]);

diff --git a/src/Compiler.php b/src/Compiler.php
@@ -27,10 +27,12 @@ class Compiler
     private $sourceOffset;
     private $sourceLine;
     private $varNameSalt = 0;
+    private $checkForOutput;
 
     public function __construct(Environment $env)
     {
         $this->env = $env;
+        $this->checkForOutput = $env->isDebug();
     }
 
     public function getEnvironment(): Environment
@@ -85,13 +87,26 @@ public function subcompile(Node $node, bool $raw = true)
         return $this;
     }
 
+    /**
+     * @return $this
+     */
+    public function checkForOutput(bool $checkForOutput)
+    {
+        $this->checkForOutput = $checkForOutput ? $this->env->isDebug() : false;
+
+        return $this;
+    }
+
     /**
      * Adds a raw string to the compiled code.
      *
      * @return $this
      */
     public function raw(string $string)
     {
+        if ($this->checkForOutput) {
+            $this->checkStringForOutput(trim($string));
+        }
         $this->source .= $string;
 
         return $this;
@@ -105,6 +120,10 @@ public function raw(string $string)
     public function write(...$strings)
     {
         foreach ($strings as $string) {
+            if ($this->checkForOutput) {
+                $this->checkStringForOutput(trim($string));
+            }
+
             $this->source .= str_repeat(' ', $this->indentation * 4).$string;
         }
 
@@ -220,4 +239,13 @@ public function getVarName(): string
     {
         return sprintf('__internal_compile_%d', $this->varNameSalt++);
     }
+
+    private function checkStringForOutput(string $string): void
+    {
+        if (str_starts_with($string, 'echo')) {
+            trigger_deprecation('twig/twig', '3.9.0', 'Using "echo" in a "Node::compile()" method is deprecated; use a "TextNode" or "PrintNode" instead or use "yield" when "use_yield" is "true" on the environment (triggered by "%s").', $string);
+        } elseif (str_starts_with($string, 'print')) {
+            trigger_deprecation('twig/twig', '3.9.0', 'Using "print" in a "Node::compile()" method is deprecated; use a "TextNode" or "PrintNode" instead or use "yield" when "use_yield" is "true" on the environment (triggered by "%s").', $string);
+        }
+    }
 }
diff --git a/src/Environment.php b/src/Environment.php
@@ -66,6 +66,8 @@ class Environment
     private $runtimeLoaders = [];
     private $runtimes = [];
     private $optionsHash;
+    /** @var bool */
+    private $useYield;
 
     /**
      * Constructor.
@@ -97,6 +99,10 @@ class Environment
      *  * optimizations: A flag that indicates which optimizations to apply
      *                   (default to -1 which means that all optimizations are enabled;
      *                   set it to 0 to disable).
+     *
+     *  * use_yield: Enable a new mode where template are using "yield" instead of "echo"
+     *               (default to "false", but switch it to "true" when possible
+     *               as this will be the only supported mode in Twig 4.0)
      */
     public function __construct(LoaderInterface $loader, $options = [])
     {
@@ -110,8 +116,14 @@ public function __construct(LoaderInterface $loader, $options = [])
             'cache' => false,
             'auto_reload' => null,
             'optimizations' => -1,
+            'use_yield' => false,
         ], $options);
 
+        $this->useYield = (bool) $options['use_yield'];
+        if (!$this->useYield) {
+            trigger_deprecation('twig/twig', '3.9.0', 'Not setting "use_yield" to "true" is deprecated.');
+        }
+
         $this->debug = (bool) $options['debug'];
         $this->setCharset($options['charset'] ?? 'UTF-8');
         $this->autoReload = null === $options['auto_reload'] ? $this->debug : (bool) $options['auto_reload'];
@@ -124,6 +136,14 @@ public function __construct(LoaderInterface $loader, $options = [])
         $this->addExtension(new OptimizerExtension($options['optimizations']));
     }
 
+    /**
+     * @internal
+     */
+    public function useYield(): bool
+    {
+        return $this->useYield;
+    }
+
     /**
      * Enables debugging mode.
      */
@@ -834,6 +854,7 @@ private function updateOptionsHash(): void
             self::VERSION,
             (int) $this->debug,
             (int) $this->strictVariables,
+            $this->useYield ? '1' : '0',
         ]);
     }
 }
diff --git a/src/Node/BlockNode.php b/src/Node/BlockNode.php
@@ -37,6 +37,14 @@ public function compile(Compiler $compiler): void
 
         $compiler
             ->subcompile($this->getNode('body'))
+        ;
+
+        if (!$this->getNode('body') instanceof NodeOutputInterface && $compiler->getEnvironment()->useYield()) {
+            // needed when body doesn't yield anything
+            $compiler->write("yield '';\n");
+        }
+
+        $compiler
             ->outdent()
             ->write("}\n\n")
         ;

diff --git a/src/Node/BlockReferenceNode.php b/src/Node/BlockReferenceNode.php
@@ -28,9 +28,16 @@ public function __construct(string $name, int $lineno, string $tag = null)
 
     public function compile(Compiler $compiler): void
     {
-        $compiler
-            ->addDebugInfo($this)
-            ->write(sprintf("\$this->displayBlock('%s', \$context, \$blocks);\n", $this->getAttribute('name')))
-        ;
+        if ($compiler->getEnvironment()->useYield()) {
+            $compiler
+                ->addDebugInfo($this)
+                ->write(sprintf("yield from \$this->unwrap()->yieldBlock('%s', \$context, \$blocks);\n", $this->getAttribute('name')))
+            ;
+        } else {
+            $compiler
+                ->addDebugInfo($this)
+                ->write(sprintf("\$this->displayBlock('%s', \$context, \$blocks);\n", $this->getAttribute('name')))
+            ;
+        }
     }
 }
diff --git a/src/Node/CaptureNode.php b/src/Node/CaptureNode.php
@@ -27,6 +27,31 @@ public function __construct(Node $body, int $lineno, string $tag = null)
 
     public function compile(Compiler $compiler): void
     {
+        if ($compiler->getEnvironment()->useYield()) {
+            if ($this->getAttribute('raw')) {
+                $compiler->raw("implode('', iterator_to_array(");
+            } else {
+                $compiler->raw("('' === \$tmp = implode('', iterator_to_array(");
+            }
+            if ($this->getAttribute('with_blocks')) {
+                $compiler->raw("(function () use (&\$context, \$macros, \$blocks) {\n");
+            } else {
+                $compiler->raw("(function () use (&\$context, \$macros) {\n");
+            }
+            $compiler
+                ->indent()
+                ->subcompile($this->getNode('body'))
+                ->outdent()
+                ->write("})() ?? new \EmptyIterator()))")
+            ;
+            if (!$this->getAttribute('raw')) {
+                $compiler->raw(") ? '' : new Markup(\$tmp, \$this->env->getCharset())");
+            }
+            $compiler->raw(";");
+
+            return;
+        }
+
         if ($this->getAttribute('with_blocks')) {
             $compiler->raw("(function () use (&\$context, \$macros, \$blocks) {\n");
         } else {

diff --git a/src/Node/Expression/BlockReferenceExpression.php b/src/Node/Expression/BlockReferenceExpression.php
@@ -40,9 +40,16 @@ public function compile(Compiler $compiler): void
             if ($this->getAttribute('output')) {
                 $compiler->addDebugInfo($this);
 
-                $this
-                    ->compileTemplateCall($compiler, 'displayBlock')
-                    ->raw(";\n");
+                if ($compiler->getEnvironment()->useYield()) {
+                    $compiler->write('yield from ');
+                    $this
+                        ->compileTemplateCall($compiler, 'yieldBlock')
+                        ->raw(";\n");
+                } else {
+                    $this
+                        ->compileTemplateCall($compiler, 'displayBlock')
+                        ->raw(";\n");
+                }
             } else {
                 $this->compileTemplateCall($compiler, 'renderBlock');
             }
@@ -65,6 +72,10 @@ private function compileTemplateCall(Compiler $compiler, string $method): Compil
             ;
         }
 
+        if ($compiler->getEnvironment()->useYield()) {
+            $compiler->raw('->unwrap()');
+        }
+
         $compiler->raw(sprintf('->%s', $method));
 
         return $this->compileBlockArguments($compiler);

diff --git a/src/Node/Expression/InlinePrint.php b/src/Node/Expression/InlinePrint.php
@@ -26,10 +26,19 @@ public function __construct(Node $node, int $lineno)
 
     public function compile(Compiler $compiler): void
     {
-        $compiler
-            ->raw('print (')
-            ->subcompile($this->getNode('node'))
-            ->raw(')')
-        ;
+        if ($compiler->getEnvironment()->useYield()) {
+            $compiler
+                ->raw('yield ')
+                ->subcompile($this->getNode('node'))
+            ;
+        } else {
+            $compiler
+                ->checkForOutput(false)
+                ->raw('print(')
+                ->checkForOutput(true)
+                ->subcompile($this->getNode('node'))
+                ->raw(')')
+            ;
+        }
     }
 }