feature #4469 Fix SafeAnalysisNodeVisitor::getSafe() return value (fa…

…bpot)

This PR was merged into the 3.x branch.

Discussion
----------

Fix SafeAnalysisNodeVisitor::getSafe() return value

`SafeAnalysisNodeVisitor::getSafe()` can return `[]` or `null`. Both mean the same thing, but `null` is partially supported as `EscaperNodeVisitor` does not support `null` everywhere (as the node visitor never set `safe` to `null`). But a third party might return `null`. This PR makes the code more robust by deprecating using `null`.

Commits
-------

c402deb Fix EscapeNodeVisitor::isSafeFor()

CHANGELOG

@@ -1,6 +1,7 @@
 # 3.16.0 (2024-XX-XX)
 
  * Deprecate not passing a `Source` instance to `TokenStream`
+ * Deprecate returning `null` from `TwigFilter::getSafe()` and `TwigFunction::getSafe()`, return `[]` instead
 
 # 3.15.0 (2024-11-17)
 

doc/deprecated.rst

@@ -319,6 +319,10 @@ Functions/Filters/Tests
   arrow functions is deprecated as of Twig 3.15; these arguments will have a
   ``\Closure`` type hint in 4.0.
 
+* Returning ``null`` from ``TwigFilter::getSafe()`` and
+  ``TwigFunction::getSafe()`` is deprecated as of Twig 3.16; return ``[]``
+  instead.
+
 Node
 ----
 

src/NodeVisitor/EscaperNodeVisitor.php

@@ -172,7 +172,7 @@ private function isSafeFor(string $type, AbstractExpression $expression, Environ
     {
         $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

@@ -37,11 +37,14 @@ public function setSafeVars(array $safeVars): void
         $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) {
@@ -55,6 +58,8 @@ public function getSafe(Node $node)
 
             return $bucket['value'];
         }
+
+        return [];
     }
 
     private function setSafe(Node $node, array $safe): void
@@ -107,11 +112,14 @@ public function leaveNode(Node $node, Environment $env): ?Node
             if ($filter) {
                 $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
@@ -123,9 +131,12 @@ public function leaveNode(Node $node, Environment $env): ?Node
             }
 
             if ($function) {
-                $this->setSafe($node, $function->getSafe($node->getNode('arguments')));
-            } else {
-                $this->setSafe($node, []);
+                $safe = $function->getSafe($node->getNode('arguments'));
+                if (null === $safe) {
+                    trigger_deprecation('twig/twig', '3.16', 'The "%s::getSafe()" method should not return "null" anymore, return "[]" instead.', $function::class);
+                    $safe = [];
+                }
+                $this->setSafe($node, $safe);
             }
         } elseif ($node instanceof MethodCallExpression || $node instanceof MacroReferenceExpression) {
             // all macro calls are safe
@@ -134,19 +145,15 @@ public function leaveNode(Node $node, Environment $env): ?Node
             $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 @@ public function getSafe(Node $filterArgs): ?array
             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