Skip to content

Commit ae82e8b

Browse files
committed
feature #4479 Simplify EscaperNodeVisitor code (fabpot)
This PR was merged into the 3.x branch. Discussion ---------- Simplify EscaperNodeVisitor code This PR simplifies how we escape the ternary operator, and simplifies the whole strategy. Instead of replacing the `PrintNode`s, we instead "just" wrap the inner expressions. For the ternary operator, the `InlinePrint` expression is useless and even weird as printing something in the middle of an expression looks very wrong. This is not done anymore and the node is deprecated. Overall, this PR makes fewer changes to the Node tree which should make things use a bit less memory. Commits ------- 02cec77 Simplify EscaperNodeVisitor code
2 parents c384fb4 + 02cec77 commit ae82e8b

File tree

4 files changed

+15
-36
lines changed

4 files changed

+15
-36
lines changed

CHANGELOG

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# 3.16.0 (2024-XX-XX)
22

3+
* Deprecate `InlinePrint`
34
* Fix having macro variables starting with an underscore
45
* Deprecate not passing a `Source` instance to `TokenStream`
56
* Deprecate returning `null` from `TwigFilter::getSafe()` and `TwigFunction::getSafe()`, return `[]` instead

doc/deprecated.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,8 @@ Nodes
189189
made ready for ``yield``; the ``use_yield`` Environment option can be turned
190190
on when all nodes use the ``#[\Twig\Attribute\YieldReady]`` attribute.
191191

192+
* The ``InlinePrint`` class is deprecated as of Twig 3.16 with no replacement.
193+
192194
Node Visitors
193195
-------------
194196

src/Node/Expression/InlinePrint.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,7 @@ final class InlinePrint extends AbstractExpression
2424
*/
2525
public function __construct(Node $node, int $lineno)
2626
{
27-
if (!$node instanceof AbstractExpression) {
28-
trigger_deprecation('twig/twig', '3.15', 'Not passing a "%s" instance to the "node" argument of "%s" is deprecated ("%s" given).', AbstractExpression::class, static::class, get_class($node));
29-
}
27+
trigger_deprecation('twig/twig', '3.16', \sprintf('The "%s" class is deprecated with no replacement.', static::class));
3028

3129
parent::__construct(['node' => $node], [], $lineno);
3230
}

src/NodeVisitor/EscaperNodeVisitor.php

Lines changed: 11 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,10 @@
1616
use Twig\Node\AutoEscapeNode;
1717
use Twig\Node\BlockNode;
1818
use Twig\Node\BlockReferenceNode;
19-
use Twig\Node\DoNode;
2019
use Twig\Node\Expression\AbstractExpression;
2120
use Twig\Node\Expression\ConditionalExpression;
2221
use Twig\Node\Expression\ConstantExpression;
2322
use Twig\Node\Expression\FilterExpression;
24-
use Twig\Node\Expression\InlinePrint;
2523
use Twig\Node\ImportNode;
2624
use Twig\Node\ModuleNode;
2725
use Twig\Node\Node;
@@ -78,10 +76,12 @@ public function leaveNode(Node $node, Environment $env): ?Node
7876
} elseif ($node instanceof PrintNode && false !== $type = $this->needEscaping()) {
7977
$expression = $node->getNode('expr');
8078
if ($expression instanceof ConditionalExpression) {
81-
return new DoNode($this->unwrapConditional($expression, $env, $type), $expression->getTemplateLine());
79+
$node->setNode('expr', $this->escapeConditional($expression, $env, $type));
80+
} else {
81+
$node->setNode('expr', $this->escapeExpression($expression, $env, $type));
8282
}
8383

84-
return $this->escapePrintNode($node, $env, $type);
84+
return $node;
8585
}
8686

8787
if ($node instanceof AutoEscapeNode || $node instanceof BlockNode) {
@@ -93,22 +93,21 @@ public function leaveNode(Node $node, Environment $env): ?Node
9393
return $node;
9494
}
9595

96-
private function unwrapConditional(ConditionalExpression $expression, Environment $env, string $type): ConditionalExpression
96+
private function escapeConditional(ConditionalExpression $expression, Environment $env, string $type): ConditionalExpression
9797
{
98-
// convert "echo a ? b : c" to "a ? echo b : echo c" recursively
9998
/** @var AbstractExpression $expr2 */
10099
$expr2 = $expression->getNode('expr2');
101100
if ($expr2 instanceof ConditionalExpression) {
102-
$expr2 = $this->unwrapConditional($expr2, $env, $type);
101+
$expr2 = $this->escapeConditional($expr2, $env, $type);
103102
} else {
104-
$expr2 = $this->escapeInlinePrintNode(new InlinePrint($expr2, $expr2->getTemplateLine()), $env, $type);
103+
$expr2 = $this->escapeExpression($expr2, $env, $type);
105104
}
106105
/** @var AbstractExpression $expr3 */
107106
$expr3 = $expression->getNode('expr3');
108107
if ($expr3 instanceof ConditionalExpression) {
109-
$expr3 = $this->unwrapConditional($expr3, $env, $type);
108+
$expr3 = $this->escapeConditional($expr3, $env, $type);
110109
} else {
111-
$expr3 = $this->escapeInlinePrintNode(new InlinePrint($expr3, $expr3->getTemplateLine()), $env, $type);
110+
$expr3 = $this->escapeExpression($expr3, $env, $type);
112111
}
113112

114113
/** @var AbstractExpression $expr1 */
@@ -117,30 +116,9 @@ private function unwrapConditional(ConditionalExpression $expression, Environmen
117116
return new ConditionalExpression($expr1, $expr2, $expr3, $expression->getTemplateLine());
118117
}
119118

120-
private function escapeInlinePrintNode(InlinePrint $node, Environment $env, string $type): AbstractExpression
119+
private function escapeExpression(AbstractExpression $expression, Environment $env, string $type): AbstractExpression
121120
{
122-
/** @var AbstractExpression $expression */
123-
$expression = $node->getNode('node');
124-
125-
if ($this->isSafeFor($type, $expression, $env)) {
126-
return $node;
127-
}
128-
129-
return new InlinePrint($this->getEscaperFilter($env, $type, $expression), $node->getTemplateLine());
130-
}
131-
132-
private function escapePrintNode(PrintNode $node, Environment $env, string $type): Node
133-
{
134-
/** @var AbstractExpression $expression */
135-
$expression = $node->getNode('expr');
136-
137-
if ($this->isSafeFor($type, $expression, $env)) {
138-
return $node;
139-
}
140-
141-
$class = \get_class($node);
142-
143-
return new $class($this->getEscaperFilter($env, $type, $expression), $node->getTemplateLine());
121+
return $this->isSafeFor($type, $expression, $env) ? $expression : $this->getEscaperFilter($env, $type, $expression);
144122
}
145123

146124
private function preEscapeFilterNode(FilterExpression $filter, Environment $env): FilterExpression

0 commit comments

Comments
 (0)