Repository: twigphp/Twig (php)

Issue:
Ability to mark nodes as implicitly safe
What is considered safe or not in the AST, is currently hardcoded in the `SafeAnalysisNodeVisitor`. For instance, a node that is an instance of `ConstantExpression`, is considered safe in all contexts. So when I'm creating my own expressions (that are safe by definition), I need to extend from `ConstantExpression` for the safe analysis to still work and the escaper node visitor not affecting the output. 

In https://github.com/twigphp/Twig/commit/c44bd1ff627d98dfd2ee1ed944af004b656d534e we're suddenly making assumptions about the class from outside. This will break child classes: In the above example a `PrintNode` wrapping a `FooConstantExpression` now gets converted to a `TextNode` ignoring the custom implementation.

How to deal with this? IMHO the ideal way would be to mark the `ConstantExpression` as `@final` and introduce an interface/flag for nodes, that marks them as safe in a certain or all contexts. Alternatively the optimizer should probably use `get_class` instead of `instanceof`.

Pull request description:
Constant expr as final
Closes #4119

Patch:
diff --git a/CHANGELOG b/CHANGELOG
index 40fc1b4a..24fc6e7d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,6 @@
 # 3.11.0 (2024-XX-XX)
 
+ * Mark `ConstantExpression` as being `@final`
  * Add the `find` filter
  * Fix optimizer mode validation in `OptimizerNodeVisitor`
  * Add the possibility to yield from a generator in `PrintNode`
diff --git a/doc/recipes.rst b/doc/recipes.rst
index b342457e..c091d34f 100644
--- a/doc/recipes.rst
+++ b/doc/recipes.rst
@@ -528,4 +528,15 @@ include in your templates:
             'tag_variable' => ['{[', ']}'],
         ]));
 
+Marking a Node as being safe
+----------------------------
+
+When using the escaper extension, you might want to mark some nodes as being
+safe to avoid any escaping. You can do so by wrapping your expression with a
+``RawFilter`` node::
+
+    use Twig\Node\Expression\Filter\RawFilter;
+
+    $safeExpr = new RawFilter(new YourSafeNode());
+
 .. _callback: https://www.php.net/manual/en/function.is-callable.php
diff --git a/extra/cache-extra/TokenParser/CacheTokenParser.php b/extra/cache-extra/TokenParser/CacheTokenParser.php
index 61d5d287..e6d6e1c8 100644
--- a/extra/cache-extra/TokenParser/CacheTokenParser.php
+++ b/extra/cache-extra/TokenParser/CacheTokenParser.php
@@ -13,7 +13,9 @@ namespace Twig\Extra\Cache\TokenParser;
 
 use Twig\Error\SyntaxError;
 use Twig\Extra\Cache\Node\CacheNode;
+use Twig\Node\Expression\ConstantExpression;
 use Twig\Node\Expression\Filter\RawFilter;
+use Twig\Node\Expression\FilterExpression;
 use Twig\Node\Node;
 use Twig\Node\PrintNode;
 use Twig\Token;
diff --git a/extra/string-extra/StringExtension.php b/extra/string-extra/StringExtension.php
index e0abb845..e541e7c1 100644
--- a/extra/string-extra/StringExtension.php
+++ b/extra/string-extra/StringExtension.php
@@ -84,7 +84,7 @@ final class StringExtension extends AbstractExtension
             case 'en':
                 return $this->englishInflector ?? $this->englishInflector = new EnglishInflector();
             default:
-                throw new \InvalidArgumentException(\sprintf('Locale "%s" is not supported.', $locale));
+                throw new \InvalidArgumentException(sprintf('Locale "%s" is not supported.', $locale));
         }
     }
 }
diff --git a/src/Extension/EscaperExtension.php b/src/Extension/EscaperExtension.php
index d8e9b6e4..f453ada9 100644
--- a/src/Extension/EscaperExtension.php
+++ b/src/Extension/EscaperExtension.php
@@ -118,7 +118,7 @@ final class EscaperExtension extends AbstractExtension
     /**
      * Defines a new escaper to be used via the escape filter.
      *
-     * @param string                                        $strategy The strategy name that should be used as a strategy in the escape call
+     * @param string                                $strategy The strategy name that should be used as a strategy in the escape call
      * @param callable(Environment, string, string): string $callable A valid PHP callable
      *
      * @deprecated since Twig 3.10
diff --git a/src/Node/Expression/ConstantExpression.php b/src/Node/Expression/ConstantExpression.php
index 7ddbcc6f..2a8909d5 100644
--- a/src/Node/Expression/ConstantExpression.php
+++ b/src/Node/Expression/ConstantExpression.php
@@ -14,6 +14,9 @@ namespace Twig\Node\Expression;
 
 use Twig\Compiler;
 
+/**
+ * @final
+ */
 class ConstantExpression extends AbstractExpression
 {
     public function __construct($value, int $lineno)
diff --git a/src/Node/Node.php b/src/Node/Node.php
index e0e473e8..17b60dc3 100644
--- a/src/Node/Node.php
+++ b/src/Node/Node.php
@@ -54,7 +54,7 @@ class Node implements \Countable, \IteratorAggregate
     {
         $attributes = [];
         foreach ($this->attributes as $name => $value) {
-            $attributes[] = \sprintf('%s: %s', $name, \is_callable($value) ? '\Closure' : str_replace("\n", '', var_export($value, true)));
+            $attributes[] = \sprintf('%s: %s', $name, is_callable($value) ? '\Closure' : str_replace("\n", '', var_export($value, true)));
         }
 
         $repr = [static::class.'('.implode(', ', $attributes)];
diff --git a/src/Runtime/EscaperRuntime.php b/src/Runtime/EscaperRuntime.php
index b1dac964..4df83430 100644
--- a/src/Runtime/EscaperRuntime.php
+++ b/src/Runtime/EscaperRuntime.php
@@ -36,7 +36,7 @@ final class EscaperRuntime implements RuntimeExtensionInterface
     /**
      * Defines a new escaper to be used via the escape filter.
      *
-     * @param string                                            $strategy The strategy name that should be used as a strategy in the escape call
+     * @param string                                    $strategy The strategy name that should be used as a strategy in the escape call
      * @param callable(string $string, string $charset): string $callable A valid PHP callable
      */
     public function setEscaper($strategy, callable $callable)


Test patch:
diff --git a/extra/markdown-extra/Tests/FunctionalTest.php b/extra/markdown-extra/Tests/FunctionalTest.php
index 0d9b73a5..42f4ff65 100644
--- a/extra/markdown-extra/Tests/FunctionalTest.php
+++ b/extra/markdown-extra/Tests/FunctionalTest.php
@@ -70,7 +70,7 @@ Hello
 Great!
 {% endapply %}
 EOF
-                , "<h1>Hello</h1>\n+<p>Great!</p>"],
+            , "<h1>Hello</h1>\n+<p>Great!</p>"],
             [<<<EOF
 {% apply markdown_to_html %}
     Hello
@@ -79,7 +79,7 @@ EOF
     Great!
 {% endapply %}
 EOF
-                , "<h1>Hello</h1>\n+<p>Great!</p>"],
+            , "<h1>Hello</h1>\n+<p>Great!</p>"],
             ["{{ include('html')|markdown_to_html }}", "<h1>Hello</h1>\n+<p>Great!</p>"],
         ];
     }
diff --git a/tests/Cache/ChainTest.php b/tests/Cache/ChainTest.php
index bb6ed7cf..f87e1629 100644
--- a/tests/Cache/ChainTest.php
+++ b/tests/Cache/ChainTest.php
@@ -121,13 +121,13 @@ class ChainTest extends TestCase
     public function testWrite()
     {
         $content = $this->generateSource();
-
+        
         $cacheA = new FilesystemCache($this->directory.'/A');
         $keyA = $cacheA->generateKey('_test_', $this->classname);
 
         $this->assertFileDoesNotExist($keyA);
         $this->assertFileDoesNotExist($this->directory.'/A');
-
+        
         $cacheB = new FilesystemCache($this->directory.'/B');
         $keyB = $cacheB->generateKey('_test_', $this->classname);
 
diff --git a/tests/Cache/ReadOnlyFilesystemTest.php b/tests/Cache/ReadOnlyFilesystemTest.php
index 424f6052..fe6dffcd 100644
--- a/tests/Cache/ReadOnlyFilesystemTest.php
+++ b/tests/Cache/ReadOnlyFilesystemTest.php
@@ -12,8 +12,8 @@ namespace Twig\Tests\Cache;
  */
 
 use PHPUnit\Framework\TestCase;
-use Twig\Cache\ReadOnlyFilesystemCache;
 use Twig\Tests\FilesystemHelper;
+use Twig\Cache\ReadOnlyFilesystemCache;
 
 class ReadOnlyFilesystemTest extends TestCase
 {
diff --git a/tests/ExpressionParserTest.php b/tests/ExpressionParserTest.php
index b1e3d501..8dd91ba6 100644
--- a/tests/ExpressionParserTest.php
+++ b/tests/ExpressionParserTest.php
@@ -93,73 +93,73 @@ class ExpressionParserTest extends TestCase
         return [
             // simple sequence
             ['{{ [1, 2] }}', new ArrayExpression([
-                new ConstantExpression(0, 1),
-                new ConstantExpression(1, 1),
+                    new ConstantExpression(0, 1),
+                    new ConstantExpression(1, 1),
 
-                new ConstantExpression(1, 1),
-                new ConstantExpression(2, 1),
-            ], 1),
+                    new ConstantExpression(1, 1),
+                    new ConstantExpression(2, 1),
+                ], 1),
             ],
 
             // sequence with trailing ,
             ['{{ [1, 2, ] }}', new ArrayExpression([
-                new ConstantExpression(0, 1),
-                new ConstantExpression(1, 1),
+                    new ConstantExpression(0, 1),
+                    new ConstantExpression(1, 1),
 
-                new ConstantExpression(1, 1),
-                new ConstantExpression(2, 1),
-            ], 1),
+                    new ConstantExpression(1, 1),
+                    new ConstantExpression(2, 1),
+                ], 1),
             ],
 
             // simple mapping
             ['{{ {"a": "b", "b": "c"} }}', new ArrayExpression([
-                new ConstantExpression('a', 1),
-                new ConstantExpression('b', 1),
+                    new ConstantExpression('a', 1),
+                    new ConstantExpression('b', 1),
 
-                new ConstantExpression('b', 1),
-                new ConstantExpression('c', 1),
-            ], 1),
+                    new ConstantExpression('b', 1),
+                    new ConstantExpression('c', 1),
+                ], 1),
             ],
 
             // mapping with trailing ,
             ['{{ {"a": "b", "b": "c", } }}', new ArrayExpression([
-                new ConstantExpression('a', 1),
-                new ConstantExpression('b', 1),
+                    new ConstantExpression('a', 1),
+                    new ConstantExpression('b', 1),
 
-                new ConstantExpression('b', 1),
-                new ConstantExpression('c', 1),
-            ], 1),
+                    new ConstantExpression('b', 1),
+                    new ConstantExpression('c', 1),
+                ], 1),
             ],
 
             // mapping in a sequence
             ['{{ [1, {"a": "b", "b": "c"}] }}', new ArrayExpression([
-                new ConstantExpression(0, 1),
-                new ConstantExpression(1, 1),
+                    new ConstantExpression(0, 1),
+                    new ConstantExpression(1, 1),
 
-                new ConstantExpression(1, 1),
-                new ArrayExpression([
-                    new ConstantExpression('a', 1),
-                    new ConstantExpression('b', 1),
+                    new ConstantExpression(1, 1),
+                    new ArrayExpression([
+                        new ConstantExpression('a', 1),
+                        new ConstantExpression('b', 1),
 
-                    new ConstantExpression('b', 1),
-                    new ConstantExpression('c', 1),
+                        new ConstantExpression('b', 1),
+                        new ConstantExpression('c', 1),
+                    ], 1),
                 ], 1),
-            ], 1),
             ],
 
             // sequence in a mapping
             ['{{ {"a": [1, 2], "b": "c"} }}', new ArrayExpression([
-                new ConstantExpression('a', 1),
-                new ArrayExpression([
-                    new ConstantExpression(0, 1),
-                    new ConstantExpression(1, 1),
+                    new ConstantExpression('a', 1),
+                    new ArrayExpression([
+                        new ConstantExpression(0, 1),
+                        new ConstantExpression(1, 1),
 
-                    new ConstantExpression(1, 1),
-                    new ConstantExpression(2, 1),
+                        new ConstantExpression(1, 1),
+                        new ConstantExpression(2, 1),
+                    ], 1),
+                    new ConstantExpression('b', 1),
+                    new ConstantExpression('c', 1),
                 ], 1),
-                new ConstantExpression('b', 1),
-                new ConstantExpression('c', 1),
-            ], 1),
             ],
             ['{{ {a, b} }}', new ArrayExpression([
                 new ConstantExpression('a', 1),
@@ -170,29 +170,29 @@ class ExpressionParserTest extends TestCase
 
             // sequence with spread operator
             ['{{ [1, 2, ...foo] }}',
-                new ArrayExpression([
-                    new ConstantExpression(0, 1),
-                    new ConstantExpression(1, 1),
+            new ArrayExpression([
+                new ConstantExpression(0, 1),
+                new ConstantExpression(1, 1),
 
-                    new ConstantExpression(1, 1),
-                    new ConstantExpression(2, 1),
+                new ConstantExpression(1, 1),
+                new ConstantExpression(2, 1),
 
-                    new ConstantExpression(2, 1),
-                    $this->createNameExpression('foo', ['spread' => true]),
-                ], 1)],
+                new ConstantExpression(2, 1),
+                $this->createNameExpression('foo', ['spread' => true]),
+            ], 1)],
 
             // mapping with spread operator
             ['{{ {"a": "b", "b": "c", ...otherLetters} }}',
-                new ArrayExpression([
-                    new ConstantExpression('a', 1),
-                    new ConstantExpression('b', 1),
+            new ArrayExpression([
+                new ConstantExpression('a', 1),
+                new ConstantExpression('b', 1),
 
-                    new ConstantExpression('b', 1),
-                    new ConstantExpression('c', 1),
+                new ConstantExpression('b', 1),
+                new ConstantExpression('c', 1),
 
-                    new ConstantExpression(0, 1),
-                    $this->createNameExpression('otherLetters', ['spread' => true]),
-                ], 1)],
+                new ConstantExpression(0, 1),
+                $this->createNameExpression('otherLetters', ['spread' => true]),
+            ], 1)],
         ];
     }
 
diff --git a/tests/Node/DeprecatedTest.php b/tests/Node/DeprecatedTest.php
index 90e958cb..63259eda 100644
--- a/tests/Node/DeprecatedTest.php
+++ b/tests/Node/DeprecatedTest.php
@@ -77,7 +77,7 @@ EOF
 \$$varName = foo();
 @trigger_error(\$$varName." (\"foo.twig\" at line 1).", E_USER_DEPRECATED);
 EOF
-            , $environment];
+        , $environment];
 
         return $tests;
     }
diff --git a/tests/Node/Expression/Filter/RawTest.php b/tests/Node/Expression/Filter/RawTest.php
index 72999fd4..89e495cc 100644
--- a/tests/Node/Expression/Filter/RawTest.php
+++ b/tests/Node/Expression/Filter/RawTest.php
@@ -24,7 +24,7 @@ class RawTest extends NodeTestCase
         $this->assertSame(12, $filter->getTemplateLine());
         $this->assertSame('raw', $filter->getNode('filter')->getAttribute('value'));
         $this->assertSame($node, $filter->getNode('node'));
-        $this->assertCount(0, $filter->getNode('arguments'));
+        $this->assertSame(0, count($filter->getNode('arguments')));
     }
 
     public function getTests()
diff --git a/tests/Node/Expression/FilterTest.php b/tests/Node/Expression/FilterTest.php
index 3e5feebe..9ce5e4e4 100644
--- a/tests/Node/Expression/FilterTest.php
+++ b/tests/Node/Expression/FilterTest.php
@@ -190,7 +190,6 @@ class FilterTest extends NodeTestCase
         if ($this->extension) {
             return $this->extension;
         }
-
         return $this->extension = new class() extends AbstractExtension {
             public function getFilters(): array
             {
diff --git a/tests/Node/IncludeTest.php b/tests/Node/IncludeTest.php
index 446fbd29..55454f8d 100644
--- a/tests/Node/IncludeTest.php
+++ b/tests/Node/IncludeTest.php
@@ -88,7 +88,7 @@ if (\$__internal_%s) {
     yield from \$__internal_%s->unwrap()->yield(CoreExtension::toArray(["foo" => true]));
 }
 EOF
-            , null, true];
+        , null, true];
 
         return $tests;
     }
diff --git a/tests/Node/ModuleTest.php b/tests/Node/ModuleTest.php
index 1306401d..974bc22c 100644
--- a/tests/Node/ModuleTest.php
+++ b/tests/Node/ModuleTest.php
@@ -121,7 +121,7 @@ class __TwigTemplate_%x extends Template
     }
 }
 EOF
-            , $twig, true];
+        , $twig, true];
 
         $import = new ImportNode(new ConstantExpression('foo.twig', 1), new AssignNameExpression('macro', 1), 2);
 
@@ -207,7 +207,7 @@ class __TwigTemplate_%x extends Template
     }
 }
 EOF
-            , $twig, true];
+        , $twig, true];
 
         $set = new SetNode(false, new Node([new AssignNameExpression('foo', 4)]), new Node([new ConstantExpression('foo', 4)]), 4);
         $body = new Node([$set]);
@@ -297,7 +297,7 @@ class __TwigTemplate_%x extends Template
     }
 }
 EOF
-            , $twig, true];
+        , $twig, true];
 
         return $tests;
     }
diff --git a/tests/TemplateTest.php b/tests/TemplateTest.php
index 37d6fe62..8ee46c19 100644
--- a/tests/TemplateTest.php
+++ b/tests/TemplateTest.php
@@ -17,6 +17,7 @@ use Twig\Error\RuntimeError;
 use Twig\Extension\CoreExtension;
 use Twig\Extension\SandboxExtension;
 use Twig\Loader\ArrayLoader;
+use Twig\Loader\LoaderInterface;
 use Twig\Sandbox\SecurityError;
 use Twig\Sandbox\SecurityPolicy;
 use Twig\Source;