From cebfbb1a5206a71b9fdd8672bbd145a41b7523bb Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Sun, 24 Nov 2024 10:05:07 +0100 Subject: [PATCH] Introduce ForElseNode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently there is no line number information for `{% else %}` inside a `for` loop. Normally, this wouldn't be such a big deal. But TwigStan uses this line information to map errors in PHP back to the original Twig source. Let's say you have the following template: ```twig {% for product in products %}

{{ product.name }}

{% else %}

No products found

{% endfor %} ``` And `products` is of type `non-empty-array`, it means that the else will never happen. This is currently reported as: ``` Negated boolean expression is always false. 🔖 booleanNot.alwaysFalse 🐘 compiled_index.php:83 🌱 templates/product/index.html.twig:2 🎯 src/Controller/ProductController.php:15 ``` But as you can see, it points to line number 2, instead of line number 3. In the compiled code, it looks like this: ```php // line /Volumes/CS/opensource/twigstan-demo/templates/product/index.html.twig:1 foreach ($context['_seq'] as $context["_key"] => $context["product"]) { // line /Volumes/CS/opensource/twigstan-demo/templates/product/index.html.twig:2 yield "

"; // line /Volumes/CS/opensource/twigstan-demo/templates/product/index.html.twig:2 yield \Twig\Extension\CoreExtension::getAttribute($this->env, $this->source, $context // line /Volumes/CS/opensource/twigstan-demo/templates/product/index.html.twig:2 yield "

\n "; $context['_iterated'] = \true; } if (!$context['_iterated']) { // line /Volumes/CS/opensource/twigstan-demo/templates/product/index.html.twig:4 yield "

No products found

\n "; } ``` With this change, we will change the compiled code to become: ```php // line /Volumes/CS/opensource/twigstan-demo/templates/product/index.html.twig:1 foreach ($context['_seq'] as $context["_key"] => $context["product"]) { // line /Volumes/CS/opensource/twigstan-demo/templates/product/index.html.twig:2 yield "

"; // line /Volumes/CS/opensource/twigstan-demo/templates/product/index.html.twig:2 yield \Twig\Extension\CoreExtension::getAttribute($this->env, $this->source, $context // line /Volumes/CS/opensource/twigstan-demo/templates/product/index.html.twig:2 yield "

\n "; $context['_iterated'] = \true; } // line /Volumes/CS/opensource/twigstan-demo/templates/product/index.html.twig:3 if (!$context['_iterated']) { // line /Volumes/CS/opensource/twigstan-demo/templates/product/index.html.twig:4 yield "

No products found

\n "; } ``` --- src/Node/ForElseNode.php | 44 ++++++++++++++++++++++++++++++ src/Node/ForNode.php | 10 ++----- src/TokenParser/ForTokenParser.php | 8 ++++-- tests/Node/ForTest.php | 7 +++-- 4 files changed, 57 insertions(+), 12 deletions(-) create mode 100644 src/Node/ForElseNode.php diff --git a/src/Node/ForElseNode.php b/src/Node/ForElseNode.php new file mode 100644 index 00000000000..e53edd28524 --- /dev/null +++ b/src/Node/ForElseNode.php @@ -0,0 +1,44 @@ + + */ +#[YieldReady] +class ForElseNode extends Node +{ + public function __construct(Node $body, int $lineno) + { + parent::__construct(['body' => $body], [], $lineno); + } + + public function compile(Compiler $compiler): void + { + $compiler + ->addDebugInfo($this) + ->write("if (0 === \$iterator->getIndex0()) {\n") + ->indent() + ->subcompile($this->getNode('body')) + ->outdent() + ->write("}\n") + ; + } +} diff --git a/src/Node/ForNode.php b/src/Node/ForNode.php index b3ca78707af..c69c7d5b06e 100644 --- a/src/Node/ForNode.php +++ b/src/Node/ForNode.php @@ -25,7 +25,7 @@ #[YieldReady] class ForNode extends Node { - public function __construct(AssignContextVariable $keyTarget, AssignContextVariable $valueTarget, AbstractExpression $seq, ?AbstractExpression $ifexpr, Node $body, ?Node $else, int $lineno) + public function __construct(AssignContextVariable $keyTarget, AssignContextVariable $valueTarget, AbstractExpression $seq, ?AbstractExpression $ifexpr, Node $body, ?ForElseNode $else, int $lineno) { if ($ifexpr) { $body = new IfNode(new Nodes([$ifexpr, $body]), null, $lineno); @@ -72,13 +72,7 @@ public function compile(Compiler $compiler): void ; if ($this->hasNode('else')) { - $compiler - ->write("if (0 === \$iterator->getIndex0()) {\n") - ->indent() - ->subcompile($this->getNode('else')) - ->outdent() - ->write("}\n") - ; + $compiler->subcompile($this->getNode('else')); } // remove some "private" loop variables (needed for nested loops) diff --git a/src/TokenParser/ForTokenParser.php b/src/TokenParser/ForTokenParser.php index a7363696125..a313bb57fc3 100644 --- a/src/TokenParser/ForTokenParser.php +++ b/src/TokenParser/ForTokenParser.php @@ -13,6 +13,7 @@ namespace Twig\TokenParser; use Twig\Node\Expression\Variable\AssignContextVariable; +use Twig\Node\ForElseNode; use Twig\Node\ForNode; use Twig\Node\Node; use Twig\Token; @@ -46,8 +47,11 @@ public function parse(Token $token): Node $stream->expect(Token::BLOCK_END_TYPE); $body = $this->parser->subparse($this->decideForFork(...)); if ('else' == $stream->next()->getValue()) { - $stream->expect(Token::BLOCK_END_TYPE); - $else = $this->parser->subparse($this->decideForEnd(...), true); + $elseLineno = $stream->expect(Token::BLOCK_END_TYPE)->getLine(); + $else = new ForElseNode( + $this->parser->subparse($this->decideForEnd(...), true), + $elseLineno + ); } else { $else = null; } diff --git a/tests/Node/ForTest.php b/tests/Node/ForTest.php index 4efc7e27fbf..36f810bd550 100644 --- a/tests/Node/ForTest.php +++ b/tests/Node/ForTest.php @@ -16,6 +16,7 @@ use Twig\Node\Expression\ConstantExpression; use Twig\Node\Expression\Variable\AssignContextVariable; use Twig\Node\Expression\Variable\ContextVariable; +use Twig\Node\ForElseNode; use Twig\Node\ForNode; use Twig\Node\IfNode; use Twig\Node\Nodes; @@ -43,7 +44,7 @@ public function testConstructor() $this->assertEquals($body, $node->getNode('body')->getNode('tests')->getNode(1)); $this->assertFalse($node->hasNode('else')); - $else = new PrintNode(new ContextVariable('foo', 1), 1); + $else = new ForElseNode(new PrintNode(new ContextVariable('foo', 1), 1), 5); $node = new ForNode($keyTarget, $valueTarget, $seq, null, $body, $else, 1); $node->setAttribute('with_loop', false); $this->assertEquals($else, $node->getNode('else')); @@ -136,7 +137,7 @@ public static function provideTests(): iterable $seq = new ContextVariable('values', 1); $ifexpr = new ConstantExpression(true, 1); $body = new Nodes([new PrintNode(new ContextVariable('foo', 1), 1)], 1); - $else = new PrintNode(new ContextVariable('foo', 1), 1); + $else = new ForElseNode(new PrintNode(new ContextVariable('foo', 6), 6), 5); $node = new ForNode($keyTarget, $valueTarget, $seq, $ifexpr, $body, $else, 1); $node->setAttribute('with_loop', true); @@ -152,7 +153,9 @@ public static function provideTests(): iterable yield {$fooGetter}; } } + // line 5 if (0 === \$iterator->getIndex0()) { + // line 6 yield {$fooGetter}; } unset(\$context['k'], \$context['v'], \$context['loop']);