From 47d745b7edaf1e6f38c799418c7bcb0ad536677a Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Wed, 28 Aug 2024 21:26:55 +0200 Subject: [PATCH] Add back the if condition on for loops --- CHANGELOG | 1 + doc/tags/for.rst | 61 +++++++++++++++++++ src/Node/ForNode.php | 6 +- src/TokenParser/ForTokenParser.php | 7 ++- tests/Fixtures/tags/for/condition.test | 12 ++++ .../Fixtures/tags/for/loop_in_condition.test | 14 +++++ tests/Node/ForTest.php | 16 +++-- 7 files changed, 111 insertions(+), 6 deletions(-) create mode 100644 tests/Fixtures/tags/for/condition.test create mode 100644 tests/Fixtures/tags/for/loop_in_condition.test diff --git a/CHANGELOG b/CHANGELOG index 39a4b7886..80ab3d22b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,6 @@ # 4.0.0 (2024-XX-XX) + * Add back the `if` condition on `for` loops * Add return types to all ExtensionInterface methods (`getFunctions()`, `getFilters()`, etc.) * Add support for recursive loops (via the `loop()` function) * Add `loop.changed`, `loop.previous`, `loop.next`, and `loop.cycle` variables diff --git a/doc/tags/for.rst b/doc/tags/for.rst index ada0e956e..549189b20 100644 --- a/doc/tags/for.rst +++ b/doc/tags/for.rst @@ -94,6 +94,67 @@ the :doc:`slice <../filters/slice>` filter: {% endfor %} +Adding a Condition +------------------ + +Skipping items during an iteration can be done in several ways: + +* Using a :doc:`filter <../filters/filter>` filter: + + .. code-block:: twig + + {% for user in users|filter(user => user.active) %} + - {{ user.username }} + {% endfor %} + + The items are filtered **before** the loop starts (the ``loop.index`` will + not be incremented for filtered items). + +* Using an ``if`` condition after ``for``: + + .. code-block:: twig + + {% for user in users if user.active %} + - {{ user.username }} + {% endfor %} + + The items are filtered **during** the loop; all items are iterated (the + ``loop.index`` will also increment for filtered items). As a consequence, be + warned that the ``loop.last`` variable might never be set to ``true`` if the + last item is skipped and ``loop.length`` returns the length of the unfiltered + sequence/mapping. + + This is just a convenient shortcut for using an ``if`` condition inside the + ``for`` body (both are equivalent): + + .. code-block:: twig + + {% for user in users %} + {% if user.active %} + - {{ user.username }} + {% endif %} + {% endfor %} + + It's recommended to use the ``filter`` filter except for when you need to + filter based on a variable that changes in the body of the loop: + + .. code-block:: twig + + {% set users = ['Thomas', 'Lucas', 'Fabien', 'Hélène'] %} + {% set stopOnFabien = false %} + {% for user in users if not stopOnFabien %} + - {{ user }} + {% set stopOnFabien = user == 'Fabien' %} + {% endfor %} + + Or when you need to use the ``loop`` variable in the condition: + + .. code-block:: twig + + {% for user in users if loop.length != 5 %} + - {{ user }} + {% endfor %} + The ``else`` Clause ------------------- diff --git a/src/Node/ForNode.php b/src/Node/ForNode.php index 1f30d296f..166428826 100644 --- a/src/Node/ForNode.php +++ b/src/Node/ForNode.php @@ -25,8 +25,12 @@ use Twig\Node\Expression\AssignNameExpression; #[YieldReady] class ForNode extends Node { - public function __construct(AssignNameExpression $keyTarget, AssignNameExpression $valueTarget, AbstractExpression $seq, ?Node $ifexpr, Node $body, ?Node $else, int $lineno) + public function __construct(AssignNameExpression $keyTarget, AssignNameExpression $valueTarget, AbstractExpression $seq, ?AbstractExpression $ifexpr, Node $body, ?Node $else, int $lineno) { + if ($ifexpr) { + $body = new IfNode(new Node([$ifexpr, $body]), null, $lineno); + } + $nodes = ['key_target' => $keyTarget, 'value_target' => $valueTarget, 'seq' => $seq, 'body' => $body]; if (null !== $else) { $nodes['else'] = $else; diff --git a/src/TokenParser/ForTokenParser.php b/src/TokenParser/ForTokenParser.php index f268e255c..da437d031 100644 --- a/src/TokenParser/ForTokenParser.php +++ b/src/TokenParser/ForTokenParser.php @@ -38,6 +38,11 @@ final class ForTokenParser extends AbstractTokenParser $stream->expect(Token::OPERATOR_TYPE, 'in'); $seq = $this->parser->getExpressionParser()->parseExpression(); + $ifexpr = null; + if ($stream->nextIf(Token::NAME_TYPE, 'if')) { + $ifexpr = $this->parser->getExpressionParser()->parseExpression(); + } + $stream->expect(Token::BLOCK_END_TYPE); $body = $this->parser->subparse($this->decideForFork(...)); if ('else' == $stream->next()->getValue()) { @@ -58,7 +63,7 @@ final class ForTokenParser extends AbstractTokenParser } $valueTarget = new AssignNameExpression($valueTarget->getAttribute('name'), $valueTarget->getTemplateLine()); - return new ForNode($keyTarget, $valueTarget, $seq, null, $body, $else, $lineno); + return new ForNode($keyTarget, $valueTarget, $seq, $ifexpr, $body, $else, $lineno); } public function decideForFork(Token $token): bool diff --git a/tests/Fixtures/tags/for/condition.test b/tests/Fixtures/tags/for/condition.test new file mode 100644 index 000000000..b1240a3d1 --- /dev/null +++ b/tests/Fixtures/tags/for/condition.test @@ -0,0 +1,12 @@ +--TEST-- +"for" tag takes a condition +--TEMPLATE-- +{% for i in 1..5 if i is odd -%} + {{ loop.index }}.{{ i }} +{% endfor %} +--DATA-- +return [] +--EXPECT-- +1.1 +3.3 +5.5 diff --git a/tests/Fixtures/tags/for/loop_in_condition.test b/tests/Fixtures/tags/for/loop_in_condition.test new file mode 100644 index 000000000..ea9e5d001 --- /dev/null +++ b/tests/Fixtures/tags/for/loop_in_condition.test @@ -0,0 +1,14 @@ +--TEST-- +"for" tag using "loop" when using a condition +--TEMPLATE-- +{% for i, item in items if i > 0 %} + {{ loop.last }} +{% endfor %} +{% for i, item in items if loop.index0 is odd %} + {{- i }} +{% endfor %} +--DATA-- +return ['items' => ['a', 'b']] +--EXPECT-- +1 +1 diff --git a/tests/Node/ForTest.php b/tests/Node/ForTest.php index d17df6f2a..51c0512d0 100644 --- a/tests/Node/ForTest.php +++ b/tests/Node/ForTest.php @@ -14,8 +14,10 @@ namespace Twig\Tests\Node; use Twig\Environment; use Twig\Loader\ArrayLoader; use Twig\Node\Expression\AssignNameExpression; +use Twig\Node\Expression\ConstantExpression; use Twig\Node\Expression\NameExpression; use Twig\Node\ForNode; +use Twig\Node\IfNode; use Twig\Node\Node; use Twig\Node\PrintNode; use Twig\Test\NodeTestCase; @@ -27,15 +29,18 @@ class ForTest extends NodeTestCase $keyTarget = new AssignNameExpression('key', 1); $valueTarget = new AssignNameExpression('item', 1); $seq = new NameExpression('items', 1); + $ifexpr = new ConstantExpression(true, 1); $body = new Node([new PrintNode(new NameExpression('foo', 1), 1)], [], 1); $else = null; - $node = new ForNode($keyTarget, $valueTarget, $seq, null, $body, $else, 1); + $node = new ForNode($keyTarget, $valueTarget, $seq, $ifexpr, $body, $else, 1); $node->setAttribute('with_loop', false); $this->assertEquals($keyTarget, $node->getNode('key_target')); $this->assertEquals($valueTarget, $node->getNode('value_target')); $this->assertEquals($seq, $node->getNode('seq')); - $this->assertEquals($body, $node->getNode('body')); + $this->assertInstanceOf(IfNode::class, $node->getNode('body')); + $this->assertEquals($ifexpr, $node->getNode('body')->getNode('tests')->getNode(0)); + $this->assertEquals($body, $node->getNode('body')->getNode('tests')->getNode(1)); $this->assertFalse($node->hasNode('else')); $else = new PrintNode(new NameExpression('foo', 1), 1); @@ -131,9 +136,10 @@ EOF $keyTarget = new AssignNameExpression('k', 1); $valueTarget = new AssignNameExpression('v', 1); $seq = new NameExpression('values', 1); + $ifexpr = new ConstantExpression(true, 1); $body = new Node([new PrintNode(new NameExpression('foo', 1), 1)], [], 1); $else = new PrintNode(new NameExpression('foo', 1), 1); - $node = new ForNode($keyTarget, $valueTarget, $seq, null, $body, $else, 1); + $node = new ForNode($keyTarget, $valueTarget, $seq, $ifexpr, $body, $else, 1); $node->setAttribute('with_loop', true); $tests[] = [$node, << \$context["v"]) { - yield {$this->getVariableGetter('foo')}; + if (true) { + yield {$this->getVariableGetter('foo')}; + } } if (0 === \$__internal_compile_0->getIndex0()) { yield {$this->getVariableGetter('foo')};