removed the "if" condition support on the "for" tag

This commit is contained in:
Fabien Potencier
2019-05-09 17:38:13 +02:00
parent 7c16b12326
commit dca3f74e49
9 changed files with 58 additions and 159 deletions
+1
View File
@@ -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
+16 -11
View File
@@ -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
<ul>
{% for user in users if user.active %}
{% for user in users|filter(user => user.active) %}
<li>{{ user.username|e }}</li>
{% endfor %}
</ul>
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
<ul>
{% set c = false %}
{% for i, user in users %}
{% if i is even or c %}
<li>{{ user.username|e }}</li>
{% set c = i > 5 %}
{% endif %}
{% endfor %}
</ul>
The `else` Clause
-----------------
+7 -12
View File
@@ -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")
;
}
}
}
}
+11 -21
View File
@@ -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 ")
+1 -59
View File
@@ -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';
@@ -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
@@ -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.
@@ -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.
+22 -21
View File
@@ -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, <<<EOF
@@ -74,10 +68,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 = 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', true);
$tests[] = [$node, <<<EOF
@@ -117,10 +110,9 @@ 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 = 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', true);
$tests[] = [$node, <<<EOF
@@ -133,12 +125,22 @@ EOF
'index' => 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, <<<EOF