pass by-value only to native filter paths

This commit is contained in:
Kacper Donat 2026-10-28 10:05:33 +01:00
parent d013a84f90
commit b2550cf80b
3 changed files with 28 additions and 11 deletions

View File

@ -22,6 +22,7 @@ namespace Kadet\Functional\Collections;
use Kadet\Functional\Collections\Lazy as lazy; use Kadet\Functional\Collections\Lazy as lazy;
use function Kadet\Functional\Internal\callback; use function Kadet\Functional\Internal\callback;
use function Kadet\Functional\Internal\takesByReference;
use function Kadet\Functional\Collections\Internal\entries; use function Kadet\Functional\Collections\Internal\entries;
use function Kadet\Functional\Collections\Internal\iterated; use function Kadet\Functional\Collections\Internal\iterated;
use function Kadet\Functional\Collections\Internal\keyedLike; use function Kadet\Functional\Collections\Internal\keyedLike;
@ -101,14 +102,18 @@ function filter(?callable $fn = null): \Closure
{ {
$fn = $fn === null ? static fn ($value) => $value : callback($fn); $fn = $fn === null ? static fn ($value) => $value : callback($fn);
return static function (iterable $input) use ($fn): iterable { // array_filter() passes values, not variables, so a callback taking a parameter by reference would warn on each
// call. Through this wrapper it gets variables; as before, its changes don't reach the result.
$native = takesByReference($fn) ? static fn ($value, $key) => $fn($value, $key) : $fn;
return static function (iterable $input) use ($fn, $native): iterable {
if ($input instanceof \Ds\Map) { if ($input instanceof \Ds\Map) {
return $input->filter(static fn ($key, $value) => $fn($value, $key)); return $input->filter(static fn ($key, $value) => $fn($value, $key));
} }
$values = iterated($input); $values = iterated($input);
if (is_array($values)) { if (is_array($values)) {
$values = array_filter($values, $fn, ARRAY_FILTER_USE_BOTH); $values = array_filter($values, $native, ARRAY_FILTER_USE_BOTH);
return is_array($input) ? $values : listLike($input, $values); return is_array($input) ? $values : listLike($input, $values);
} }

View File

@ -64,17 +64,13 @@ function callback(callable $callable, int $arity = 1): \Closure
$leadingOnly = $target->isInternal() $leadingOnly = $target->isInternal()
|| (!$target->isVariadic() && $target->getNumberOfParameters() === $arity); || (!$target->isVariadic() && $target->getNumberOfParameters() === $arity);
// Internal functions like array_filter() pass values, not variables, so a closure taking a parameter by reference
// would warn on each call. Called from a wrapper, it gets the wrapper's variables instead.
$reflection = new \ReflectionFunction($compiled);
$byReference = takesByReference($reflection);
if (!$leadingOnly) { if (!$leadingOnly) {
return $byReference ? static fn (...$args) => $compiled(...$args) : $compiled; return $compiled;
} }
// A userland closure taking no more than the leading arguments already ignores the rest. // A userland closure taking no more than the leading arguments already ignores the rest.
if (!$byReference && !$reflection->isInternal() && !$reflection->isVariadic() && $reflection->getNumberOfParameters() <= $arity) { $reflection = new \ReflectionFunction($compiled);
if (!$reflection->isInternal() && !$reflection->isVariadic() && $reflection->getNumberOfParameters() <= $arity) {
return $compiled; return $compiled;
} }
@ -88,9 +84,9 @@ function callback(callable $callable, int $arity = 1): \Closure
* *
* @internal * @internal
*/ */
function takesByReference(\ReflectionFunction $reflection): bool function takesByReference(\Closure $closure): bool
{ {
foreach ($reflection->getParameters() as $parameter) { foreach ((new \ReflectionFunction($closure))->getParameters() as $parameter) {
if ($parameter->isPassedByReference()) { if ($parameter->isPassedByReference()) {
return true; return true;
} }

View File

@ -130,6 +130,22 @@ class TransformsTest extends \PHPUnit\Framework\TestCase
$this->assertSame([1 => 2, 2 => 3], $both); $this->assertSame([1 => 2, 2 => 3], $both);
} }
public function testByReferenceCallbacksWriteBackOutsideNativeFilter()
{
// Where the callback is called with a variable, its changes to the value are kept, as in 1.0.0.
$trim = function (&$v) {
$v = trim($v);
return $v !== '';
};
$gen = fn () => self::gen([' a ', ' ', 'b ']);
$this->assertSame(['a', 'b'], iterator_to_array(\Kadet\Functional\Collections\Lazy\filter($trim)($gen()), false));
$this->assertSame([1 => ''], c\reject($trim)([' a ', ' ', 'b ']));
$this->assertSame('x', c\find($trim)(new Seq([' x '])));
$this->assertSame('a', c\first($trim)($gen()));
}
public function testFilterWithInternalPredicate() public function testFilterWithInternalPredicate()
{ {
$this->assertSame([1 => 'a'], c\filter(is_string(...))([1, 'a'])); $this->assertSame([1 => 'a'], c\filter(is_string(...))([1, 'a']));