Add back the if condition on for loops

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

doc/tags/for.rst

@@ -94,6 +94,67 @@ the :doc:`slice <../filters/slice>` filter:
         {% endfor %}
     </ul>
 
+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
 -------------------
 

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;

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

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

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

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, <<<EOF
@@ -144,7 +150,9 @@ EOF
     \$__internal_compile_2 = \$context;
     \$context['loop'] = new \Twig\Runtime\LoopContext(\$__internal_compile_0, \$__internal_compile_2, \$blocks, \$__internal_compile_1, \$depth);
     foreach (\$__internal_compile_0 as \$context["k"] => \$context["v"]) {
-        yield {$this->getVariableGetter('foo')};
+        if (true) {
+            yield {$this->getVariableGetter('foo')};
+        }
     }
     if (0 === \$__internal_compile_0->getIndex0()) {
         yield {$this->getVariableGetter('foo')};