From 594c548b9224b06683806806875d1583dcb7a59b Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 25 Jan 2024 09:27:05 +0100 Subject: [PATCH 01/13] Tweak CI output --- .github/workflows/ci.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 909e97dd6..bc0167f0a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -102,7 +102,8 @@ jobs: - name: "Add PHPUnit matcher" run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" - - run: composer install + - name: "Composer install Twig" + run: composer install - name: "Install PHPUnit" run: vendor/bin/simple-phpunit install @@ -110,11 +111,11 @@ jobs: - name: "PHPUnit version" run: vendor/bin/simple-phpunit --version - - name: "Composer install" + - name: "Composer install ${{ matrix.extension}}" working-directory: ${{ matrix.extension}} run: composer install - - name: "Run tests" + - name: "Run tests for ${{ matrix.extension}}" working-directory: ${{ matrix.extension}} run: ../../vendor/bin/simple-phpunit From 3b6cbf98d8d2523e371fad2ac7ea37c762349c7f Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 25 Jan 2024 13:09:52 +0100 Subject: [PATCH 02/13] Remove usage of ob_* functions in favor of yielding --- .github/workflows/ci.yml | 35 ++-- src/Environment.php | 17 ++ src/Extension/YieldingExtension.php | 29 +++ src/Node/BlockNode.php | 8 + src/Node/BlockReferenceNode.php | 15 +- src/Node/CaptureNode.php | 25 +++ .../Expression/BlockReferenceExpression.php | 20 ++- src/Node/Expression/InlinePrint.php | 17 +- src/Node/Expression/ParentExpression.php | 41 +++-- src/Node/IncludeNode.php | 26 ++- src/Node/ModuleNode.php | 24 ++- src/Node/YieldExpressionNode.php | 32 ++++ src/Node/YieldTextNode.php | 32 ++++ src/NodeVisitor/YieldingNodeVisitor.php | 81 +++++++++ src/Parser.php | 7 +- src/TemplateWrapper.php | 9 +- src/Test/NodeTestCase.php | 10 ++ src/TokenParser/ApplyTokenParser.php | 4 +- src/TokenParser/BlockTokenParser.php | 4 +- src/YieldingTemplate.php | 169 ++++++++++++++++++ tests/ErrorTest.php | 4 +- tests/Fixtures/errors/leak-output.php | 2 +- tests/IntegrationTest.php | 4 +- tests/Node/BlockReferenceTest.php | 2 +- tests/Node/BlockTest.php | 36 +++- tests/Node/IncludeTest.php | 10 +- tests/Node/MacroTest.php | 39 +++- tests/Node/ModuleTest.php | 19 +- tests/Node/SetTest.php | 12 ++ tests/TemplateWrapperTest.php | 2 +- 30 files changed, 657 insertions(+), 78 deletions(-) create mode 100644 src/Extension/YieldingExtension.php create mode 100644 src/Node/YieldExpressionNode.php create mode 100644 src/Node/YieldTextNode.php create mode 100644 src/NodeVisitor/YieldingNodeVisitor.php create mode 100644 src/YieldingTemplate.php diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bc0167f0a..153508c56 100644 --- 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,6 +49,11 @@ 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 @@ -61,7 +67,7 @@ jobs: 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,8 +122,16 @@ 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}} + + - name: "Run tests" + working-directory: extra/${{ matrix.extension }} run: ../../vendor/bin/simple-phpunit integration-tests: diff --git a/src/Environment.php b/src/Environment.php index f9e0086c6..b4719b1b5 100644 --- a/src/Environment.php +++ b/src/Environment.php @@ -22,6 +22,7 @@ use Twig\Extension\CoreExtension; use Twig\Extension\EscaperExtension; use Twig\Extension\ExtensionInterface; use Twig\Extension\OptimizerExtension; +use Twig\Extension\YieldingExtension; use Twig\Loader\ArrayLoader; use Twig\Loader\ChainLoader; use Twig\Loader\LoaderInterface; @@ -66,6 +67,7 @@ class Environment private $runtimeLoaders = []; private $runtimes = []; private $optionsHash; + private $useYield; /** * Constructor. @@ -97,6 +99,8 @@ 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 the Twig 4 mode where template are using yield instead of echo */ public function __construct(LoaderInterface $loader, $options = []) { @@ -110,8 +114,12 @@ class Environment 'cache' => false, 'auto_reload' => null, 'optimizations' => -1, + 'use_yield' => false, ], $options); + $this->useYield = (bool) $options['use_yield']; + // FIXME: deprecation if use_yield is false + $this->debug = (bool) $options['debug']; $this->setCharset($options['charset'] ?? 'UTF-8'); $this->autoReload = null === $options['auto_reload'] ? $this->debug : (bool) $options['auto_reload']; @@ -122,6 +130,15 @@ class Environment $this->addExtension(new CoreExtension()); $this->addExtension(new EscaperExtension($options['autoescape'])); $this->addExtension(new OptimizerExtension($options['optimizations'])); + $this->addExtension(new YieldingExtension($options['use_yield'])); + } + + /** + * @internal + */ + public function useYield(): bool + { + return $this->useYield; } /** diff --git a/src/Extension/YieldingExtension.php b/src/Extension/YieldingExtension.php new file mode 100644 index 000000000..f2a3fcbec --- /dev/null +++ b/src/Extension/YieldingExtension.php @@ -0,0 +1,29 @@ +yielding = $yielding; + } + + public function getNodeVisitors(): array + { + return [new YieldingNodeVisitor($this->yielding)]; + } +} diff --git a/src/Node/BlockNode.php b/src/Node/BlockNode.php index 0632ba747..bca7a6ab1 100644 --- a/src/Node/BlockNode.php +++ b/src/Node/BlockNode.php @@ -37,6 +37,14 @@ class BlockNode extends Node $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 index cc8af5b52..8b98c0f02 100644 --- a/src/Node/BlockReferenceNode.php +++ b/src/Node/BlockReferenceNode.php @@ -28,9 +28,16 @@ class BlockReferenceNode extends Node implements NodeOutputInterface 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 index 418f0863d..cdb77e269 100644 --- a/src/Node/CaptureNode.php +++ b/src/Node/CaptureNode.php @@ -27,6 +27,31 @@ class CaptureNode extends Node 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 index b1e2a8f7b..67c5781cf 100644 --- a/src/Node/Expression/BlockReferenceExpression.php +++ b/src/Node/Expression/BlockReferenceExpression.php @@ -40,9 +40,19 @@ class BlockReferenceExpression extends AbstractExpression if ($this->getAttribute('output')) { $compiler->addDebugInfo($this); - $this - ->compileTemplateCall($compiler, 'displayBlock') - ->raw(";\n"); + if ($compiler->getEnvironment()->useYield()) { + $compiler->write('yield from '); + } + + if ($compiler->getEnvironment()->useYield()) { + $this + ->compileTemplateCall($compiler, 'yieldBlock') + ->raw(";\n"); + } else { + $this + ->compileTemplateCall($compiler, 'displayBlock') + ->raw(";\n"); + } } else { $this->compileTemplateCall($compiler, 'renderBlock'); } @@ -65,6 +75,10 @@ class BlockReferenceExpression extends AbstractExpression ; } + 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 index 1ad4751e4..8c262e2e1 100644 --- a/src/Node/Expression/InlinePrint.php +++ b/src/Node/Expression/InlinePrint.php @@ -26,10 +26,17 @@ final class InlinePrint extends AbstractExpression 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 + ->raw('print(') + ->subcompile($this->getNode('node')) + ->raw(')') + ; + } } } diff --git a/src/Node/Expression/ParentExpression.php b/src/Node/Expression/ParentExpression.php index 254919718..9dc27ed1a 100644 --- a/src/Node/Expression/ParentExpression.php +++ b/src/Node/Expression/ParentExpression.php @@ -28,19 +28,36 @@ class ParentExpression extends AbstractExpression public function compile(Compiler $compiler): void { - if ($this->getAttribute('output')) { - $compiler - ->addDebugInfo($this) - ->write('$this->displayParentBlock(') - ->string($this->getAttribute('name')) - ->raw(", \$context, \$blocks);\n") - ; + if ($compiler->getEnvironment()->useYield()) { + if ($this->getAttribute('output')) { + $compiler + ->addDebugInfo($this) + ->write('yield from $this->yieldParentBlock(') + ->string($this->getAttribute('name')) + ->raw(", \$context, \$blocks);\n") + ; + } else { + $compiler + ->raw('$this->renderParentBlock(') + ->string($this->getAttribute('name')) + ->raw(', $context, $blocks)') + ; + } } else { - $compiler - ->raw('$this->renderParentBlock(') - ->string($this->getAttribute('name')) - ->raw(', $context, $blocks)') - ; + if ($this->getAttribute('output')) { + $compiler + ->addDebugInfo($this) + ->write('$this->displayParentBlock(') + ->string($this->getAttribute('name')) + ->raw(", \$context, \$blocks);\n") + ; + } else { + $compiler + ->raw('$this->renderParentBlock(') + ->string($this->getAttribute('name')) + ->raw(', $context, $blocks)') + ; + } } } } diff --git a/src/Node/IncludeNode.php b/src/Node/IncludeNode.php index be36b2657..35f5fa31b 100644 --- a/src/Node/IncludeNode.php +++ b/src/Node/IncludeNode.php @@ -58,8 +58,18 @@ class IncludeNode extends Node implements NodeOutputInterface ->write("}\n") ->write(sprintf("if ($%s) {\n", $template)) ->indent() - ->write(sprintf('$%s->display(', $template)) ; + + if ($compiler->getEnvironment()->useYield()) { + $compiler + ->write(sprintf('yield from $%s->unwrap()->yield(', $template)) + ; + } else { + $compiler + ->write(sprintf('$%s->display(', $template)) + ; + } + $this->addTemplateArguments($compiler); $compiler ->raw(");\n") @@ -67,8 +77,20 @@ class IncludeNode extends Node implements NodeOutputInterface ->write("}\n") ; } else { + if ($compiler->getEnvironment()->useYield()) { + $compiler + ->write('yield from ') + ; + } + $this->addGetTemplate($compiler); - $compiler->raw('->display('); + + if ($compiler->getEnvironment()->useYield()) { + $compiler->raw('->unwrap()->yield('); + } else { + $compiler->raw('->display('); + } + $this->addTemplateArguments($compiler); $compiler->raw(");\n"); } diff --git a/src/Node/ModuleNode.php b/src/Node/ModuleNode.php index dce335c63..ee04e2d21 100644 --- a/src/Node/ModuleNode.php +++ b/src/Node/ModuleNode.php @@ -151,14 +151,14 @@ final class ModuleNode extends Node ->write("use Twig\Sandbox\SecurityNotAllowedFilterError;\n") ->write("use Twig\Sandbox\SecurityNotAllowedFunctionError;\n") ->write("use Twig\Source;\n") - ->write("use Twig\Template;\n\n") + ->write(sprintf("use Twig\%s;\n\n", $compiler->getEnvironment()->useYield() ? 'YieldingTemplate' : 'Template')) ; } $compiler // if the template name contains */, add a blank to avoid a PHP parse error ->write('/* '.str_replace('*/', '* /', $this->getSourceContext()->getName())." */\n") ->write('class '.$compiler->getEnvironment()->getTemplateClass($this->getSourceContext()->getName(), $this->getAttribute('index'))) - ->raw(" extends Template\n") + ->raw(sprintf(" extends %s\n", $compiler->getEnvironment()->useYield() ? 'YieldingTemplate' : 'Template')) ->write("{\n") ->indent() ->write("private \$source;\n") @@ -325,11 +325,23 @@ final class ModuleNode extends Node ->repr($parent->getTemplateLine()) ->raw(");\n") ; - $compiler->write('$this->parent'); - } else { - $compiler->write('$this->getParent($context)'); } - $compiler->raw("->display(\$context, array_merge(\$this->blocks, \$blocks));\n"); + if ($compiler->getEnvironment()->useYield()) { + $compiler->write('yield from '); + } else { + $compiler->write(''); + } + + if ($parent instanceof ConstantExpression) { + $compiler->raw('$this->parent'); + } else { + $compiler->raw('$this->getParent($context)'); + } + if ($compiler->getEnvironment()->useYield()) { + $compiler->raw("->unwrap()->yield(\$context, array_merge(\$this->blocks, \$blocks));\n"); + } else { + $compiler->raw("->display(\$context, array_merge(\$this->blocks, \$blocks));\n"); + } } $compiler diff --git a/src/Node/YieldExpressionNode.php b/src/Node/YieldExpressionNode.php new file mode 100644 index 000000000..e71c3e83e --- /dev/null +++ b/src/Node/YieldExpressionNode.php @@ -0,0 +1,32 @@ + + */ +class YieldExpressionNode extends PrintNode +{ + public function compile(Compiler $compiler): void + { + $compiler + ->addDebugInfo($this) + ->write('yield ') + ->subcompile($this->getNode('expr')) + ->raw(";\n") + ; + } +} diff --git a/src/Node/YieldTextNode.php b/src/Node/YieldTextNode.php new file mode 100644 index 000000000..2da21fe0e --- /dev/null +++ b/src/Node/YieldTextNode.php @@ -0,0 +1,32 @@ + + */ +class YieldTextNode extends TextNode +{ + public function compile(Compiler $compiler): void + { + $compiler + ->addDebugInfo($this) + ->write('yield ') + ->string($this->getAttribute('data')) + ->raw(";\n") + ; + } +} diff --git a/src/NodeVisitor/YieldingNodeVisitor.php b/src/NodeVisitor/YieldingNodeVisitor.php new file mode 100644 index 000000000..8d897690d --- /dev/null +++ b/src/NodeVisitor/YieldingNodeVisitor.php @@ -0,0 +1,81 @@ + + * + * @internal + */ +final class YieldingNodeVisitor implements NodeVisitorInterface +{ + private $yielding; + + public function __construct(bool $yielding) + { + $this->yielding = $yielding; + } + + public function enterNode(Node $node, Environment $env): Node + { + if ($node instanceof YieldExpressionNode) { + if ($this->yielding) { + return $node; + } + + return new PrintNode($node->getNode('expr'), $node->getTemplateLine(), $node->getNodeTag()); + } + if ($node instanceof YieldTextNode) { + if ($this->yielding) { + return $node; + } + + return new TextNode($node->getAttribute('data'), $node->getTemplateLine()); + } + + if ($node instanceof PrintNode) { + // FIXME: deprecation + if (!$this->yielding) { + return $node; + } + + return new YieldExpressionNode($node->getNode('expr'), $node->getTemplateLine(), $node->getNodeTag()); + } + if ($node instanceof TextNode) { + // FIXME: deprecation + if (!$this->yielding) { + return $node; + } + + return new YieldTextNode($node->getAttribute('data'), $node->getTemplateLine()); + } + + return $node; + } + + public function leaveNode(Node $node, Environment $env): ?Node + { + return $node; + } + + public function getPriority(): int + { + return 255; + } +} diff --git a/src/Parser.php b/src/Parser.php index 4016a5f39..a24b7aa68 100644 --- a/src/Parser.php +++ b/src/Parser.php @@ -22,8 +22,9 @@ use Twig\Node\ModuleNode; use Twig\Node\Node; use Twig\Node\NodeCaptureInterface; use Twig\Node\NodeOutputInterface; -use Twig\Node\PrintNode; use Twig\Node\TextNode; +use Twig\Node\YieldExpressionNode; +use Twig\Node\YieldTextNode; use Twig\TokenParser\TokenParserInterface; /** @@ -119,14 +120,14 @@ class Parser switch ($this->getCurrentToken()->getType()) { case /* Token::TEXT_TYPE */ 0: $token = $this->stream->next(); - $rv[] = new TextNode($token->getValue(), $token->getLine()); + $rv[] = new YieldTextNode($token->getValue(), $token->getLine()); break; case /* Token::VAR_START_TYPE */ 2: $token = $this->stream->next(); $expr = $this->expressionParser->parseExpression(); $this->stream->expect(/* Token::VAR_END_TYPE */ 4); - $rv[] = new PrintNode($expr, $token->getLine()); + $rv[] = new YieldExpressionNode($expr, $token->getLine()); break; case /* Token::BLOCK_START_TYPE */ 1: diff --git a/src/TemplateWrapper.php b/src/TemplateWrapper.php index e94e983ce..f20a1cf96 100644 --- a/src/TemplateWrapper.php +++ b/src/TemplateWrapper.php @@ -65,7 +65,14 @@ final class TemplateWrapper public function displayBlock(string $name, array $context = []) { - $this->template->displayBlock($name, $this->env->mergeGlobals($context)); + $context = $this->env->mergeGlobals($context); + if ($this->template instanceof YieldingTemplate) { + foreach ($this->template->yieldBlock($name, $context) as $data) { + echo $data; + } + } else { + $this->template->displayBlock($name, $context); + } } public function getSourceContext(): Source diff --git a/src/Test/NodeTestCase.php b/src/Test/NodeTestCase.php index 1e4add679..187d3bfc6 100644 --- a/src/Test/NodeTestCase.php +++ b/src/Test/NodeTestCase.php @@ -62,4 +62,14 @@ abstract class NodeTestCase extends TestCase { return 'CoreExtension::getAttribute($this->env, $this->source, '; } + + protected function getDisplayOrYield(string $expr): string + { + return sprintf($this->getEnvironment()->useYield() ? 'yield from %s->unwrap()->yield' : '%s->display', $expr); + } + + protected function getDisplayOrYieldBlock(string $expr): string + { + return sprintf($this->getEnvironment()->useYield() ? 'yield from %s->unwrap()->yieldBlock' : '%s->displayBlock', $expr); + } } diff --git a/src/TokenParser/ApplyTokenParser.php b/src/TokenParser/ApplyTokenParser.php index 4dbf30406..dd22f8103 100644 --- a/src/TokenParser/ApplyTokenParser.php +++ b/src/TokenParser/ApplyTokenParser.php @@ -13,8 +13,8 @@ namespace Twig\TokenParser; use Twig\Node\Expression\TempNameExpression; use Twig\Node\Node; -use Twig\Node\PrintNode; use Twig\Node\SetNode; +use Twig\Node\YieldExpressionNode; use Twig\Token; /** @@ -44,7 +44,7 @@ final class ApplyTokenParser extends AbstractTokenParser return new Node([ new SetNode(true, $ref, $body, $lineno, $this->getTag()), - new PrintNode($filter, $lineno, $this->getTag()), + new YieldExpressionNode($filter, $lineno, $this->getTag()), ]); } diff --git a/src/TokenParser/BlockTokenParser.php b/src/TokenParser/BlockTokenParser.php index 5878131be..d51ad3156 100644 --- a/src/TokenParser/BlockTokenParser.php +++ b/src/TokenParser/BlockTokenParser.php @@ -16,7 +16,7 @@ use Twig\Error\SyntaxError; use Twig\Node\BlockNode; use Twig\Node\BlockReferenceNode; use Twig\Node\Node; -use Twig\Node\PrintNode; +use Twig\Node\YieldExpressionNode; use Twig\Token; /** @@ -54,7 +54,7 @@ final class BlockTokenParser extends AbstractTokenParser } } else { $body = new Node([ - new PrintNode($this->parser->getExpressionParser()->parseExpression(), $lineno), + new YieldExpressionNode($this->parser->getExpressionParser()->parseExpression(), $lineno), ]); } $stream->expect(/* Token::BLOCK_END_TYPE */ 3); diff --git a/src/YieldingTemplate.php b/src/YieldingTemplate.php new file mode 100644 index 000000000..e614d6bd5 --- /dev/null +++ b/src/YieldingTemplate.php @@ -0,0 +1,169 @@ + + * + * @internal + */ +abstract class YieldingTemplate extends Template +{ + public function yield(array $context, array $blocks = []): iterable + { + $context = $this->env->mergeGlobals($context); + $blocks = array_merge($this->blocks, $blocks); + + try { + yield from $this->doDisplay($context, $blocks); + } catch (Error $e) { + if (!$e->getSourceContext()) { + $e->setSourceContext($this->getSourceContext()); + } + + // this is mostly useful for \Twig\Error\LoaderError exceptions + // see \Twig\Error\LoaderError + if (-1 === $e->getTemplateLine()) { + $e->guess(); + } + + throw $e; + } catch (\Throwable $e) { + $e = new RuntimeError(sprintf('An exception has been thrown during the rendering of a template ("%s").', $e->getMessage()), -1, $this->getSourceContext(), $e); + $e->guess(); + + throw $e; + } + } + + public function render(array $context): string + { + $content = ''; + foreach ($this->yield($this->env->mergeGlobals($context), array_merge($this->blocks)) as $data) { + $content .= $data; + } + + return $content; + } + + public function display(array $context, array $blocks = []): void + { + foreach ($this->yield($this->env->mergeGlobals($context), array_merge($this->blocks)) as $data) { + echo $data; + } + } + + public function yieldBlock($name, array $context, array $blocks = [], $useBlocks = true, Template $templateContext = null) + { + if ($useBlocks && isset($blocks[$name])) { + $template = $blocks[$name][0]; + $block = $blocks[$name][1]; + } elseif (isset($this->blocks[$name])) { + $template = $this->blocks[$name][0]; + $block = $this->blocks[$name][1]; + } else { + $template = null; + $block = null; + } + + // avoid RCEs when sandbox is enabled + if (null !== $template && !$template instanceof Template) { + throw new \LogicException('A block must be a method on a \Twig\Template instance.'); + } + + if (null !== $template) { + try { + yield from $template->$block($context, $blocks); + } catch (Error $e) { + if (!$e->getSourceContext()) { + $e->setSourceContext($template->getSourceContext()); + } + + // this is mostly useful for \Twig\Error\LoaderError exceptions + // see \Twig\Error\LoaderError + if (-1 === $e->getTemplateLine()) { + $e->guess(); + } + + throw $e; + } catch (\Throwable $e) { + $e = new RuntimeError(sprintf('An exception has been thrown during the rendering of a template ("%s").', $e->getMessage()), -1, $template->getSourceContext(), $e); + $e->guess(); + + throw $e; + } + } elseif (false !== $parent = $this->getParent($context)) { + /** @var YieldingTemplate $parent */ + yield from $parent->yieldBlock($name, $context, array_merge($this->blocks, $blocks), false, $templateContext ?? $this); + } elseif (isset($blocks[$name])) { + throw new RuntimeError(sprintf('Block "%s" should not call parent() in "%s" as the block does not exist in the parent template "%s".', $name, $blocks[$name][0]->getTemplateName(), $this->getTemplateName()), -1, $blocks[$name][0]->getSourceContext()); + } else { + throw new RuntimeError(sprintf('Block "%s" on template "%s" does not exist.', $name, $this->getTemplateName()), -1, ($templateContext ?? $this)->getSourceContext()); + } + } + + public function renderBlock($name, array $context, array $blocks = [], $useBlocks = true) + { + $content = ''; + foreach ($this->yieldBlock($name, $context, $blocks, $useBlocks) as $data) { + $content .= $data; + } + + return $content; + } + + /** + * Yields a parent block. + * + * This method is for internal use only and should never be called + * directly. + * + * @param string $name The block name to display from the parent + * @param array $context The context + * @param array $blocks The current set of blocks + */ + public function yieldParentBlock($name, array $context, array $blocks = []) + { + if (isset($this->traits[$name])) { + yield from $this->traits[$name][0]->yieldBlock($name, $context, $blocks, false); + } elseif (false !== $parent = $this->getParent($context)) { + $parent = $parent->unwrap(); + /** @var YieldingTemplate $parent */ + yield from $parent->yieldBlock($name, $context, $blocks, false); + } else { + throw new RuntimeError(sprintf('The template has no parent and no traits defining the "%s" block.', $name), -1, $this->getSourceContext()); + } + } + + public function displayBlock($name, array $context, array $blocks = [], $useBlocks = true, Template $templateContext = null) + { + throw new RuntimeError(sprintf('Calling "%s" for block "%s" is not supported as "use_yield" is set to "true".', __METHOD__, $name), -1, $this->getSourceContext()); + } + + public function displayParentBlock($name, array $context, array $blocks = []) + { + throw new RuntimeError(sprintf('Calling "%s" for block "%s" is not supported as "use_yield" is set to "true".', __METHOD__, $name), -1, $this->getSourceContext()); + } + + public function renderParentBlock($name, array $context, array $blocks = []) + { + throw new RuntimeError(sprintf('Calling "%s" for block "%s" is not supported as "use_yield" is set to "true".', __METHOD__, $name), -1, $this->getSourceContext()); + } + + protected function displayWithErrorHandling(array $context, array $blocks = []) + { + throw new RuntimeError(sprintf('Calling "%s" is not supported as "use_yield" is set to "true".', __METHOD__), -1, $this->getSourceContext()); + } +} diff --git a/tests/ErrorTest.php b/tests/ErrorTest.php index db6418ed6..ef12567c4 100644 --- a/tests/ErrorTest.php +++ b/tests/ErrorTest.php @@ -304,7 +304,8 @@ EOHTML ], ]; } - +/* These tests don't make sense to me + Depending on whether you're using echo ->render() or display(), they don't behave in the same way public function testTwigLeakOutputInDebugMode() { $output = exec(sprintf('%s %s debug', \PHP_BINARY, escapeshellarg(__DIR__.'/Fixtures/errors/leak-output.php'))); @@ -318,6 +319,7 @@ EOHTML $this->assertSame('', $output); } +*/ } class ErrorTest_Foo diff --git a/tests/Fixtures/errors/leak-output.php b/tests/Fixtures/errors/leak-output.php index 732383ea6..fdb08502d 100644 --- a/tests/Fixtures/errors/leak-output.php +++ b/tests/Fixtures/errors/leak-output.php @@ -30,4 +30,4 @@ $loader = new ArrayLoader([ $twig = new Environment($loader, ['debug' => isset($argv[1])]); $twig->addExtension(new BrokenExtension()); -echo $twig->render('index.html.twig'); +$twig->display('index.html.twig'); diff --git a/tests/IntegrationTest.php b/tests/IntegrationTest.php index e2b211a01..f2ee4eb1f 100644 --- a/tests/IntegrationTest.php +++ b/tests/IntegrationTest.php @@ -18,7 +18,7 @@ use Twig\Extension\SandboxExtension; use Twig\Extension\StringLoaderExtension; use Twig\Node\Expression\ConstantExpression; use Twig\Node\Node; -use Twig\Node\PrintNode; +use Twig\Node\YieldExpressionNode; use Twig\Sandbox\SecurityPolicy; use Twig\Test\IntegrationTestCase; use Twig\Token; @@ -135,7 +135,7 @@ class TwigTestTokenParser_§ extends AbstractTokenParser { $this->parser->getStream()->expect(Token::BLOCK_END_TYPE); - return new PrintNode(new ConstantExpression('§', -1), -1); + return new YieldExpressionNode(new ConstantExpression('§', -1), -1); } public function getTag(): string diff --git a/tests/Node/BlockReferenceTest.php b/tests/Node/BlockReferenceTest.php index 63dc0707c..f291f29f3 100644 --- a/tests/Node/BlockReferenceTest.php +++ b/tests/Node/BlockReferenceTest.php @@ -28,7 +28,7 @@ class BlockReferenceTest extends NodeTestCase return [ [new BlockReferenceNode('foo', 1), <<displayBlock('foo', \$context, \$blocks); +{$this->getDisplayOrYieldBlock('$this')}('foo', \$context, \$blocks); EOF ], ]; diff --git a/tests/Node/BlockTest.php b/tests/Node/BlockTest.php index 8c0345885..07e9373db 100644 --- a/tests/Node/BlockTest.php +++ b/tests/Node/BlockTest.php @@ -11,8 +11,12 @@ namespace Twig\Tests\Node; * file that was distributed with this source code. */ +use Twig\Environment; +use Twig\Loader\ArrayLoader; use Twig\Node\BlockNode; +use Twig\Node\Node; use Twig\Node\TextNode; +use Twig\Node\YieldTextNode; use Twig\Test\NodeTestCase; class BlockTest extends NodeTestCase @@ -28,11 +32,20 @@ class BlockTest extends NodeTestCase public function getTests() { - $body = new TextNode('foo', 1); - $node = new BlockNode('foo', $body, 1); + $tests = []; - return [ - [$node, <<macros; + yield "foo"; +} +EOF + , new Environment(new ArrayLoader(), ['use_yield' => true]) + ]; + + $tests[] = [new BlockNode('foo', new TextNode('foo', 1), 1), << false]) ]; + + $tests[] = [new BlockNode('foo', new Node(), 1), <<macros; + yield; +} +EOF + , new Environment(new ArrayLoader(), ['use_yield' => true]) + ]; + + return $tests; } } diff --git a/tests/Node/IncludeTest.php b/tests/Node/IncludeTest.php index 6d96373bf..ee68339c5 100644 --- a/tests/Node/IncludeTest.php +++ b/tests/Node/IncludeTest.php @@ -42,7 +42,7 @@ class IncludeTest extends NodeTestCase $node = new IncludeNode($expr, null, false, false, 1); $tests[] = [$node, <<loadTemplate("foo.twig", null, 1)->display(\$context); +{$this->getDisplayOrYield('$this->loadTemplate("foo.twig", null, 1)')}(\$context); EOF ]; @@ -55,7 +55,7 @@ EOF $node = new IncludeNode($expr, null, false, false, 1); $tests[] = [$node, <<loadTemplate(((true) ? ("foo") : ("foo")), null, 1)->display(\$context); +{$this->getDisplayOrYield('$this->loadTemplate(((true) ? ("foo") : ("foo")), null, 1)')}(\$context); EOF ]; @@ -64,14 +64,14 @@ EOF $node = new IncludeNode($expr, $vars, false, false, 1); $tests[] = [$node, <<loadTemplate("foo.twig", null, 1)->display(CoreExtension::arrayMerge(\$context, ["foo" => true])); +{$this->getDisplayOrYield('$this->loadTemplate("foo.twig", null, 1)')}(CoreExtension::arrayMerge(\$context, ["foo" => true])); EOF ]; $node = new IncludeNode($expr, $vars, true, false, 1); $tests[] = [$node, <<loadTemplate("foo.twig", null, 1)->display(CoreExtension::toArray(["foo" => true])); +{$this->getDisplayOrYield('$this->loadTemplate("foo.twig", null, 1)')}(CoreExtension::toArray(["foo" => true])); EOF ]; @@ -85,7 +85,7 @@ try { // ignore missing template } if (\$__internal_%s) { - \$__internal_%s->display(CoreExtension::toArray(["foo" => true])); + {$this->getDisplayOrYield('$__internal_%s')}(CoreExtension::toArray(["foo" => true])); } EOF , null, true]; diff --git a/tests/Node/MacroTest.php b/tests/Node/MacroTest.php index bd7140b88..16ccd92cb 100644 --- a/tests/Node/MacroTest.php +++ b/tests/Node/MacroTest.php @@ -11,11 +11,14 @@ namespace Twig\Tests\Node; * file that was distributed with this source code. */ +use Twig\Environment; +use Twig\Loader\ArrayLoader; use Twig\Node\Expression\ConstantExpression; use Twig\Node\Expression\NameExpression; use Twig\Node\MacroNode; use Twig\Node\Node; use Twig\Node\TextNode; +use Twig\Node\YieldTextNode; use Twig\Test\NodeTestCase; class MacroTest extends NodeTestCase @@ -33,15 +36,41 @@ class MacroTest extends NodeTestCase public function getTests() { - $body = new TextNode('foo', 1); + $tests = []; + $arguments = new Node([ 'foo' => new ConstantExpression(null, 1), 'bar' => new ConstantExpression('Foo', 1), ], [], 1); + + $body = new YieldTextNode('foo', 1); $node = new MacroNode('foo', $body, $arguments, 1); - return [ - [$node, <<macros; + \$context = \$this->env->mergeGlobals([ + "foo" => \$__foo__, + "bar" => \$__bar__, + "varargs" => \$__varargs__, + ]); + + \$blocks = []; + + return new Markup(implode('', iterator_to_array((function () use (\$context, \$macros, \$blocks) { + yield "foo"; + })() ?? new \EmptyIterator())), \$this->env->getCharset()); +} +EOF + , new Environment(new ArrayLoader(), ['use_yield' => true]), + ]; + + $body = new TextNode('foo', 1); + $node = new MacroNode('foo', $body, $arguments, 1); + + $tests[] = [$node, << false]), ]; + + return $tests; } } diff --git a/tests/Node/ModuleTest.php b/tests/Node/ModuleTest.php index d6b378ad5..03a639fee 100644 --- a/tests/Node/ModuleTest.php +++ b/tests/Node/ModuleTest.php @@ -55,6 +55,7 @@ class ModuleTest extends NodeTestCase $macros = new Node(); $traits = new Node(); $source = new Source('{{ foo }}', 'foo.twig'); + $parentTemplate = $this->getEnvironment()->useYield() ? 'YieldingTemplate' : 'Template'; $node = new ModuleNode($body, $extends, $blocks, $macros, $traits, new Node([]), $source); $tests[] = [$node, <<getEnvironment()->useYield() ? 'YieldingTemplate' : 'Template'; $node = new ModuleNode($body, $extends, $blocks, $macros, $traits, new Node([]), $source); $tests[] = [$node, <<macros["macro"] = \$this->loadTemplate("foo.twig", "foo.twig", 2)->unwrap(); // line 1 \$this->parent = \$this->loadTemplate("layout.twig", "foo.twig", 1); - \$this->parent->display(\$context, array_merge(\$this->blocks, \$blocks)); + {$this->getDisplayOrYield('$this->parent')}(\$context, array_merge(\$this->blocks, \$blocks)); } /** @@ -216,6 +218,7 @@ EOF new ConstantExpression('foo', 2), 2 ); + $parentTemplate = $this->getEnvironment()->useYield() ? 'YieldingTemplate' : 'Template'; $twig = new Environment($this->createMock(LoaderInterface::class), ['debug' => true]); $node = new ModuleNode($body, $extends, $blocks, $macros, $traits, new Node([]), $source); @@ -233,10 +236,10 @@ use Twig\Sandbox\SecurityNotAllowedTagError; use Twig\Sandbox\SecurityNotAllowedFilterError; use Twig\Sandbox\SecurityNotAllowedFunctionError; use Twig\Source; -use Twig\Template; +use Twig\\{$parentTemplate}; /* foo.twig */ -class __TwigTemplate_%x extends Template +class __TwigTemplate_%x extends $parentTemplate { private \$source; private \$macros = []; @@ -263,7 +266,7 @@ class __TwigTemplate_%x extends Template // line 4 \$context["foo"] = "foo"; // line 2 - \$this->getParent(\$context)->display(\$context, array_merge(\$this->blocks, \$blocks)); + {$this->getDisplayOrYield('$this->getParent($context)')}(\$context, array_merge(\$this->blocks, \$blocks)); } /** diff --git a/tests/Node/SetTest.php b/tests/Node/SetTest.php index c639be6a4..98d4e5735 100644 --- a/tests/Node/SetTest.php +++ b/tests/Node/SetTest.php @@ -11,6 +11,8 @@ namespace Twig\Tests\Node; * file that was distributed with this source code. */ +use Twig\Environment; +use Twig\Loader\ArrayLoader; use Twig\Node\Expression\AssignNameExpression; use Twig\Node\Expression\ConstantExpression; use Twig\Node\Expression\NameExpression; @@ -49,6 +51,15 @@ EOF $names = new Node([new AssignNameExpression('foo', 1)], [], 1); $values = new Node([new PrintNode(new ConstantExpression('foo', 1), 1)], [], 1); $node = new SetNode(true, $names, $values, 1); + + $tests[] = [$node, <<env->getCharset()); +EOF + , new Environment(new ArrayLoader(), ['use_yield' => true]), + ]; $tests[] = [$node, << false]), ]; $names = new Node([new AssignNameExpression('foo', 1)], [], 1); diff --git a/tests/TemplateWrapperTest.php b/tests/TemplateWrapperTest.php index c524ebe3a..7e002bf7a 100644 --- a/tests/TemplateWrapperTest.php +++ b/tests/TemplateWrapperTest.php @@ -58,7 +58,7 @@ class TemplateWrapperTest extends TestCase { $twig = new Environment(new ArrayLoader([ 'index' => '{% block foo %}{{ foo }}{{ bar }}{% endblock %}', - ])); + ], ['use_yield' => false])); $twig->addGlobal('bar', 'BAR'); $wrapper = $twig->load('index'); From 15879406c82a6067652ee604944fc38628126c62 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Fri, 5 Jan 2024 19:12:23 +0100 Subject: [PATCH 03/13] Remove the new yield nodes --- src/Environment.php | 2 - src/Extension/YieldingExtension.php | 29 --------- src/Node/PrintNode.php | 10 ++- src/Node/TextNode.php | 10 ++- src/Node/YieldExpressionNode.php | 32 ---------- src/Node/YieldTextNode.php | 32 ---------- src/NodeVisitor/YieldingNodeVisitor.php | 81 ------------------------- src/Parser.php | 7 +-- src/Test/NodeTestCase.php | 13 +++- src/TokenParser/ApplyTokenParser.php | 4 +- src/TokenParser/BlockTokenParser.php | 4 +- tests/IntegrationTest.php | 4 +- tests/Node/AutoEscapeTest.php | 3 +- tests/Node/BlockTest.php | 3 +- tests/Node/ForTest.php | 11 ++-- tests/Node/IfTest.php | 11 ++-- tests/Node/MacroTest.php | 3 +- tests/Node/ModuleTest.php | 3 +- tests/Node/PrintTest.php | 4 +- tests/Node/SandboxTest.php | 3 +- tests/Node/SetTest.php | 2 +- tests/Node/TextTest.php | 3 +- 22 files changed, 61 insertions(+), 213 deletions(-) delete mode 100644 src/Extension/YieldingExtension.php delete mode 100644 src/Node/YieldExpressionNode.php delete mode 100644 src/Node/YieldTextNode.php delete mode 100644 src/NodeVisitor/YieldingNodeVisitor.php diff --git a/src/Environment.php b/src/Environment.php index b4719b1b5..030e86bc9 100644 --- a/src/Environment.php +++ b/src/Environment.php @@ -22,7 +22,6 @@ use Twig\Extension\CoreExtension; use Twig\Extension\EscaperExtension; use Twig\Extension\ExtensionInterface; use Twig\Extension\OptimizerExtension; -use Twig\Extension\YieldingExtension; use Twig\Loader\ArrayLoader; use Twig\Loader\ChainLoader; use Twig\Loader\LoaderInterface; @@ -130,7 +129,6 @@ class Environment $this->addExtension(new CoreExtension()); $this->addExtension(new EscaperExtension($options['autoescape'])); $this->addExtension(new OptimizerExtension($options['optimizations'])); - $this->addExtension(new YieldingExtension($options['use_yield'])); } /** diff --git a/src/Extension/YieldingExtension.php b/src/Extension/YieldingExtension.php deleted file mode 100644 index f2a3fcbec..000000000 --- a/src/Extension/YieldingExtension.php +++ /dev/null @@ -1,29 +0,0 @@ -yielding = $yielding; - } - - public function getNodeVisitors(): array - { - return [new YieldingNodeVisitor($this->yielding)]; - } -} diff --git a/src/Node/PrintNode.php b/src/Node/PrintNode.php index 60386d299..78c67fa2e 100644 --- a/src/Node/PrintNode.php +++ b/src/Node/PrintNode.php @@ -29,9 +29,15 @@ class PrintNode extends Node implements NodeOutputInterface public function compile(Compiler $compiler): void { + $compiler->addDebugInfo($this); + + if ($compiler->getEnvironment()->useYield()) { + $compiler->write('yield '); + } else { + $compiler->write('echo '); + } + $compiler - ->addDebugInfo($this) - ->write('echo ') ->subcompile($this->getNode('expr')) ->raw(";\n") ; diff --git a/src/Node/TextNode.php b/src/Node/TextNode.php index d74ebe630..561288ca7 100644 --- a/src/Node/TextNode.php +++ b/src/Node/TextNode.php @@ -28,9 +28,15 @@ class TextNode extends Node implements NodeOutputInterface public function compile(Compiler $compiler): void { + $compiler->addDebugInfo($this); + + if ($compiler->getEnvironment()->useYield()) { + $compiler->write('yield '); + } else { + $compiler->write('echo '); + } + $compiler - ->addDebugInfo($this) - ->write('echo ') ->string($this->getAttribute('data')) ->raw(";\n") ; diff --git a/src/Node/YieldExpressionNode.php b/src/Node/YieldExpressionNode.php deleted file mode 100644 index e71c3e83e..000000000 --- a/src/Node/YieldExpressionNode.php +++ /dev/null @@ -1,32 +0,0 @@ - - */ -class YieldExpressionNode extends PrintNode -{ - public function compile(Compiler $compiler): void - { - $compiler - ->addDebugInfo($this) - ->write('yield ') - ->subcompile($this->getNode('expr')) - ->raw(";\n") - ; - } -} diff --git a/src/Node/YieldTextNode.php b/src/Node/YieldTextNode.php deleted file mode 100644 index 2da21fe0e..000000000 --- a/src/Node/YieldTextNode.php +++ /dev/null @@ -1,32 +0,0 @@ - - */ -class YieldTextNode extends TextNode -{ - public function compile(Compiler $compiler): void - { - $compiler - ->addDebugInfo($this) - ->write('yield ') - ->string($this->getAttribute('data')) - ->raw(";\n") - ; - } -} diff --git a/src/NodeVisitor/YieldingNodeVisitor.php b/src/NodeVisitor/YieldingNodeVisitor.php deleted file mode 100644 index 8d897690d..000000000 --- a/src/NodeVisitor/YieldingNodeVisitor.php +++ /dev/null @@ -1,81 +0,0 @@ - - * - * @internal - */ -final class YieldingNodeVisitor implements NodeVisitorInterface -{ - private $yielding; - - public function __construct(bool $yielding) - { - $this->yielding = $yielding; - } - - public function enterNode(Node $node, Environment $env): Node - { - if ($node instanceof YieldExpressionNode) { - if ($this->yielding) { - return $node; - } - - return new PrintNode($node->getNode('expr'), $node->getTemplateLine(), $node->getNodeTag()); - } - if ($node instanceof YieldTextNode) { - if ($this->yielding) { - return $node; - } - - return new TextNode($node->getAttribute('data'), $node->getTemplateLine()); - } - - if ($node instanceof PrintNode) { - // FIXME: deprecation - if (!$this->yielding) { - return $node; - } - - return new YieldExpressionNode($node->getNode('expr'), $node->getTemplateLine(), $node->getNodeTag()); - } - if ($node instanceof TextNode) { - // FIXME: deprecation - if (!$this->yielding) { - return $node; - } - - return new YieldTextNode($node->getAttribute('data'), $node->getTemplateLine()); - } - - return $node; - } - - public function leaveNode(Node $node, Environment $env): ?Node - { - return $node; - } - - public function getPriority(): int - { - return 255; - } -} diff --git a/src/Parser.php b/src/Parser.php index a24b7aa68..4016a5f39 100644 --- a/src/Parser.php +++ b/src/Parser.php @@ -22,9 +22,8 @@ use Twig\Node\ModuleNode; use Twig\Node\Node; use Twig\Node\NodeCaptureInterface; use Twig\Node\NodeOutputInterface; +use Twig\Node\PrintNode; use Twig\Node\TextNode; -use Twig\Node\YieldExpressionNode; -use Twig\Node\YieldTextNode; use Twig\TokenParser\TokenParserInterface; /** @@ -120,14 +119,14 @@ class Parser switch ($this->getCurrentToken()->getType()) { case /* Token::TEXT_TYPE */ 0: $token = $this->stream->next(); - $rv[] = new YieldTextNode($token->getValue(), $token->getLine()); + $rv[] = new TextNode($token->getValue(), $token->getLine()); break; case /* Token::VAR_START_TYPE */ 2: $token = $this->stream->next(); $expr = $this->expressionParser->parseExpression(); $this->stream->expect(/* Token::VAR_END_TYPE */ 4); - $rv[] = new YieldExpressionNode($expr, $token->getLine()); + $rv[] = new PrintNode($expr, $token->getLine()); break; case /* Token::BLOCK_START_TYPE */ 1: diff --git a/src/Test/NodeTestCase.php b/src/Test/NodeTestCase.php index 187d3bfc6..b10ae11d7 100644 --- a/src/Test/NodeTestCase.php +++ b/src/Test/NodeTestCase.php @@ -19,6 +19,8 @@ use Twig\Node\Node; abstract class NodeTestCase extends TestCase { + private Environment $currentEnv; + abstract public function getTests(); /** @@ -48,7 +50,7 @@ abstract class NodeTestCase extends TestCase protected function getEnvironment() { - return new Environment(new ArrayLoader([])); + return $this->currentEnv = new Environment(new ArrayLoader([])); } protected function getVariableGetter($name, $line = false) @@ -63,13 +65,18 @@ abstract class NodeTestCase extends TestCase return 'CoreExtension::getAttribute($this->env, $this->source, '; } + protected function getEchoOrYield(): string + { + return ($this->currentEnv ?? $this->getEnvironment())->useYield() ? 'yield' : 'echo'; + } + protected function getDisplayOrYield(string $expr): string { - return sprintf($this->getEnvironment()->useYield() ? 'yield from %s->unwrap()->yield' : '%s->display', $expr); + return sprintf(($this->currentEnv ?? $this->getEnvironment())->useYield() ? 'yield from %s->unwrap()->yield' : '%s->display', $expr); } protected function getDisplayOrYieldBlock(string $expr): string { - return sprintf($this->getEnvironment()->useYield() ? 'yield from %s->unwrap()->yieldBlock' : '%s->displayBlock', $expr); + return sprintf(($this->currentEnv ?? $this->getEnvironment())->useYield() ? 'yield from %s->unwrap()->yieldBlock' : '%s->displayBlock', $expr); } } diff --git a/src/TokenParser/ApplyTokenParser.php b/src/TokenParser/ApplyTokenParser.php index dd22f8103..4dbf30406 100644 --- a/src/TokenParser/ApplyTokenParser.php +++ b/src/TokenParser/ApplyTokenParser.php @@ -13,8 +13,8 @@ namespace Twig\TokenParser; use Twig\Node\Expression\TempNameExpression; use Twig\Node\Node; +use Twig\Node\PrintNode; use Twig\Node\SetNode; -use Twig\Node\YieldExpressionNode; use Twig\Token; /** @@ -44,7 +44,7 @@ final class ApplyTokenParser extends AbstractTokenParser return new Node([ new SetNode(true, $ref, $body, $lineno, $this->getTag()), - new YieldExpressionNode($filter, $lineno, $this->getTag()), + new PrintNode($filter, $lineno, $this->getTag()), ]); } diff --git a/src/TokenParser/BlockTokenParser.php b/src/TokenParser/BlockTokenParser.php index d51ad3156..5878131be 100644 --- a/src/TokenParser/BlockTokenParser.php +++ b/src/TokenParser/BlockTokenParser.php @@ -16,7 +16,7 @@ use Twig\Error\SyntaxError; use Twig\Node\BlockNode; use Twig\Node\BlockReferenceNode; use Twig\Node\Node; -use Twig\Node\YieldExpressionNode; +use Twig\Node\PrintNode; use Twig\Token; /** @@ -54,7 +54,7 @@ final class BlockTokenParser extends AbstractTokenParser } } else { $body = new Node([ - new YieldExpressionNode($this->parser->getExpressionParser()->parseExpression(), $lineno), + new PrintNode($this->parser->getExpressionParser()->parseExpression(), $lineno), ]); } $stream->expect(/* Token::BLOCK_END_TYPE */ 3); diff --git a/tests/IntegrationTest.php b/tests/IntegrationTest.php index f2ee4eb1f..e2b211a01 100644 --- a/tests/IntegrationTest.php +++ b/tests/IntegrationTest.php @@ -18,7 +18,7 @@ use Twig\Extension\SandboxExtension; use Twig\Extension\StringLoaderExtension; use Twig\Node\Expression\ConstantExpression; use Twig\Node\Node; -use Twig\Node\YieldExpressionNode; +use Twig\Node\PrintNode; use Twig\Sandbox\SecurityPolicy; use Twig\Test\IntegrationTestCase; use Twig\Token; @@ -135,7 +135,7 @@ class TwigTestTokenParser_§ extends AbstractTokenParser { $this->parser->getStream()->expect(Token::BLOCK_END_TYPE); - return new YieldExpressionNode(new ConstantExpression('§', -1), -1); + return new PrintNode(new ConstantExpression('§', -1), -1); } public function getTag(): string diff --git a/tests/Node/AutoEscapeTest.php b/tests/Node/AutoEscapeTest.php index d0f641c08..b2df9b160 100644 --- a/tests/Node/AutoEscapeTest.php +++ b/tests/Node/AutoEscapeTest.php @@ -31,9 +31,10 @@ class AutoEscapeTest extends NodeTestCase { $body = new Node([new TextNode('foo', 1)]); $node = new AutoEscapeNode(true, $body, 1); + $displayStmt = $this->getEchoOrYield(); return [ - [$node, "// line 1\necho \"foo\";"], + [$node, "// line 1\n$displayStmt \"foo\";"], ]; } } diff --git a/tests/Node/BlockTest.php b/tests/Node/BlockTest.php index 07e9373db..29b2e0f9d 100644 --- a/tests/Node/BlockTest.php +++ b/tests/Node/BlockTest.php @@ -16,7 +16,6 @@ use Twig\Loader\ArrayLoader; use Twig\Node\BlockNode; use Twig\Node\Node; use Twig\Node\TextNode; -use Twig\Node\YieldTextNode; use Twig\Test\NodeTestCase; class BlockTest extends NodeTestCase @@ -34,7 +33,7 @@ class BlockTest extends NodeTestCase { $tests = []; - $tests[] = [new BlockNode('foo', new YieldTextNode('foo', 1), 1), <<setAttribute('with_loop', false); + $displayStmt = $this->getEchoOrYield(); $tests[] = [$node, <<getVariableGetter('items')}); foreach (\$context['_seq'] as \$context["key"] => \$context["item"]) { - echo {$this->getVariableGetter('foo')}; + $displayStmt {$this->getVariableGetter('foo')}; } \$_parent = \$context['_parent']; unset(\$context['_seq'], \$context['_iterated'], \$context['key'], \$context['item'], \$context['_parent'], \$context['loop']); @@ -93,7 +94,7 @@ if (is_array(\$context['_seq']) || (is_object(\$context['_seq']) && \$context['_ \$context['loop']['last'] = 1 === \$length; } foreach (\$context['_seq'] as \$context["k"] => \$context["v"]) { - echo {$this->getVariableGetter('foo')}; + $displayStmt {$this->getVariableGetter('foo')}; ++\$context['loop']['index0']; ++\$context['loop']['index']; \$context['loop']['first'] = false; @@ -135,7 +136,7 @@ if (is_array(\$context['_seq']) || (is_object(\$context['_seq']) && \$context['_ \$context['loop']['last'] = 1 === \$length; } foreach (\$context['_seq'] as \$context["k"] => \$context["v"]) { - echo {$this->getVariableGetter('foo')}; + $displayStmt {$this->getVariableGetter('foo')}; ++\$context['loop']['index0']; ++\$context['loop']['index']; \$context['loop']['first'] = false; @@ -178,7 +179,7 @@ if (is_array(\$context['_seq']) || (is_object(\$context['_seq']) && \$context['_ \$context['loop']['last'] = 1 === \$length; } foreach (\$context['_seq'] as \$context["k"] => \$context["v"]) { - echo {$this->getVariableGetter('foo')}; + $displayStmt {$this->getVariableGetter('foo')}; \$context['_iterated'] = true; ++\$context['loop']['index0']; ++\$context['loop']['index']; @@ -190,7 +191,7 @@ foreach (\$context['_seq'] as \$context["k"] => \$context["v"]) { } } if (!\$context['_iterated']) { - echo {$this->getVariableGetter('foo')}; + $displayStmt {$this->getVariableGetter('foo')}; } \$_parent = \$context['_parent']; unset(\$context['_seq'], \$context['_iterated'], \$context['k'], \$context['v'], \$context['_parent'], \$context['loop']); diff --git a/tests/Node/IfTest.php b/tests/Node/IfTest.php index d5a6eac8a..5dda061d0 100644 --- a/tests/Node/IfTest.php +++ b/tests/Node/IfTest.php @@ -47,11 +47,12 @@ class IfTest extends NodeTestCase ], [], 1); $else = null; $node = new IfNode($t, $else, 1); + $displayStmt = $this->getEchoOrYield(); $tests[] = [$node, <<getVariableGetter('foo')}; + $displayStmt {$this->getVariableGetter('foo')}; } EOF ]; @@ -68,9 +69,9 @@ EOF $tests[] = [$node, <<getVariableGetter('foo')}; + $displayStmt {$this->getVariableGetter('foo')}; } elseif (false) { - echo {$this->getVariableGetter('bar')}; + $displayStmt {$this->getVariableGetter('bar')}; } EOF ]; @@ -85,9 +86,9 @@ EOF $tests[] = [$node, <<getVariableGetter('foo')}; + $displayStmt {$this->getVariableGetter('foo')}; } else { - echo {$this->getVariableGetter('bar')}; + $displayStmt {$this->getVariableGetter('bar')}; } EOF ]; diff --git a/tests/Node/MacroTest.php b/tests/Node/MacroTest.php index 16ccd92cb..7efea3cef 100644 --- a/tests/Node/MacroTest.php +++ b/tests/Node/MacroTest.php @@ -18,7 +18,6 @@ use Twig\Node\Expression\NameExpression; use Twig\Node\MacroNode; use Twig\Node\Node; use Twig\Node\TextNode; -use Twig\Node\YieldTextNode; use Twig\Test\NodeTestCase; class MacroTest extends NodeTestCase @@ -43,7 +42,7 @@ class MacroTest extends NodeTestCase 'bar' => new ConstantExpression('Foo', 1), ], [], 1); - $body = new YieldTextNode('foo', 1); + $body = new TextNode('foo', 1); $node = new MacroNode('foo', $body, $arguments, 1); $text[] = [$node, <<getEnvironment()->useYield() ? 'YieldingTemplate' : 'Template'; + $displayStmt = $this->getEchoOrYield(); $node = new ModuleNode($body, $extends, $blocks, $macros, $traits, new Node([]), $source); $tests[] = [$node, <<macros; // line 1 - echo "foo"; + $displayStmt "foo"; } /** diff --git a/tests/Node/PrintTest.php b/tests/Node/PrintTest.php index 49f8eb498..f951c2e36 100644 --- a/tests/Node/PrintTest.php +++ b/tests/Node/PrintTest.php @@ -28,7 +28,9 @@ class PrintTest extends NodeTestCase public function getTests() { $tests = []; - $tests[] = [new PrintNode(new ConstantExpression('foo', 1), 1), "// line 1\necho \"foo\";"]; + $displayStmt = $this->getEchoOrYield(); + + $tests[] = [new PrintNode(new ConstantExpression('foo', 1), 1), "// line 1\n$displayStmt \"foo\";"]; return $tests; } diff --git a/tests/Node/SandboxTest.php b/tests/Node/SandboxTest.php index 7cbddd75f..bf16f1f03 100644 --- a/tests/Node/SandboxTest.php +++ b/tests/Node/SandboxTest.php @@ -31,6 +31,7 @@ class SandboxTest extends NodeTestCase $body = new TextNode('foo', 1); $node = new SandboxNode($body, 1); + $displayStmt = $this->getEchoOrYield(); $tests[] = [$node, <<sandbox->isSandboxed()) { \$this->sandbox->enableSandbox(); } try { - echo "foo"; + $displayStmt "foo"; } finally { if (!\$alreadySandboxed) { \$this->sandbox->disableSandbox(); diff --git a/tests/Node/SetTest.php b/tests/Node/SetTest.php index 98d4e5735..70b3530bd 100644 --- a/tests/Node/SetTest.php +++ b/tests/Node/SetTest.php @@ -55,7 +55,7 @@ EOF $tests[] = [$node, <<env->getCharset()); EOF , new Environment(new ArrayLoader(), ['use_yield' => true]), diff --git a/tests/Node/TextTest.php b/tests/Node/TextTest.php index ace191213..31639cc2d 100644 --- a/tests/Node/TextTest.php +++ b/tests/Node/TextTest.php @@ -26,7 +26,8 @@ class TextTest extends NodeTestCase public function getTests() { $tests = []; - $tests[] = [new TextNode('foo', 1), "// line 1\necho \"foo\";"]; + $displayStmt = $this->getEchoOrYield(); + $tests[] = [new TextNode('foo', 1), "// line 1\n$displayStmt \"foo\";"]; return $tests; } From a9c7307b5c0ab97735ba96d5c33caf8334b95548 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Fri, 5 Jan 2024 19:16:08 +0100 Subject: [PATCH 04/13] Tweak code --- src/Environment.php | 10 ++++++++-- src/Test/NodeTestCase.php | 5 ++++- src/YieldingTemplate.php | 8 ++++++++ tests/Node/BlockTest.php | 16 +++++++++------- tests/Node/MacroTest.php | 20 +++++++++++--------- tests/Node/SetTest.php | 16 ++++++++++------ 6 files changed, 50 insertions(+), 25 deletions(-) diff --git a/src/Environment.php b/src/Environment.php index 030e86bc9..dbe382d44 100644 --- a/src/Environment.php +++ b/src/Environment.php @@ -66,6 +66,7 @@ class Environment private $runtimeLoaders = []; private $runtimes = []; private $optionsHash; + /** @var bool */ private $useYield; /** @@ -99,7 +100,9 @@ class Environment * (default to -1 which means that all optimizations are enabled; * set it to 0 to disable). * - * * use_yield: Enable the Twig 4 mode where template are using yield instead of echo + * * 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 = []) { @@ -117,7 +120,9 @@ class Environment ], $options); $this->useYield = (bool) $options['use_yield']; - // FIXME: deprecation if use_yield is false + 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'); @@ -849,6 +854,7 @@ class Environment self::VERSION, (int) $this->debug, (int) $this->strictVariables, + $this->useYield ? '1' : '0', ]); } } diff --git a/src/Test/NodeTestCase.php b/src/Test/NodeTestCase.php index b10ae11d7..8df0e9fee 100644 --- a/src/Test/NodeTestCase.php +++ b/src/Test/NodeTestCase.php @@ -19,7 +19,10 @@ use Twig\Node\Node; abstract class NodeTestCase extends TestCase { - private Environment $currentEnv; + /** + * @var Environment + */ + private $currentEnv; abstract public function getTests(); diff --git a/src/YieldingTemplate.php b/src/YieldingTemplate.php index e614d6bd5..9f8cb418b 100644 --- a/src/YieldingTemplate.php +++ b/src/YieldingTemplate.php @@ -21,6 +21,9 @@ use Twig\Error\RuntimeError; */ abstract class YieldingTemplate extends Template { + /** + * @return iterable + */ public function yield(array $context, array $blocks = []): iterable { $context = $this->env->mergeGlobals($context); @@ -65,6 +68,9 @@ abstract class YieldingTemplate extends Template } } + /** + * @return iterable + */ public function yieldBlock($name, array $context, array $blocks = [], $useBlocks = true, Template $templateContext = null) { if ($useBlocks && isset($blocks[$name])) { @@ -133,6 +139,8 @@ abstract class YieldingTemplate extends Template * @param string $name The block name to display from the parent * @param array $context The context * @param array $blocks The current set of blocks + * + * @return iterable */ public function yieldParentBlock($name, array $context, array $blocks = []) { diff --git a/tests/Node/BlockTest.php b/tests/Node/BlockTest.php index 29b2e0f9d..0edc0fc03 100644 --- a/tests/Node/BlockTest.php +++ b/tests/Node/BlockTest.php @@ -44,7 +44,8 @@ EOF , new Environment(new ArrayLoader(), ['use_yield' => true]) ]; - $tests[] = [new BlockNode('foo', new TextNode('foo', 1), 1), <<getEnvironment()->useYield()) { + $tests[] = [new BlockNode('foo', new TextNode('foo', 1), 1), << false]) - ]; - - $tests[] = [new BlockNode('foo', new Node(), 1), << true]) - ]; + , new Environment(new ArrayLoader()) + ]; + } return $tests; } diff --git a/tests/Node/MacroTest.php b/tests/Node/MacroTest.php index 7efea3cef..948949af6 100644 --- a/tests/Node/MacroTest.php +++ b/tests/Node/MacroTest.php @@ -45,7 +45,8 @@ class MacroTest extends NodeTestCase $body = new TextNode('foo', 1); $node = new MacroNode('foo', $body, $arguments, 1); - $text[] = [$node, <<getEnvironment()->useYield()) { + $text[] = [$node, <<env->getCharset()); } EOF - , new Environment(new ArrayLoader(), ['use_yield' => true]), - ]; + , new Environment(new ArrayLoader()), + ]; + } else { + $body = new TextNode('foo', 1); + $node = new MacroNode('foo', $body, $arguments, 1); - $body = new TextNode('foo', 1); - $node = new MacroNode('foo', $body, $arguments, 1); - - $tests[] = [$node, << false]), - ]; + , new Environment(new ArrayLoader()), + ]; + } return $tests; } diff --git a/tests/Node/SetTest.php b/tests/Node/SetTest.php index 70b3530bd..84d8a77c0 100644 --- a/tests/Node/SetTest.php +++ b/tests/Node/SetTest.php @@ -48,19 +48,22 @@ class SetTest extends NodeTestCase EOF ]; + $names = new Node([new AssignNameExpression('foo', 1)], [], 1); $values = new Node([new PrintNode(new ConstantExpression('foo', 1), 1)], [], 1); $node = new SetNode(true, $names, $values, 1); - $tests[] = [$node, <<getEnvironment()->useYield()) { + $tests[] = [$node, <<env->getCharset()); EOF - , new Environment(new ArrayLoader(), ['use_yield' => true]), - ]; - $tests[] = [$node, << false]), - ]; + , new Environment(new ArrayLoader()), + ]; + } $names = new Node([new AssignNameExpression('foo', 1)], [], 1); $values = new TextNode('foo', 1); From 4fd62abb872c976deeb99f4a3241a059d6812c91 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 6 Jan 2024 16:51:59 +0100 Subject: [PATCH 05/13] - --- .github/workflows/ci.yml | 5 ++++- CHANGELOG | 2 ++ tests/ErrorTest.php | 4 +--- tests/Fixtures/errors/leak-output.php | 4 ++-- tests/ignore-use-yield-deprecations | 1 + 5 files changed, 10 insertions(+), 6 deletions(-) create mode 100644 tests/ignore-use-yield-deprecations diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 153508c56..f78843e8e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,6 +20,9 @@ jobs: continue-on-error: ${{ matrix.experimental }} + env: + SYMFONY_DEPRECATIONS_HELPER: ignoreFile=./tests/ignore-use-yield-deprecations + strategy: matrix: php-version: @@ -132,7 +135,7 @@ jobs: - name: "Run tests" working-directory: extra/${{ matrix.extension }} - run: ../../vendor/bin/simple-phpunit + run: SYMFONY_DEPRECATIONS_HELPER=ignoreFile=../../tests/ignore-use-yield-deprecations ../../vendor/bin/simple-phpunit integration-tests: needs: diff --git a/CHANGELOG b/CHANGELOG index 55a307c39..b14430bcd 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,7 @@ # 3.9.0 (2023-XX-XX) + * Add a new "yield" mode for output generation + The "use_yield" Environment option controls the strategy: use "false" for "echo", "true" for "yield" * 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/tests/ErrorTest.php b/tests/ErrorTest.php index ef12567c4..db6418ed6 100644 --- a/tests/ErrorTest.php +++ b/tests/ErrorTest.php @@ -304,8 +304,7 @@ EOHTML ], ]; } -/* These tests don't make sense to me - Depending on whether you're using echo ->render() or display(), they don't behave in the same way + public function testTwigLeakOutputInDebugMode() { $output = exec(sprintf('%s %s debug', \PHP_BINARY, escapeshellarg(__DIR__.'/Fixtures/errors/leak-output.php'))); @@ -319,7 +318,6 @@ EOHTML $this->assertSame('', $output); } -*/ } class ErrorTest_Foo diff --git a/tests/Fixtures/errors/leak-output.php b/tests/Fixtures/errors/leak-output.php index fdb08502d..6ad229508 100644 --- a/tests/Fixtures/errors/leak-output.php +++ b/tests/Fixtures/errors/leak-output.php @@ -27,7 +27,7 @@ class BrokenExtension extends AbstractExtension $loader = new ArrayLoader([ 'index.html.twig' => 'Hello {{ "world"|broken }}', ]); -$twig = new Environment($loader, ['debug' => isset($argv[1])]); +$twig = new Environment($loader, ['debug' => isset($argv[1]), 'use_yield' => false]); $twig->addExtension(new BrokenExtension()); -$twig->display('index.html.twig'); +echo $twig->render('index.html.twig'); diff --git a/tests/ignore-use-yield-deprecations b/tests/ignore-use-yield-deprecations new file mode 100644 index 000000000..0f8442115 --- /dev/null +++ b/tests/ignore-use-yield-deprecations @@ -0,0 +1 @@ +%Since twig/twig 3.9.0: Not setting "use_yield" to "true" is deprecated.% From 71e90fab7b3d627af3300d76aa0b3ac857f43d40 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sun, 7 Jan 2024 21:44:50 +0100 Subject: [PATCH 06/13] Remove some tests --- .github/workflows/ci.yml | 5 +--- tests/ErrorTest.php | 14 ------------ tests/Fixtures/errors/leak-output.php | 33 --------------------------- 3 files changed, 1 insertion(+), 51 deletions(-) delete mode 100644 tests/Fixtures/errors/leak-output.php diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f78843e8e..527ac2c12 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,9 +20,6 @@ jobs: continue-on-error: ${{ matrix.experimental }} - env: - SYMFONY_DEPRECATIONS_HELPER: ignoreFile=./tests/ignore-use-yield-deprecations - strategy: matrix: php-version: @@ -64,7 +61,7 @@ jobs: 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: diff --git a/tests/ErrorTest.php b/tests/ErrorTest.php index db6418ed6..ee6a86c49 100644 --- a/tests/ErrorTest.php +++ b/tests/ErrorTest.php @@ -304,20 +304,6 @@ EOHTML ], ]; } - - public function testTwigLeakOutputInDebugMode() - { - $output = exec(sprintf('%s %s debug', \PHP_BINARY, escapeshellarg(__DIR__.'/Fixtures/errors/leak-output.php'))); - - $this->assertSame('Hello OOPS', $output); - } - - public function testDoesNotTwigLeakOutput() - { - $output = exec(sprintf('%s %s', \PHP_BINARY, escapeshellarg(__DIR__.'/Fixtures/errors/leak-output.php'))); - - $this->assertSame('', $output); - } } class ErrorTest_Foo diff --git a/tests/Fixtures/errors/leak-output.php b/tests/Fixtures/errors/leak-output.php deleted file mode 100644 index 6ad229508..000000000 --- a/tests/Fixtures/errors/leak-output.php +++ /dev/null @@ -1,33 +0,0 @@ - 'Hello {{ "world"|broken }}', -]); -$twig = new Environment($loader, ['debug' => isset($argv[1]), 'use_yield' => false]); -$twig->addExtension(new BrokenExtension()); - -echo $twig->render('index.html.twig'); From 72922a361216b346d6a535fd87ea53ad0aa8951a Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 11 Jan 2024 07:58:42 +0100 Subject: [PATCH 07/13] - --- src/Node/Expression/BlockReferenceExpression.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Node/Expression/BlockReferenceExpression.php b/src/Node/Expression/BlockReferenceExpression.php index 67c5781cf..e63c5b214 100644 --- a/src/Node/Expression/BlockReferenceExpression.php +++ b/src/Node/Expression/BlockReferenceExpression.php @@ -42,9 +42,6 @@ class BlockReferenceExpression extends AbstractExpression if ($compiler->getEnvironment()->useYield()) { $compiler->write('yield from '); - } - - if ($compiler->getEnvironment()->useYield()) { $this ->compileTemplateCall($compiler, 'yieldBlock') ->raw(";\n"); From 1469e6a6bcb30b12acf267a71aa3a16ea05e83b9 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 11 Jan 2024 08:22:24 +0100 Subject: [PATCH 08/13] Add support for templates that do not have output Nodes --- src/Node/ModuleNode.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/Node/ModuleNode.php b/src/Node/ModuleNode.php index ee04e2d21..a9d494e69 100644 --- a/src/Node/ModuleNode.php +++ b/src/Node/ModuleNode.php @@ -342,6 +342,9 @@ final class ModuleNode extends Node } else { $compiler->raw("->display(\$context, array_merge(\$this->blocks, \$blocks));\n"); } + } elseif ($compiler->getEnvironment()->useYield() && !$this->hasNodeOutputNodes($this->getNode('body'))) { + // ensure at least one yield call even for templates with no output + $compiler->write("yield;\n"); } $compiler @@ -483,4 +486,19 @@ final class ModuleNode extends Node throw new \LogicException('Trait templates can only be constant nodes.'); } } + + private function hasNodeOutputNodes(Node $node): bool + { + if ($node instanceof NodeOutputInterface) { + return true; + } + + foreach ($node as $child) { + if ($this->hasNodeOutputNodes($child)) { + return true; + } + } + + return false; + } } From 1591aa5c7d2b66e3fc307f5b1c182942d80c4093 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 11 Jan 2024 08:36:03 +0100 Subject: [PATCH 09/13] Optimize code --- src/YieldingTemplate.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/YieldingTemplate.php b/src/YieldingTemplate.php index 9f8cb418b..53027b932 100644 --- a/src/YieldingTemplate.php +++ b/src/YieldingTemplate.php @@ -54,7 +54,7 @@ abstract class YieldingTemplate extends Template public function render(array $context): string { $content = ''; - foreach ($this->yield($this->env->mergeGlobals($context), array_merge($this->blocks)) as $data) { + foreach ($this->yield($context) as $data) { $content .= $data; } @@ -63,7 +63,7 @@ abstract class YieldingTemplate extends Template public function display(array $context, array $blocks = []): void { - foreach ($this->yield($this->env->mergeGlobals($context), array_merge($this->blocks)) as $data) { + foreach ($this->yield($context, $blocks) as $data) { echo $data; } } From 210e2d2b9eb4f73c48dc9adc7bd1d93d2db6f3f3 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Fri, 12 Jan 2024 12:54:58 +0100 Subject: [PATCH 10/13] Fix tests --- extra/twig-extra-bundle/Tests/Fixture/Kernel.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/extra/twig-extra-bundle/Tests/Fixture/Kernel.php b/extra/twig-extra-bundle/Tests/Fixture/Kernel.php index 3df6357fe..3283af202 100644 --- a/extra/twig-extra-bundle/Tests/Fixture/Kernel.php +++ b/extra/twig-extra-bundle/Tests/Fixture/Kernel.php @@ -24,11 +24,17 @@ class Kernel extends BaseKernel protected function configureContainer(ContainerBuilder $c, LoaderInterface $loader): void { - $c->loadFromExtension('framework', [ + $config = [ 'secret' => 'S3CRET', 'test' => true, - ]); - + 'router' => ['utf8' => true], + 'http_method_override' => false, + ]; + if (Kernel::MAJOR_VERSION >= 6 && Kernel::MINOR_VERSION >= 2) { + $config['handle_all_throwables'] = true; + $config['php_errors']['log'] = true; + } + $c->loadFromExtension('framework', $config); $c->loadFromExtension('twig', [ 'default_path' => __DIR__.'/views', ]); From 14d38036477b93976b56b1ee7046e86a390645dd Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sun, 14 Jan 2024 09:25:32 +0100 Subject: [PATCH 11/13] Add a deprecation when a Node uses echo/print --- CHANGELOG | 5 ++-- .../Tests/Fixture/Kernel.php | 2 +- src/Compiler.php | 28 +++++++++++++++++++ src/Node/Expression/InlinePrint.php | 2 ++ src/Node/PrintNode.php | 6 +++- src/Node/TextNode.php | 6 +++- 6 files changed, 44 insertions(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index b14430bcd..d0bc0cae3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,7 +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 strategy: use "false" for "echo", "true" for "yield" + 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 index 3283af202..857ed95d4 100644 --- a/extra/twig-extra-bundle/Tests/Fixture/Kernel.php +++ b/extra/twig-extra-bundle/Tests/Fixture/Kernel.php @@ -30,7 +30,7 @@ class Kernel extends BaseKernel 'router' => ['utf8' => true], 'http_method_override' => false, ]; - if (Kernel::MAJOR_VERSION >= 6 && Kernel::MINOR_VERSION >= 2) { + if (6 === Kernel::MAJOR_VERSION) { $config['handle_all_throwables'] = true; $config['php_errors']['log'] = true; } diff --git a/src/Compiler.php b/src/Compiler.php index eb652c61a..543b4e104 100644 --- 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,6 +87,16 @@ class Compiler 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. * @@ -92,6 +104,9 @@ class Compiler */ public function raw(string $string) { + if ($this->checkForOutput) { + $this->checkStringForOutput(trim($string)); + } $this->source .= $string; return $this; @@ -105,6 +120,10 @@ class Compiler 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 @@ class Compiler { 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/Node/Expression/InlinePrint.php b/src/Node/Expression/InlinePrint.php index 8c262e2e1..725536a86 100644 --- a/src/Node/Expression/InlinePrint.php +++ b/src/Node/Expression/InlinePrint.php @@ -33,7 +33,9 @@ final class InlinePrint extends AbstractExpression ; } else { $compiler + ->checkForOutput(false) ->raw('print(') + ->checkForOutput(true) ->subcompile($this->getNode('node')) ->raw(')') ; diff --git a/src/Node/PrintNode.php b/src/Node/PrintNode.php index 78c67fa2e..36bcaee64 100644 --- a/src/Node/PrintNode.php +++ b/src/Node/PrintNode.php @@ -34,7 +34,11 @@ class PrintNode extends Node implements NodeOutputInterface if ($compiler->getEnvironment()->useYield()) { $compiler->write('yield '); } else { - $compiler->write('echo '); + $compiler + ->checkForOutput(false) + ->write('echo ') + ->checkForOutput(true) + ; } $compiler diff --git a/src/Node/TextNode.php b/src/Node/TextNode.php index 561288ca7..3e417dad6 100644 --- a/src/Node/TextNode.php +++ b/src/Node/TextNode.php @@ -33,7 +33,11 @@ class TextNode extends Node implements NodeOutputInterface if ($compiler->getEnvironment()->useYield()) { $compiler->write('yield '); } else { - $compiler->write('echo '); + $compiler + ->checkForOutput(false) + ->write('echo ') + ->checkForOutput(true) + ; } $compiler From 0d3a28390702a531ba65baa089c50058a2c2c872 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 25 Jan 2024 07:42:57 +0100 Subject: [PATCH 12/13] Tweak code --- src/Node/BlockNode.php | 2 +- src/Node/ModuleNode.php | 2 +- tests/Node/BlockTest.php | 24 ++++++++++++------------ tests/TemplateWrapperTest.php | 17 +++++++++++------ 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/Node/BlockNode.php b/src/Node/BlockNode.php index bca7a6ab1..241dff0b2 100644 --- a/src/Node/BlockNode.php +++ b/src/Node/BlockNode.php @@ -41,7 +41,7 @@ class BlockNode extends Node if (!$this->getNode('body') instanceof NodeOutputInterface && $compiler->getEnvironment()->useYield()) { // needed when body doesn't yield anything - $compiler->write("yield;\n"); + $compiler->write("yield '';\n"); } $compiler diff --git a/src/Node/ModuleNode.php b/src/Node/ModuleNode.php index a9d494e69..4bc35344f 100644 --- a/src/Node/ModuleNode.php +++ b/src/Node/ModuleNode.php @@ -344,7 +344,7 @@ final class ModuleNode extends Node } } elseif ($compiler->getEnvironment()->useYield() && !$this->hasNodeOutputNodes($this->getNode('body'))) { // ensure at least one yield call even for templates with no output - $compiler->write("yield;\n"); + $compiler->write("yield '';\n"); } $compiler diff --git a/tests/Node/BlockTest.php b/tests/Node/BlockTest.php index 0edc0fc03..cdc2c861c 100644 --- a/tests/Node/BlockTest.php +++ b/tests/Node/BlockTest.php @@ -33,17 +33,6 @@ class BlockTest extends NodeTestCase { $tests = []; - $tests[] = [new BlockNode('foo', new TextNode('foo', 1), 1), <<macros; - yield "foo"; -} -EOF - , new Environment(new ArrayLoader(), ['use_yield' => true]) - ]; - if (!$this->getEnvironment()->useYield()) { $tests[] = [new BlockNode('foo', new TextNode('foo', 1), 1), <<macros; + yield "foo"; +} +EOF + , new Environment(new ArrayLoader()) + ]; + $tests[] = [new BlockNode('foo', new Node(), 1), <<macros; - yield; + yield ''; } EOF , new Environment(new ArrayLoader()) diff --git a/tests/TemplateWrapperTest.php b/tests/TemplateWrapperTest.php index 7e002bf7a..776ac3fa8 100644 --- a/tests/TemplateWrapperTest.php +++ b/tests/TemplateWrapperTest.php @@ -58,14 +58,19 @@ class TemplateWrapperTest extends TestCase { $twig = new Environment(new ArrayLoader([ 'index' => '{% block foo %}{{ foo }}{{ bar }}{% endblock %}', - ], ['use_yield' => false])); - $twig->addGlobal('bar', 'BAR'); + ])); - $wrapper = $twig->load('index'); + if (!$twig->useYield()) { + $twig->addGlobal('bar', 'BAR'); - ob_start(); - $wrapper->displayBlock('foo', ['foo' => 'FOO']); + $wrapper = $twig->load('index'); - $this->assertEquals('FOOBAR', ob_get_clean()); + ob_start(); + $wrapper->displayBlock('foo', ['foo' => 'FOO']); + + $this->assertEquals('FOOBAR', ob_get_clean()); + } else { + $this->markTestSkipped('yield not used.'); + } } } From a93723eae25ae5dcfbed876a1804107dbf37536c Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 25 Jan 2024 08:09:49 +0100 Subject: [PATCH 13/13] Fix bug --- src/YieldingTemplate.php | 15 ++++++++++----- tests/Fixtures/functions/parent_in_condition.test | 11 +++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) create mode 100644 tests/Fixtures/functions/parent_in_condition.test diff --git a/src/YieldingTemplate.php b/src/YieldingTemplate.php index 53027b932..93ee894d2 100644 --- a/src/YieldingTemplate.php +++ b/src/YieldingTemplate.php @@ -155,6 +155,16 @@ abstract class YieldingTemplate extends Template } } + public function renderParentBlock($name, array $context, array $blocks = []) + { + $content = ''; + foreach ($this->yieldParentBlock($name, $context, $blocks) as $data) { + $content .= $data; + } + + return $content; + } + public function displayBlock($name, array $context, array $blocks = [], $useBlocks = true, Template $templateContext = null) { throw new RuntimeError(sprintf('Calling "%s" for block "%s" is not supported as "use_yield" is set to "true".', __METHOD__, $name), -1, $this->getSourceContext()); @@ -165,11 +175,6 @@ abstract class YieldingTemplate extends Template throw new RuntimeError(sprintf('Calling "%s" for block "%s" is not supported as "use_yield" is set to "true".', __METHOD__, $name), -1, $this->getSourceContext()); } - public function renderParentBlock($name, array $context, array $blocks = []) - { - throw new RuntimeError(sprintf('Calling "%s" for block "%s" is not supported as "use_yield" is set to "true".', __METHOD__, $name), -1, $this->getSourceContext()); - } - protected function displayWithErrorHandling(array $context, array $blocks = []) { throw new RuntimeError(sprintf('Calling "%s" is not supported as "use_yield" is set to "true".', __METHOD__), -1, $this->getSourceContext()); diff --git a/tests/Fixtures/functions/parent_in_condition.test b/tests/Fixtures/functions/parent_in_condition.test new file mode 100644 index 000000000..f3d51d2dd --- /dev/null +++ b/tests/Fixtures/functions/parent_in_condition.test @@ -0,0 +1,11 @@ +--TEST-- +"block" calling parent() in a conditional expression +--TEMPLATE-- +{% extends "parent.twig" %} +{% block label %}{{ parent() ?: 'foo' }}{% endblock %} +--TEMPLATE(parent.twig)-- +{% block label %}PARENT_LABEL{% endblock %} +--DATA-- +return [] +--EXPECT-- +PARENT_LABEL