Repository: twigphp/Twig (php)

Issue:
Possible BC break with `EscaperExtension` in `3.10.x`
In `3.9.x` the escaper callback was called with 3 arguments:

* The environment `$env`
* The string `$string`
* The character set `$charset`

https://github.com/twigphp/Twig/blob/a842d75fed59cdbcbd3a3ad7fb9eb768fc350d58/src/Extension/EscaperExtension.php#L408

However, in `3.10.1` (and probably `3.10.0`) it is only called with 2 arguments:

* The environment `$env`
* The string `$string`

https://github.com/twigphp/Twig/blob/3af5ab2e52279e5e23dc192b1a26db3b8cffa4e7/src/Extension/EscaperExtension.php#L135

Resulting in an error like this for example:

```
ArgumentCountError:
Too few arguments to function Contao\CoreBundle\Twig\Interop\ContaoEscaper::escapeHtml(), 2 passed in vendor\twig\twig\src\Extension\EscaperExtension.php on line 135 and exactly 3 expected

  at vendor\contao\contao\core-bundle\src\Twig\Interop\ContaoEscaper.php:38
  at Contao\CoreBundle\Twig\Interop\ContaoEscaper->escapeHtml(object(Environment), 'Main navigation')
     (vendor\twig\twig\src\Extension\EscaperExtension.php:135)
  at Twig\Extension\EscaperExtension->Twig\Extension\{closure}('Main navigation', 'UTF-8')
     (vendor\twig\twig\src\Runtime\EscaperRuntime.php:311)
```

for custom escapers when updating from `3.9` to `3.10`.

Pull request:
Fix old escaper signature + add docs
Closes #4087 
Closes #4088


Patch:
diff --git a/CHANGELOG b/CHANGELOG
index eed466a2..80234c86 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,6 @@
 # 3.10.2 (2024-XX-XX)
 
- * n/a
+ * Fix support for the deprecated escaper signature
 
 # 3.10.1 (2024-05-12)
 
diff --git a/doc/filters/escape.rst b/doc/filters/escape.rst
index e8b735db..70d18eb5 100644
--- a/doc/filters/escape.rst
+++ b/doc/filters/escape.rst
@@ -93,15 +93,34 @@ to learn more about this topic.
 Custom Escapers
 ---------------
 
+.. versionadded:: 3.10
+
+    The ``EscaperRuntime`` class has been added in 3.10. On previous versions,
+    you can define custom escapers by calling the ``setEscaper()`` method on
+    the escaper extension instance. The first argument is the escaper strategy
+    (to be used in the ``escape`` call) and the second one must be a valid PHP
+    callable::
+
+        use Twig\Extension\EscaperExtension;
+
+        $twig = new \Twig\Environment($loader);
+        $twig->getExtension(EscaperExtension::class)->setEscaper('csv', 'csv_escaper');
+
+    When called by Twig, the callable receives the Twig environment instance,
+    the string to escape, and the charset.
+
 You can define custom escapers by calling the ``setEscaper()`` method on the
-escaper extension instance. The first argument is the escaper name (to be
-used in the ``escape`` call) and the second one must be a valid PHP callable::
+escaper runtime instance. It accepts two arguments: the strategy name and a PHP
+callable that accepts a string to escape and the charset::
+
+    use Twig\Runtime\EscaperRuntime;
 
     $twig = new \Twig\Environment($loader);
-    $twig->getExtension(\Twig\Extension\EscaperExtension::class)->setEscaper('csv', 'csv_escaper');
+    $escaper = fn ($string, $charset) => $string;
+    $twig->getRuntime(EscaperRuntime::class)->setEscaper('identity', $escaper);
 
-When called by Twig, the callable receives the Twig environment instance, the
-string to escape, and the charset.
+    # Usage in a template:
+    # {{ 'foo'|escape('identity') }}
 
 .. note::
 
diff --git a/doc/internals.rst b/doc/internals.rst
index ccbb202f..2aeb12f3 100644
--- a/doc/internals.rst
+++ b/doc/internals.rst
@@ -124,9 +124,12 @@ using)::
     {
         protected function doDisplay(array $context, array $blocks = [])
         {
+            $macros = $this->macros;
             // line 1
-            echo "Hello ";
-            echo twig_escape_filter($this->env, (isset($context["name"]) ? $context["name"] : null), "html", null, true);
+            yield "Hello ";
+            // line 2
+            yield $this->env->getRuntime('Twig\Runtime\EscaperRuntime')->escape((isset($context["name"]) || array_key_exists("name", $context) ? $context["name"] : (function () { throw new RuntimeError('Variable "name" does not exist.', 2, $this->source); })()), "html", null, true);
+            return; yield '';
         }
 
         // some more code
diff --git a/src/Extension/EscaperExtension.php b/src/Extension/EscaperExtension.php
index cf821c6d..3360ef67 100644
--- a/src/Extension/EscaperExtension.php
+++ b/src/Extension/EscaperExtension.php
@@ -117,8 +117,8 @@ 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 callable(Environment, string) $callable A valid PHP callable
+     * @param string                                $strategy The strategy name that should be used as a strategy in the escape call
+     * @param callable(Environment, string, string) $callable A valid PHP callable
      *
      * @deprecated since Twig 3.10
      */
@@ -132,7 +132,7 @@ final class EscaperExtension extends AbstractExtension
 
         $this->escapers[$strategy] = $callable;
         $callable = function ($string, $charset) use ($callable) {
-            return $callable($this->environment, $string);
+            return $callable($this->environment, $string, $charset);
         };
 
         $this->escaper->setEscaper($strategy, $callable);
@@ -141,7 +141,7 @@ final class EscaperExtension extends AbstractExtension
     /**
      * Gets all defined escapers.
      *
-     * @return array<callable(Environment, string)> An array of escapers
+     * @return array<callable(Environment, string, string)> An array of escapers
      *
      * @deprecated since Twig 3.10
      */


Test patch:
diff --git a/tests/Extension/EscaperTest.php b/tests/Extension/EscaperTest.php
index 950bd1b4..e211677b 100644
--- a/tests/Extension/EscaperTest.php
+++ b/tests/Extension/EscaperTest.php
@@ -29,15 +29,15 @@ class EscaperTest extends TestCase
         $twig = new Environment($this->createMock(LoaderInterface::class));
         $escaperExt = $twig->getExtension(EscaperExtension::class);
         $escaperExt->setEscaper('foo', 'Twig\Tests\legacy_escaper');
-        $this->assertSame($expected, $twig->getRuntime(EscaperRuntime::class)->escape($string, $strategy));
+        $this->assertSame($expected, $twig->getRuntime(EscaperRuntime::class)->escape($string, $strategy, 'ISO-8859-1'));
     }
 
     public function provideCustomEscaperCases()
     {
         return [
-            ['fooUTF-8', 'foo', 'foo'],
-            ['UTF-8', null, 'foo'],
-            ['42UTF-8', 42, 'foo'],
+            ['foo**ISO-8859-1**UTF-8', 'foo', 'foo'],
+            ['**ISO-8859-1**UTF-8', null, 'foo'],
+            ['42**ISO-8859-1**UTF-8', 42, 'foo'],
         ];
     }
 
@@ -51,7 +51,7 @@ class EscaperTest extends TestCase
         $twig = new Environment($this->createMock(LoaderInterface::class));
         $escaperExt = $twig->getExtension(EscaperExtension::class);
         $escaperExt->setEscaper('foo', 'Twig\Tests\legacy_escaper');
-        $this->assertSame($expected, $twig->getRuntime(EscaperRuntime::class)->escape($string, $strategy));
+        $this->assertSame($expected, $twig->getRuntime(EscaperRuntime::class)->escape($string, $strategy, 'ISO-8859-1'));
     }
 
     /**
@@ -67,17 +67,17 @@ class EscaperTest extends TestCase
         $escaperExt2 = $env2->getExtension(EscaperExtension::class);
         $escaperExt2->setEscaper('foo', 'Twig\Tests\legacy_escaper_again');
 
-        $this->assertSame('fooUTF-8', $env1->getRuntime(EscaperRuntime::class)->escape('foo', 'foo'));
-        $this->assertSame('fooUTF-81', $env2->getRuntime(EscaperRuntime::class)->escape('foo', 'foo'));
+        $this->assertSame('foo**ISO-8859-1**UTF-8', $env1->getRuntime(EscaperRuntime::class)->escape('foo', 'foo', 'ISO-8859-1'));
+        $this->assertSame('foo**ISO-8859-1**UTF-8**again', $env2->getRuntime(EscaperRuntime::class)->escape('foo', 'foo', 'ISO-8859-1'));
     }
 }
 
-function legacy_escaper(Environment $twig, $string)
+function legacy_escaper(Environment $twig, $string, $charset)
 {
-    return $string.$twig->getCharset();
+    return $string.'**'.$charset.'**'.$twig->getCharset();
 }
 
-function legacy_escaper_again(Environment $twig, $string)
+function legacy_escaper_again(Environment $twig, $string, $charset)
 {
-    return $string.$twig->getCharset().'1';
+    return $string.'**'.$charset.'**'.$twig->getCharset().'**again';
 }
diff --git a/tests/Runtime/EscaperRuntimeTest.php b/tests/Runtime/EscaperRuntimeTest.php
index 04247340..11764f43 100644
--- a/tests/Runtime/EscaperRuntimeTest.php
+++ b/tests/Runtime/EscaperRuntimeTest.php
@@ -350,19 +350,19 @@ class EscaperRuntimeTest extends TestCase
     /**
      * @dataProvider provideCustomEscaperCases
      */
-    public function testCustomEscaper($expected, $string, $strategy)
+    public function testCustomEscaper($expected, $string, $strategy, $charset)
     {
         $escaper = new EscaperRuntime();
         $escaper->setEscaper('foo', 'Twig\Tests\escaper');
-        $this->assertSame($expected, $escaper->escape($string, $strategy));
+        $this->assertSame($expected, $escaper->escape($string, $strategy, $charset));
     }
 
     public function provideCustomEscaperCases()
     {
         return [
-            ['fooUTF-8', 'foo', 'foo'],
-            ['UTF-8', null, 'foo'],
-            ['42UTF-8', 42, 'foo'],
+            ['foo**ISO-8859-1', 'foo', 'foo', 'ISO-8859-1'],
+            ['**ISO-8859-1', null, 'foo', 'ISO-8859-1'],
+            ['42**UTF-8', 42, 'foo', null],
         ];
     }
 
@@ -391,7 +391,7 @@ class EscaperRuntimeTest extends TestCase
 
 function escaper($string, $charset)
 {
-    return $string.$charset;
+    return $string.'**'.$charset;
 }
 
 interface Extension_SafeHtmlInterface