Skip to content

Commit cf46de1

Browse files
committed
Introduce a CorrectnessNodeVisitor to validate that templates are semantically correct
1 parent a0093cd commit cf46de1

27 files changed

Lines changed: 568 additions & 164 deletions

CHANGELOG

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# 3.27.2 (2026-XX-XX)
22

3+
* Deprecate the possibility to use a `block` tag within a capture node (like `set`)
4+
* Throw an exception when a `macro`, `extends`, or `use` tag is used outside the root of a template
35
* Stop reporting a skipped test in `IntegrationTestCase` when there is no legacy test to run
46
* Make the `IntegrationTestCase` and `NodeTestCase` test helpers compatible with PHPUnit 11
57
* Cast printed expressions to string so values that cannot be converted to a string (arrays, non-`Stringable` objects, ...) report a usable stack trace at the print location

doc/deprecated.rst

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,29 @@ Templates
291291
in ``Environment::resolveTemplate()`` and ``Environment::load()``); pass
292292
instances of ``Twig\TemplateWrapper`` instead.
293293

294+
* Having a "block" definition nested in another node that captures the output
295+
(like "set") is deprecated in Twig 3.14 and will throw in Twig 4.0. Such use
296+
cases should be avoided as the "block" tag is used to both define the block
297+
AND display it in place. Here is how you can decouple both easily:
298+
299+
Before::
300+
301+
{% extends "layout.twig" %}
302+
303+
{% set str %}
304+
{% block content %}Some content{% endblock %}
305+
{% endset %}
306+
307+
After::
308+
309+
{% extends "layout.twig" %}
310+
311+
{% block content %}Some content{% endblock %}
312+
313+
{% set str %}
314+
{{ block('content') }}
315+
{% endset %}
316+
294317
Filters
295318
-------
296319

src/Extension/CoreExtension.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
use Twig\Node\Expression\Unary\PosUnary;
8686
use Twig\Node\Expression\Unary\SpreadUnary;
8787
use Twig\Node\Node;
88+
use Twig\NodeVisitor\CorrectnessNodeVisitor;
8889
use Twig\Parser;
8990
use Twig\Sandbox\SecurityNotAllowedMethodError;
9091
use Twig\Sandbox\SecurityNotAllowedPropertyError;
@@ -328,7 +329,9 @@ public function getTests(): array
328329

329330
public function getNodeVisitors(): array
330331
{
331-
return [];
332+
return [
333+
new CorrectnessNodeVisitor(),
334+
];
332335
}
333336

334337
public function getExpressionParsers(): array

src/Node/ConfigNode.php

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
/*
4+
* This file is part of Twig.
5+
*
6+
* (c) Fabien Potencier
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Twig\Node;
13+
14+
use Twig\Attribute\YieldReady;
15+
16+
/**
17+
* Represents a node that has global side effects but does not generate template code.
18+
*
19+
* Such nodes must be at the root level of the body of a template.
20+
*
21+
* @author Fabien Potencier <fabien@symfony.com>
22+
*/
23+
#[YieldReady]
24+
final class ConfigNode extends Node
25+
{
26+
public function __construct(int $lineno)
27+
{
28+
parent::__construct([], [], $lineno);
29+
}
30+
}

src/Node/TextNode.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,20 @@ public function compile(Compiler $compiler): void
3838
->raw(";\n")
3939
;
4040
}
41+
42+
public function isBlank(): bool
43+
{
44+
if (ctype_space($this->getAttribute('data'))) {
45+
return true;
46+
}
47+
48+
if (str_contains((string) $this, \chr(0xEF).\chr(0xBB).\chr(0xBF))) {
49+
$t = substr($this->getAttribute('data'), 3);
50+
if ('' === $t || ctype_space($t)) {
51+
return true;
52+
}
53+
}
54+
55+
return false;
56+
}
4157
}
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
<?php
2+
3+
/*
4+
* This file is part of Twig.
5+
*
6+
* (c) Fabien Potencier
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Twig\NodeVisitor;
13+
14+
use Twig\Environment;
15+
use Twig\Error\SyntaxError;
16+
use Twig\Node\BlockReferenceNode;
17+
use Twig\Node\ConfigNode;
18+
use Twig\Node\ModuleNode;
19+
use Twig\Node\Node;
20+
use Twig\Node\NodeCaptureInterface;
21+
use Twig\Node\Nodes;
22+
use Twig\Node\TextNode;
23+
24+
/**
25+
* @author Fabien Potencier <fabien@symfony.com>
26+
*
27+
* @internal
28+
*/
29+
final class CorrectnessNodeVisitor implements NodeVisitorInterface
30+
{
31+
private ?\WeakMap $rootNodes = null;
32+
// in a tag node that does not support "block" nodes (all of them except "block")
33+
private ?Node $currentTagNode = null;
34+
private bool $hasParent = false;
35+
private ?\WeakMap $blockNodes = null;
36+
private int $currentBlockNodeLevel = 0;
37+
38+
public function enterNode(Node $node, Environment $env): Node
39+
{
40+
if ($node instanceof ModuleNode) {
41+
$this->rootNodes = new \WeakMap();
42+
$this->hasParent = $node->hasNode('parent');
43+
44+
// allows to identify when we enter/leave the block nodes
45+
$this->blockNodes = new \WeakMap();
46+
foreach ($node->getNode('blocks') as $n) {
47+
$this->blockNodes[$n] = true;
48+
}
49+
50+
$body = $node->getNode('body')->getNode('0');
51+
// see Parser::subparse() which does not wrap the parsed Nodes if there is only one node
52+
foreach (\count($body) ? $body : new Nodes([$body]) as $k => $n) {
53+
// check that this root node of a child template only contains empty output nodes
54+
if ($this->hasParent && !$this->isEmptyOutputNode($n)) {
55+
throw new SyntaxError('A template that extends another one cannot include content outside Twig blocks. Did you forget to put the content inside a {% block %} tag?', $n->getTemplateLine(), $n->getSourceContext());
56+
}
57+
$this->rootNodes[$n] = true;
58+
}
59+
60+
return $node;
61+
}
62+
63+
if (isset($this->blockNodes[$node])) {
64+
++$this->currentBlockNodeLevel;
65+
}
66+
67+
if ($this->hasParent && $node->getNodeTag() && !$node instanceof BlockReferenceNode) {
68+
$this->currentTagNode = $node;
69+
}
70+
71+
if ($node instanceof ConfigNode && !isset($this->rootNodes[$node])) {
72+
throw new SyntaxError(\sprintf('The "%s" tag must always be at the root of the body of a template.', $node->getNodeTag()), $node->getTemplateLine(), $node->getSourceContext());
73+
}
74+
75+
if ($this->currentTagNode && $node instanceof BlockReferenceNode) {
76+
if ($this->currentTagNode instanceof NodeCaptureInterface || \count($this->blockNodes) > 1) {
77+
trigger_deprecation('twig/twig', '3.14', \sprintf('Having a "block" tag under a "%s" tag (line %d) is deprecated in %s at line %d.', $this->currentTagNode->getNodeTag(), $this->currentTagNode->getTemplateLine(), $node->getSourceContext()->getName(), $node->getTemplateLine()));
78+
} else {
79+
throw new SyntaxError(\sprintf('A "block" tag cannot be under a "%s" tag (line %d).', $this->currentTagNode->getNodeTag(), $this->currentTagNode->getTemplateLine()), $node->getTemplateLine(), $node->getSourceContext());
80+
}
81+
}
82+
83+
return $node;
84+
}
85+
86+
public function leaveNode(Node $node, Environment $env): Node
87+
{
88+
if ($node instanceof ModuleNode) {
89+
$this->rootNodes = null;
90+
$this->hasParent = false;
91+
$this->blockNodes = null;
92+
$this->currentBlockNodeLevel = 0;
93+
}
94+
if ($this->hasParent && $node->getNodeTag() && !$node instanceof BlockReferenceNode) {
95+
$this->currentTagNode = null;
96+
}
97+
if ($this->hasParent && isset($this->blockNodes[$node])) {
98+
--$this->currentBlockNodeLevel;
99+
}
100+
101+
return $node;
102+
}
103+
104+
public function getPriority(): int
105+
{
106+
return -255;
107+
}
108+
109+
/**
110+
* Returns true if the node never outputs anything or if the output is empty.
111+
*/
112+
private function isEmptyOutputNode(Node $node): bool
113+
{
114+
if ($node instanceof NodeCaptureInterface) {
115+
// a "block" tag in such a node will serve as a block definition AND be displayed in place as well
116+
return true;
117+
}
118+
119+
// Can the text be considered "empty" (only whitespace)?
120+
if ($node instanceof TextNode) {
121+
return $node->isBlank();
122+
}
123+
124+
foreach ($node as $n) {
125+
if (!$this->isEmptyOutputNode($n)) {
126+
return false;
127+
}
128+
}
129+
130+
return true;
131+
}
132+
}

src/Parser.php

Lines changed: 12 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@
2929
use Twig\Node\MacroNode;
3030
use Twig\Node\ModuleNode;
3131
use Twig\Node\Node;
32-
use Twig\Node\NodeCaptureInterface;
33-
use Twig\Node\NodeOutputInterface;
3432
use Twig\Node\Nodes;
3533
use Twig\Node\PrintNode;
3634
use Twig\Node\TextNode;
@@ -108,10 +106,6 @@ public function parse(TokenStream $stream, $test = null, bool $dropNeedle = fals
108106

109107
try {
110108
$body = $this->subparse($test, $dropNeedle);
111-
112-
if (null !== $this->parent && null === $body = $this->filterBodyNodes($body)) {
113-
$body = new EmptyNode();
114-
}
115109
} catch (SyntaxError $e) {
116110
if (!$e->getSourceContext()) {
117111
$e->setSourceContext($this->stream->getSourceContext());
@@ -126,6 +120,10 @@ public function parse(TokenStream $stream, $test = null, bool $dropNeedle = fals
126120
$this->expressionRefs = null;
127121
}
128122

123+
if ($this->parent) {
124+
$this->cleanupBodyForChildTemplates($body);
125+
}
126+
129127
$node = new ModuleNode(
130128
new BodyNode([$body]),
131129
$this->parent,
@@ -550,52 +548,17 @@ public function getTest(int $line): TwigTest
550548
return $test;
551549
}
552550

553-
private function filterBodyNodes(Node $node, bool $nested = false): ?Node
551+
private function cleanupBodyForChildTemplates(Node $body): void
554552
{
555-
// check that the body does not contain non-empty output nodes
556-
if (
557-
($node instanceof TextNode && !ctype_space($node->getAttribute('data')))
558-
|| (!$node instanceof TextNode && !$node instanceof BlockReferenceNode && $node instanceof NodeOutputInterface)
559-
) {
560-
if (str_contains((string) $node, \chr(0xEF).\chr(0xBB).\chr(0xBF))) {
561-
$t = substr($node->getAttribute('data'), 3);
562-
if ('' === $t || ctype_space($t)) {
563-
// bypass empty nodes starting with a BOM
564-
return null;
565-
}
553+
foreach ($body as $k => $node) {
554+
if ($node instanceof BlockReferenceNode) {
555+
// as it has a parent, the block reference won't be used
556+
$body->removeNode($k);
557+
} elseif ($node instanceof TextNode && $node->isBlank()) {
558+
// remove nodes considered as "empty"
559+
$body->removeNode($k);
566560
}
567-
568-
throw new SyntaxError('A template that extends another one cannot include content outside Twig blocks. Did you forget to put the content inside a {% block %} tag?', $node->getTemplateLine(), $this->stream->getSourceContext());
569-
}
570-
571-
// bypass nodes that "capture" the output
572-
if ($node instanceof NodeCaptureInterface) {
573-
// a "block" tag in such a node will serve as a block definition AND be displayed in place as well
574-
return $node;
575561
}
576-
577-
// "block" tags that are not captured (see above) are only used for defining
578-
// the content of the block. In such a case, nesting it does not work as
579-
// expected as the definition is not part of the default template code flow.
580-
if ($nested && $node instanceof BlockReferenceNode) {
581-
throw new SyntaxError('A block definition cannot be nested under non-capturing nodes.', $node->getTemplateLine(), $this->stream->getSourceContext());
582-
}
583-
584-
if ($node instanceof NodeOutputInterface) {
585-
return null;
586-
}
587-
588-
// here, $nested means "being at the root level of a child template"
589-
// we need to discard the wrapping "Node" for the "body" node
590-
// Node::class !== \get_class($node) should be removed in Twig 4.0
591-
$nested = $nested || (Node::class !== $node::class && !$node instanceof Nodes);
592-
foreach ($node as $k => $n) {
593-
if (null !== $n && null === $this->filterBodyNodes($n, $nested)) {
594-
$node->removeNode($k);
595-
}
596-
}
597-
598-
return $node;
599562
}
600563

601564
private function checkPrecedenceDeprecations(ExpressionParserInterface $expressionParser, AbstractExpression $expr)

0 commit comments

Comments
 (0)