diff --git a/CHANGELOG b/CHANGELOG
index d38d3635b..f8d1b0d2c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,6 @@
* 3.0.0 (2019-XX-XX)
+ * removed the "if" condition support on the "for" tag
* made the in, <, >, <=, >=, ==, and != operators more strict when comparing strings and integers/floats
* removed the "filter" tag
* added type hints everywhere
diff --git a/doc/tags/for.rst b/doc/tags/for.rst
index 77b1dc82d..a30cdee5d 100644
--- a/doc/tags/for.rst
+++ b/doc/tags/for.rst
@@ -78,8 +78,7 @@ Variable Description
The ``loop.length``, ``loop.revindex``, ``loop.revindex0``, and
``loop.last`` variables are only available for PHP arrays, or objects that
- implement the ``Countable`` interface. They are also not available when
- looping with a condition.
+ implement the ``Countable`` interface.
Adding a condition
------------------
@@ -91,22 +90,28 @@ items. The following example skips all the users which are not active:
.. code-block:: twig
- {% for user in users if user.active %}
+ {% for user in users|filter(user => user.active) %}
- {{ user.username|e }}
{% endfor %}
The advantage is that the special loop variable will count correctly thus not
-counting the users not iterated over. Keep in mind that properties like
-``loop.last`` will not be defined when using loop conditions.
+counting the users not iterated over.
-.. note::
+If you need to skip items "dynamically", use an ``if`` tag within the ``for``'s
+body:
- Using the ``loop`` variable within the condition is not recommended as it
- will probably not be doing what you expect it to. For instance, adding a
- condition like ``loop.index > 4`` won't work as the index is only
- incremented when the condition is true (so the condition will never
- match).
+.. code-block:: twig
+
+
+ {% set c = false %}
+ {% for i, user in users %}
+ {% if i is even or c %}
+ - {{ user.username|e }}
+ {% set c = i > 5 %}
+ {% endif %}
+ {% endfor %}
+
The `else` Clause
-----------------
diff --git a/src/Node/ForLoopNode.php b/src/Node/ForLoopNode.php
index 7aec737d5..d5ce845a7 100644
--- a/src/Node/ForLoopNode.php
+++ b/src/Node/ForLoopNode.php
@@ -36,19 +36,14 @@ class ForLoopNode extends Node
->write("++\$context['loop']['index0'];\n")
->write("++\$context['loop']['index'];\n")
->write("\$context['loop']['first'] = false;\n")
+ ->write("if (isset(\$context['loop']['length'])) {\n")
+ ->indent()
+ ->write("--\$context['loop']['revindex0'];\n")
+ ->write("--\$context['loop']['revindex'];\n")
+ ->write("\$context['loop']['last'] = 0 === \$context['loop']['revindex0'];\n")
+ ->outdent()
+ ->write("}\n")
;
-
- if (!$this->getAttribute('ifexpr')) {
- $compiler
- ->write("if (isset(\$context['loop']['length'])) {\n")
- ->indent()
- ->write("--\$context['loop']['revindex0'];\n")
- ->write("--\$context['loop']['revindex'];\n")
- ->write("\$context['loop']['last'] = 0 === \$context['loop']['revindex0'];\n")
- ->outdent()
- ->write("}\n")
- ;
- }
}
}
}
diff --git a/src/Node/ForNode.php b/src/Node/ForNode.php
index ac65adc66..fd7ea689e 100644
--- a/src/Node/ForNode.php
+++ b/src/Node/ForNode.php
@@ -25,20 +25,16 @@ class ForNode extends Node
{
private $loop;
- public function __construct(AssignNameExpression $keyTarget, AssignNameExpression $valueTarget, AbstractExpression $seq, AbstractExpression $ifexpr = null, Node $body, Node $else = null, int $lineno, string $tag = null)
+ public function __construct(AssignNameExpression $keyTarget, AssignNameExpression $valueTarget, AbstractExpression $seq, $ifexpr = null, Node $body, Node $else = null, int $lineno, string $tag = null)
{
$body = new Node([$body, $this->loop = new ForLoopNode($lineno, $tag)]);
- if (null !== $ifexpr) {
- $body = new IfNode(new Node([$ifexpr, $body]), null, $lineno, $tag);
- }
-
$nodes = ['key_target' => $keyTarget, 'value_target' => $valueTarget, 'seq' => $seq, 'body' => $body];
if (null !== $else) {
$nodes['else'] = $else;
}
- parent::__construct($nodes, ['with_loop' => true, 'ifexpr' => null !== $ifexpr], $lineno, $tag);
+ parent::__construct($nodes, ['with_loop' => true], $lineno, $tag);
}
public function compile(Compiler $compiler): void
@@ -63,26 +59,20 @@ class ForNode extends Node
->write(" 'index' => 1,\n")
->write(" 'first' => true,\n")
->write("];\n")
+ ->write("if (is_array(\$context['_seq']) || (is_object(\$context['_seq']) && \$context['_seq'] instanceof \Countable)) {\n")
+ ->indent()
+ ->write("\$length = count(\$context['_seq']);\n")
+ ->write("\$context['loop']['revindex0'] = \$length - 1;\n")
+ ->write("\$context['loop']['revindex'] = \$length;\n")
+ ->write("\$context['loop']['length'] = \$length;\n")
+ ->write("\$context['loop']['last'] = 1 === \$length;\n")
+ ->outdent()
+ ->write("}\n")
;
-
- if (!$this->getAttribute('ifexpr')) {
- $compiler
- ->write("if (is_array(\$context['_seq']) || (is_object(\$context['_seq']) && \$context['_seq'] instanceof \Countable)) {\n")
- ->indent()
- ->write("\$length = count(\$context['_seq']);\n")
- ->write("\$context['loop']['revindex0'] = \$length - 1;\n")
- ->write("\$context['loop']['revindex'] = \$length;\n")
- ->write("\$context['loop']['length'] = \$length;\n")
- ->write("\$context['loop']['last'] = 1 === \$length;\n")
- ->outdent()
- ->write("}\n")
- ;
- }
}
$this->loop->setAttribute('else', $this->hasNode('else'));
$this->loop->setAttribute('with_loop', $this->getAttribute('with_loop'));
- $this->loop->setAttribute('ifexpr', $this->getAttribute('ifexpr'));
$compiler
->write("foreach (\$context['_seq'] as ")
diff --git a/src/TokenParser/ForTokenParser.php b/src/TokenParser/ForTokenParser.php
index df49b2fa3..0a6b740f5 100644
--- a/src/TokenParser/ForTokenParser.php
+++ b/src/TokenParser/ForTokenParser.php
@@ -12,15 +12,10 @@
namespace Twig\TokenParser;
-use Twig\Error\SyntaxError;
use Twig\Node\Expression\AssignNameExpression;
-use Twig\Node\Expression\ConstantExpression;
-use Twig\Node\Expression\GetAttrExpression;
-use Twig\Node\Expression\NameExpression;
use Twig\Node\ForNode;
use Twig\Node\Node;
use Twig\Token;
-use Twig\TokenStream;
/**
* Loops over each item of a sequence.
@@ -41,13 +36,6 @@ final class ForTokenParser extends AbstractTokenParser
$stream->expect(/* Token::OPERATOR_TYPE */ 8, 'in');
$seq = $this->parser->getExpressionParser()->parseExpression();
- $ifexpr = null;
- if ($stream->nextIf(/* Token::NAME_TYPE */ 5, 'if')) {
- @trigger_error(sprintf('Using an "if" condition on "for" tag is deprecated since Twig 2.10.0, use a "filter" filter or an "if" condition inside the "for" body instead (if your condition depends on a variable updated inside the loop).', __CLASS__), E_USER_DEPRECATED);
-
- $ifexpr = $this->parser->getExpressionParser()->parseExpression();
- }
-
$stream->expect(/* Token::BLOCK_END_TYPE */ 3);
$body = $this->parser->subparse([$this, 'decideForFork']);
if ('else' == $stream->next()->getValue()) {
@@ -69,12 +57,7 @@ final class ForTokenParser extends AbstractTokenParser
$valueTarget = new AssignNameExpression($valueTarget->getAttribute('name'), $valueTarget->getTemplateLine());
}
- if ($ifexpr) {
- $this->checkLoopUsageCondition($stream, $ifexpr);
- $this->checkLoopUsageBody($stream, $body);
- }
-
- return new ForNode($keyTarget, $valueTarget, $seq, $ifexpr, $body, $else, $lineno, $this->getTag());
+ return new ForNode($keyTarget, $valueTarget, $seq, null, $body, $else, $lineno, $this->getTag());
}
public function decideForFork(Token $token): bool
@@ -87,47 +70,6 @@ final class ForTokenParser extends AbstractTokenParser
return $token->test('endfor');
}
- // the loop variable cannot be used in the condition
- private function checkLoopUsageCondition(TokenStream $stream, Node $node): void
- {
- if ($node instanceof GetAttrExpression && $node->getNode('node') instanceof NameExpression && 'loop' == $node->getNode('node')->getAttribute('name')) {
- throw new SyntaxError('The "loop" variable cannot be used in a looping condition.', $node->getTemplateLine(), $stream->getSourceContext());
- }
-
- foreach ($node as $n) {
- if (!$n) {
- continue;
- }
-
- $this->checkLoopUsageCondition($stream, $n);
- }
- }
-
- // check usage of non-defined loop-items
- // it does not catch all problems (for instance when a for is included into another or when the variable is used in an include)
- private function checkLoopUsageBody(TokenStream $stream, Node $node): void
- {
- if ($node instanceof GetAttrExpression && $node->getNode('node') instanceof NameExpression && 'loop' == $node->getNode('node')->getAttribute('name')) {
- $attribute = $node->getNode('attribute');
- if ($attribute instanceof ConstantExpression && \in_array($attribute->getAttribute('value'), ['length', 'revindex0', 'revindex', 'last'])) {
- throw new SyntaxError(sprintf('The "loop.%s" variable is not defined when looping with a condition.', $attribute->getAttribute('value')), $node->getTemplateLine(), $stream->getSourceContext());
- }
- }
-
- // should check for parent.loop.XXX usage
- if ($node instanceof ForNode) {
- return;
- }
-
- foreach ($node as $n) {
- if (!$n) {
- continue;
- }
-
- $this->checkLoopUsageBody($stream, $n);
- }
- }
-
public function getTag(): string
{
return 'for';
diff --git a/test/Twig/Tests/Fixtures/tags/for/condition.legacy.test b/test/Twig/Tests/Fixtures/tags/for/condition.legacy.test
deleted file mode 100644
index 690207e42..000000000
--- a/test/Twig/Tests/Fixtures/tags/for/condition.legacy.test
+++ /dev/null
@@ -1,16 +0,0 @@
---TEST--
-"for" tag takes a condition
---DEPRECATION--
-Using an "if" condition on "for" tag is deprecated since Twig 2.10.0, use a "filter" filter or an "if" condition inside the "for" body instead (if your condition depends on a variable updated inside the loop).
---TEMPLATE--
-{% for i in 1..5 if i is odd -%}
- {{ loop.index }}.{{ i }}{{ foo.bar }}
-{% endfor %}
---DATA--
-return ['foo' => ['bar' => 'X']]
---CONFIG--
-return ['strict_variables' => false]
---EXPECT--
-1.1X
-2.3X
-3.5X
diff --git a/test/Twig/Tests/Fixtures/tags/for/loop_not_defined.test b/test/Twig/Tests/Fixtures/tags/for/loop_not_defined.test
deleted file mode 100644
index 2d8c0b873..000000000
--- a/test/Twig/Tests/Fixtures/tags/for/loop_not_defined.test
+++ /dev/null
@@ -1,10 +0,0 @@
---TEST--
-"for" tag
---TEMPLATE--
-{% for i, item in items if i > 0 %}
- {{ loop.last }}
-{% endfor %}
---DATA--
-return ['items' => ['a', 'b']]
---EXCEPTION--
-Twig\Error\SyntaxError: The "loop.last" variable is not defined when looping with a condition in "index.twig" at line 3.
diff --git a/test/Twig/Tests/Fixtures/tags/for/loop_not_defined_cond.test b/test/Twig/Tests/Fixtures/tags/for/loop_not_defined_cond.test
deleted file mode 100644
index e90e96b4a..000000000
--- a/test/Twig/Tests/Fixtures/tags/for/loop_not_defined_cond.test
+++ /dev/null
@@ -1,9 +0,0 @@
---TEST--
-"for" tag
---TEMPLATE--
-{% for i, item in items if loop.last > 0 %}
-{% endfor %}
---DATA--
-return ['items' => ['a', 'b']]
---EXCEPTION--
-Twig\Error\SyntaxError: The "loop" variable cannot be used in a looping condition in "index.twig" at line 2.
diff --git a/test/Twig/Tests/Node/ForTest.php b/test/Twig/Tests/Node/ForTest.php
index 47aefac4f..c868a04ef 100644
--- a/test/Twig/Tests/Node/ForTest.php
+++ b/test/Twig/Tests/Node/ForTest.php
@@ -10,10 +10,8 @@
*/
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;
@@ -25,22 +23,19 @@ class Twig_Tests_Node_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, $ifexpr, $body, $else, 1);
+ $node = new ForNode($keyTarget, $valueTarget, $seq, null, $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->assertTrue($node->getAttribute('ifexpr'));
- $this->assertInstanceOf(IfNode::class, $node->getNode('body'));
- $this->assertEquals($body, $node->getNode('body')->getNode('tests')->getNode(1)->getNode(0));
+ $this->assertEquals($body, $node->getNode('body')->getNode(0));
$this->assertFalse($node->hasNode('else'));
$else = new PrintNode(new NameExpression('foo', 1), 1);
- $node = new ForNode($keyTarget, $valueTarget, $seq, $ifexpr, $body, $else, 1);
+ $node = new ForNode($keyTarget, $valueTarget, $seq, null, $body, $else, 1);
$node->setAttribute('with_loop', false);
$this->assertEquals($else, $node->getNode('else'));
}
@@ -52,10 +47,9 @@ class Twig_Tests_Node_ForTest extends NodeTestCase
$keyTarget = new AssignNameExpression('key', 1);
$valueTarget = new AssignNameExpression('item', 1);
$seq = new NameExpression('items', 1);
- $ifexpr = null;
$body = new Node([new PrintNode(new NameExpression('foo', 1), 1)], [], 1);
$else = null;
- $node = new ForNode($keyTarget, $valueTarget, $seq, $ifexpr, $body, $else, 1);
+ $node = new ForNode($keyTarget, $valueTarget, $seq, null, $body, $else, 1);
$node->setAttribute('with_loop', false);
$tests[] = [$node, <<setAttribute('with_loop', true);
$tests[] = [$node, <<setAttribute('with_loop', true);
$tests[] = [$node, << 1,
'first' => true,
];
+if (is_array(\$context['_seq']) || (is_object(\$context['_seq']) && \$context['_seq'] instanceof \Countable)) {
+ \$length = count(\$context['_seq']);
+ \$context['loop']['revindex0'] = \$length - 1;
+ \$context['loop']['revindex'] = \$length;
+ \$context['loop']['length'] = \$length;
+ \$context['loop']['last'] = 1 === \$length;
+}
foreach (\$context['_seq'] as \$context["k"] => \$context["v"]) {
- if (true) {
- echo {$this->getVariableGetter('foo')};
- ++\$context['loop']['index0'];
- ++\$context['loop']['index'];
- \$context['loop']['first'] = false;
+ echo {$this->getVariableGetter('foo')};
+ ++\$context['loop']['index0'];
+ ++\$context['loop']['index'];
+ \$context['loop']['first'] = false;
+ if (isset(\$context['loop']['length'])) {
+ --\$context['loop']['revindex0'];
+ --\$context['loop']['revindex'];
+ \$context['loop']['last'] = 0 === \$context['loop']['revindex0'];
}
}
\$_parent = \$context['_parent'];
@@ -150,10 +152,9 @@ EOF
$keyTarget = new AssignNameExpression('k', 1);
$valueTarget = new AssignNameExpression('v', 1);
$seq = new NameExpression('values', 1);
- $ifexpr = null;
$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, $ifexpr, $body, $else, 1);
+ $node = new ForNode($keyTarget, $valueTarget, $seq, null, $body, $else, 1);
$node->setAttribute('with_loop', true);
$tests[] = [$node, <<