Merge branch '3.x' into 4.x

* 3.x:
  Simplify EscaperNodeVisitor code
  Fix having macro variables starting with an underscore
  Fix CS
  Fix EscapeNodeVisitor::isSafeFor()
  Simplify code

src/Node/Expression/InlinePrint.php

@@ -1,33 +0,0 @@
-<?php
-
-/*
- * This file is part of Twig.
- *
- * (c) Fabien Potencier
- *
- * For the full copyright and license information, please view the LICENSE
- * file that was distributed with this source code.
- */
-
-namespace Twig\Node\Expression;
-
-use Twig\Compiler;
-
-/**
- * @internal
- */
-final class InlinePrint extends AbstractExpression
-{
-    public function __construct(AbstractExpression $node, int $lineno)
-    {
-        parent::__construct(['node' => $node], [], $lineno);
-    }
-
-    public function compile(Compiler $compiler): void
-    {
-        $compiler
-            ->raw('yield ')
-            ->subcompile($this->getNode('node'))
-        ;
-    }
-}

src/Node/MacroNode.php

@@ -14,7 +14,6 @@ namespace Twig\Node;
 use Twig\Attribute\YieldReady;
 use Twig\Compiler;
 use Twig\Error\SyntaxError;
-use Twig\Markup;
 use Twig\Node\Expression\ArrayExpression;
 use Twig\Node\Expression\Variable\LocalVariable;
 
@@ -31,7 +30,7 @@ class MacroNode extends Node
     public function __construct(string $name, BodyNode $body, ArrayExpression $arguments, int $lineno)
     {
         foreach ($arguments->getKeyValuePairs() as $pair) {
-            if ('_'.self::VARARGS_NAME.'_' === $pair['key']->getAttribute('name')) {
+            if ("\u{035C}".self::VARARGS_NAME === $pair['key']->getAttribute('name')) {
                 throw new SyntaxError(\sprintf('The argument "%s" in macro "%s" cannot be defined because the variable "%s" is reserved for arbitrary arguments.', self::VARARGS_NAME, $name, self::VARARGS_NAME), $pair['value']->getTemplateLine(), $pair['value']->getSourceContext());
             }
         }
@@ -71,9 +70,13 @@ class MacroNode extends Node
 
         foreach ($arguments->getKeyValuePairs() as $pair) {
             $name = $pair['key'];
+            $var = $name->getAttribute('name');
+            if (str_starts_with($var, "\u{035C}")) {
+                $var = substr($var, \strlen("\u{035C}"));
+            }
             $compiler
                 ->write('')
-                ->string(trim($name->getAttribute('name'), '_'))
+                ->string($var)
                 ->raw(' => ')
                 ->subcompile($name)
                 ->raw(",\n")

src/NodeVisitor/EscaperNodeVisitor.php

@@ -16,12 +16,10 @@ use Twig\Extension\EscaperExtension;
 use Twig\Node\AutoEscapeNode;
 use Twig\Node\BlockNode;
 use Twig\Node\BlockReferenceNode;
-use Twig\Node\DoNode;
 use Twig\Node\Expression\AbstractExpression;
 use Twig\Node\Expression\ConditionalExpression;
 use Twig\Node\Expression\ConstantExpression;
 use Twig\Node\Expression\FilterExpression;
-use Twig\Node\Expression\InlinePrint;
 use Twig\Node\ImportNode;
 use Twig\Node\ModuleNode;
 use Twig\Node\Node;
@@ -83,11 +81,13 @@ final class EscaperNodeVisitor implements NodeVisitorInterface
             return $this->preEscapeFilterNode($node, $env);
         } elseif ($node instanceof PrintNode && false !== $type = $this->needEscaping()) {
             $expression = $node->getNode('expr');
-            if ($expression instanceof ConditionalExpression && $this->shouldUnwrapConditional($expression, $env, $type)) {
-                return new DoNode($this->unwrapConditional($expression, $env, $type), $expression->getTemplateLine());
+            if ($expression instanceof ConditionalExpression) {
+                $node->setNode('expr', $this->escapeConditional($expression, $env, $type));
+            } else {
+                $node->setNode('expr', $this->escapeExpression($expression, $env, $type));
             }
 
-            return $this->escapePrintNode($node, $env, $type);
+            return $node;
         }
 
         if ($node instanceof AutoEscapeNode || $node instanceof BlockNode) {
@@ -99,35 +99,21 @@ final class EscaperNodeVisitor implements NodeVisitorInterface
         return $node;
     }
 
-    private function shouldUnwrapConditional(ConditionalExpression $expression, Environment $env, string $type): bool
+    private function escapeConditional(ConditionalExpression $expression, Environment $env, string $type): ConditionalExpression
     {
         /** @var AbstractExpression $expr2 */
         $expr2 = $expression->getNode('expr2');
-        /** @var AbstractExpression $expr3 */
-        $expr3 = $expression->getNode('expr3');
-
-        $expr2Safe = $this->isSafeFor($type, $expr2, $env);
-        $expr3Safe = $this->isSafeFor($type, $expr3, $env);
-
-        return $expr2Safe !== $expr3Safe;
-    }
-
-    private function unwrapConditional(ConditionalExpression $expression, Environment $env, string $type): ConditionalExpression
-    {
-        // convert "echo a ? b : c" to "a ? echo b : echo c" recursively
-        /** @var AbstractExpression $expr2 */
-        $expr2 = $expression->getNode('expr2');
-        if ($expr2 instanceof ConditionalExpression && $this->shouldUnwrapConditional($expr2, $env, $type)) {
-            $expr2 = $this->unwrapConditional($expr2, $env, $type);
+        if ($expr2 instanceof ConditionalExpression) {
+            $expr2 = $this->escapeConditional($expr2, $env, $type);
         } else {
-            $expr2 = $this->escapeInlinePrintNode(new InlinePrint($expr2, $expr2->getTemplateLine()), $env, $type);
+            $expr2 = $this->escapeExpression($expr2, $env, $type);
         }
         /** @var AbstractExpression $expr3 */
         $expr3 = $expression->getNode('expr3');
-        if ($expr3 instanceof ConditionalExpression && $this->shouldUnwrapConditional($expr3, $env, $type)) {
-            $expr3 = $this->unwrapConditional($expr3, $env, $type);
+        if ($expr3 instanceof ConditionalExpression) {
+            $expr3 = $this->escapeConditional($expr3, $env, $type);
         } else {
-            $expr3 = $this->escapeInlinePrintNode(new InlinePrint($expr3, $expr3->getTemplateLine()), $env, $type);
+            $expr3 = $this->escapeExpression($expr3, $env, $type);
         }
 
         /** @var AbstractExpression $expr1 */
@@ -136,30 +122,9 @@ final class EscaperNodeVisitor implements NodeVisitorInterface
         return new ConditionalExpression($expr1, $expr2, $expr3, $expression->getTemplateLine());
     }
 
-    private function escapeInlinePrintNode(InlinePrint $node, Environment $env, string $type): AbstractExpression
+    private function escapeExpression(AbstractExpression $expression, Environment $env, string $type): AbstractExpression
     {
-        /** @var AbstractExpression $expression */
-        $expression = $node->getNode('node');
-
-        if ($this->isSafeFor($type, $expression, $env)) {
-            return $node;
-        }
-
-        return new InlinePrint($this->getEscaperFilter($env, $type, $expression), $node->getTemplateLine());
-    }
-
-    private function escapePrintNode(PrintNode $node, Environment $env, string $type): Node
-    {
-        /** @var AbstractExpression $expression */
-        $expression = $node->getNode('expr');
-
-        if ($this->isSafeFor($type, $expression, $env)) {
-            return $node;
-        }
-
-        $class = $node::class;
-
-        return new $class($this->getEscaperFilter($env, $type, $expression), $node->getTemplateLine());
+        return $this->isSafeFor($type, $expression, $env) ? $expression : $this->getEscaperFilter($env, $type, $expression);
     }
 
     private function preEscapeFilterNode(FilterExpression $filter, Environment $env): FilterExpression
@@ -183,7 +148,7 @@ final class EscaperNodeVisitor implements NodeVisitorInterface
     {
         $safe = $this->safeAnalysis->getSafe($expression);
 
-        if (null === $safe) {
+        if (!$safe) {
             if (null === $this->traverser) {
                 $this->traverser = new NodeTraverser($env, [$this->safeAnalysis]);
             }

src/NodeVisitor/SafeAnalysisNodeVisitor.php

@@ -36,11 +36,14 @@ final class SafeAnalysisNodeVisitor implements NodeVisitorInterface
         $this->safeVars = $safeVars;
     }
 
+    /**
+     * @return array
+     */
     public function getSafe(Node $node)
     {
         $hash = spl_object_hash($node);
         if (!isset($this->data[$hash])) {
-            return;
+            return [];
         }
 
         foreach ($this->data[$hash] as $bucket) {
@@ -54,6 +57,8 @@ final class SafeAnalysisNodeVisitor implements NodeVisitorInterface
 
             return $bucket['value'];
         }
+
+        return [];
     }
 
     private function setSafe(Node $node, array $safe): void
@@ -99,11 +104,14 @@ final class SafeAnalysisNodeVisitor implements NodeVisitorInterface
             if ($filter = $node->getAttribute('twig_callable')) {
                 $safe = $filter->getSafe($node->getNode('arguments'));
                 if (null === $safe) {
+                    trigger_deprecation('twig/twig', '3.16', 'The "%s::getSafe()" method should not return "null" anymore, return "[]" instead.', $filter::class);
+                    $safe = [];
+                }
+
+                if (!$safe) {
                     $safe = $this->intersectSafe($this->getSafe($node->getNode('node')), $filter->getPreservesSafety());
                 }
                 $this->setSafe($node, $safe);
-            } else {
-                $this->setSafe($node, []);
             }
         } elseif ($node instanceof FunctionExpression) {
             // function expression is safe when the function is safe
@@ -119,19 +127,15 @@ final class SafeAnalysisNodeVisitor implements NodeVisitorInterface
             $name = $node->getNode('node')->getAttribute('name');
             if (\in_array($name, $this->safeVars)) {
                 $this->setSafe($node, ['all']);
-            } else {
-                $this->setSafe($node, []);
             }
-        } else {
-            $this->setSafe($node, []);
         }
 
         return $node;
     }
 
-    private function intersectSafe(?array $a = null, ?array $b = null): array
+    private function intersectSafe(array $a, array $b): array
     {
-        if (null === $a || null === $b) {
+        if (!$a || !$b) {
             return [];
         }
 

src/TwigFilter.php

@@ -54,12 +54,12 @@ final class TwigFilter extends AbstractTwigCallable
             return $this->options['is_safe_callback']($filterArgs);
         }
 
-        return null;
+        return [];
     }
 
-    public function getPreservesSafety(): ?array
+    public function getPreservesSafety(): array
     {
-        return $this->options['preserves_safety'];
+        return $this->options['preserves_safety'] ?? [];
     }
 
     public function getPreEscape(): ?string

tests/Node/MacroTest.php

@@ -42,6 +42,8 @@ class MacroTest extends NodeTestCase
             new ConstantExpression(null, 1),
             new LocalVariable('bar', 1),
             new ConstantExpression('Foo', 1),
+            new LocalVariable('_underscore', 1),
+            new ConstantExpression(null, 1),
         ], 1);
 
         $body = new BodyNode([new TextNode('foo', 1)]);
@@ -49,12 +51,13 @@ class MacroTest extends NodeTestCase
 
         yield 'with use_yield = true' => [$node, <<<EOF
 // line 1
-public function macro_foo(\$foo = null, \$bar = "Foo", ...\$varargs): string|Markup
+public function macro_foo(\$foo = null, \$bar = "Foo", \$_underscore = null, ...\$varargs): string|Markup
 {
     \$macros = \$this->macros;
     \$context = [
         "foo" => \$foo,
         "bar" => \$bar,
+        "_underscore" => \$_underscore,
         "varargs" => \$varargs,
     ] + \$this->env->getGlobals();
 
@@ -71,12 +74,13 @@ EOF
 
         yield 'with use_yield = false' => [$node, <<<EOF
 // line 1
-public function macro_foo(\$foo = null, \$bar = "Foo", ...\$varargs): string|Markup
+public function macro_foo(\$foo = null, \$bar = "Foo", \$_underscore = null, ...\$varargs): string|Markup
 {
     \$macros = \$this->macros;
     \$context = [
         "foo" => \$foo,
         "bar" => \$bar,
+        "_underscore" => \$_underscore,
         "varargs" => \$varargs,
     ] + \$this->env->getGlobals();