Skip to content

Commit

Permalink
feature #4377 Deprecate passing a string or an array to Twig callable…
Browse files Browse the repository at this point in the history
… arguments accepting arrow functions (pass a Closure) (fabpot)

This PR was merged into the 3.x branch.

Discussion
----------

Deprecate passing a string or an array to Twig callable arguments accepting arrow functions (pass a Closure)

Instead of restricting callable arguments only in sandbox mode, let's deprecate using functions/arrays as callables.

Commits
-------

5e94483 Deprecate passing a string or an array to Twig callable arguments accepting arrow functions (pass a Closure)
  • Loading branch information
fabpot committed Oct 4, 2024
2 parents f363ab2 + 5e94483 commit 1876635
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# 3.15.0 (2024-XX-XX)

* Deprecate passing a string or an array to Twig callable arguments accepting arrow functions (pass a `\Closure`)
* Add support for triggering deprecations for future operator precedence changes
* Deprecate using the `not` unary operator in an expression with ``*``, ``/``, ``//``, or ``%`` without using explicit parentheses to clarify precedence
* Deprecate using the `??` binary operator without explicit parentheses
Expand Down
4 changes: 4 additions & 0 deletions doc/deprecated.rst
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,10 @@ Functions/Filters/Tests
* For variadic arguments, use snake-case for the argument name to ease the
transition to 4.0.

* Passing a ``string`` or an ``array`` to Twig callable arguments accepting
arrow functions is deprecated as of Twig 3.15; these arguments will have a
``\Closure`` type hint in 4.0.

Node
----

Expand Down
37 changes: 28 additions & 9 deletions src/Extension/CoreExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,7 @@ public static function shuffle(string $charset, $item)
* Sorts an array.
*
* @param array|\Traversable $array
* @param ?\Closure $arrow
*
* @internal
*/
Expand All @@ -984,7 +985,7 @@ public static function sort(Environment $env, $array, $arrow = null): array
}

if (null !== $arrow) {
self::checkArrowInSandbox($env, $arrow, 'sort', 'filter');
self::checkArrow($env, $arrow, 'sort', 'filter');

uasort($array, $arrow);
} else {
Expand Down Expand Up @@ -1838,6 +1839,8 @@ public static function column($array, $name, $index = null): array
}

/**
* @param \Closure $arrow
*
* @internal
*/
public static function filter(Environment $env, $array, $arrow)
Expand All @@ -1846,7 +1849,7 @@ public static function filter(Environment $env, $array, $arrow)
throw new RuntimeError(\sprintf('The "filter" filter expects a sequence/mapping or "Traversable", got "%s".', get_debug_type($array)));
}

self::checkArrowInSandbox($env, $arrow, 'filter', 'filter');
self::checkArrow($env, $arrow, 'filter', 'filter');

if (\is_array($array)) {
return array_filter($array, $arrow, \ARRAY_FILTER_USE_BOTH);
Expand All @@ -1857,6 +1860,8 @@ public static function filter(Environment $env, $array, $arrow)
}

/**
* @param \Closure $arrow
*
* @internal
*/
public static function find(Environment $env, $array, $arrow)
Expand All @@ -1865,7 +1870,7 @@ public static function find(Environment $env, $array, $arrow)
throw new RuntimeError(\sprintf('The "find" filter expects a sequence or a mapping, got "%s".', get_debug_type($array)));
}

self::checkArrowInSandbox($env, $arrow, 'find', 'filter');
self::checkArrow($env, $arrow, 'find', 'filter');

foreach ($array as $k => $v) {
if ($arrow($v, $k)) {
Expand All @@ -1877,6 +1882,8 @@ public static function find(Environment $env, $array, $arrow)
}

/**
* @param \Closure $arrow
*
* @internal
*/
public static function map(Environment $env, $array, $arrow)
Expand All @@ -1885,7 +1892,7 @@ public static function map(Environment $env, $array, $arrow)
throw new RuntimeError(\sprintf('The "map" filter expects a sequence or a mapping, got "%s".', get_debug_type($array)));
}

self::checkArrowInSandbox($env, $arrow, 'map', 'filter');
self::checkArrow($env, $arrow, 'map', 'filter');

$r = [];
foreach ($array as $k => $v) {
Expand All @@ -1896,6 +1903,8 @@ public static function map(Environment $env, $array, $arrow)
}

/**
* @param \Closure $arrow
*
* @internal
*/
public static function reduce(Environment $env, $array, $arrow, $initial = null)
Expand All @@ -1904,7 +1913,7 @@ public static function reduce(Environment $env, $array, $arrow, $initial = null)
throw new RuntimeError(\sprintf('The "reduce" filter expects a sequence or a mapping, got "%s".', get_debug_type($array)));
}

self::checkArrowInSandbox($env, $arrow, 'reduce', 'filter');
self::checkArrow($env, $arrow, 'reduce', 'filter');

$accumulator = $initial;
foreach ($array as $key => $value) {
Expand All @@ -1915,6 +1924,8 @@ public static function reduce(Environment $env, $array, $arrow, $initial = null)
}

/**
* @param \Closure $arrow
*
* @internal
*/
public static function arraySome(Environment $env, $array, $arrow)
Expand All @@ -1923,7 +1934,7 @@ public static function arraySome(Environment $env, $array, $arrow)
throw new RuntimeError(\sprintf('The "has some" test expects a sequence or a mapping, got "%s".', get_debug_type($array)));
}

self::checkArrowInSandbox($env, $arrow, 'has some', 'operator');
self::checkArrow($env, $arrow, 'has some', 'operator');

foreach ($array as $k => $v) {
if ($arrow($v, $k)) {
Expand All @@ -1935,6 +1946,8 @@ public static function arraySome(Environment $env, $array, $arrow)
}

/**
* @param \Closure $arrow
*
* @internal
*/
public static function arrayEvery(Environment $env, $array, $arrow)
Expand All @@ -1943,7 +1956,7 @@ public static function arrayEvery(Environment $env, $array, $arrow)
throw new RuntimeError(\sprintf('The "has every" test expects a sequence or a mapping, got "%s".', get_debug_type($array)));
}

self::checkArrowInSandbox($env, $arrow, 'has every', 'operator');
self::checkArrow($env, $arrow, 'has every', 'operator');

foreach ($array as $k => $v) {
if (!$arrow($v, $k)) {
Expand All @@ -1957,11 +1970,17 @@ public static function arrayEvery(Environment $env, $array, $arrow)
/**
* @internal
*/
public static function checkArrowInSandbox(Environment $env, $arrow, $thing, $type)
public static function checkArrow(Environment $env, $arrow, $thing, $type)
{
if (!$arrow instanceof \Closure && $env->hasExtension(SandboxExtension::class) && $env->getExtension(SandboxExtension::class)->isSandboxed()) {
if ($arrow instanceof \Closure) {
return;
}

if ($env->hasExtension(SandboxExtension::class) && $env->getExtension(SandboxExtension::class)->isSandboxed()) {
throw new RuntimeError(\sprintf('The callable passed to the "%s" %s must be a Closure in sandbox mode.', $thing, $type));
}

trigger_deprecation('twig/twig', '3.15', 'Passing a callable that is not a PHP \Closure as an argument to the "%s" %s is deprecated.', $thing, $type);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Resources/core.php
Original file line number Diff line number Diff line change
Expand Up @@ -537,5 +537,5 @@ function twig_check_arrow_in_sandbox(Environment $env, $arrow, $thing, $type)
{
trigger_deprecation('twig/twig', '3.9', 'Using the internal "%s" function is deprecated.', __FUNCTION__);

return CoreExtension::checkArrowInSandbox($env, $arrow, $thing, $type);
CoreExtension::checkArrow($env, $arrow, $thing, $type);
}

0 comments on commit 1876635

Please sign in to comment.