From d013a84f907d839bdec464437a81b1189dbc6ea6 Mon Sep 17 00:00:00 2001 From: Kacper Donat Date: Wed, 28 Oct 2026 09:56:11 +0100 Subject: [PATCH] avoid by-reference warnings in filter --- src/Internal/functions.php | 26 +++++++++++++++++++++++--- tests/Collections/TransformsTest.php | 23 +++++++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/Internal/functions.php b/src/Internal/functions.php index 04e2f8f..b03d1b4 100644 --- a/src/Internal/functions.php +++ b/src/Internal/functions.php @@ -64,13 +64,17 @@ function callback(callable $callable, int $arity = 1): \Closure $leadingOnly = $target->isInternal() || (!$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) { - return $compiled; + return $byReference ? static fn (...$args) => $compiled(...$args) : $compiled; } // A userland closure taking no more than the leading arguments already ignores the rest. - $reflection = new \ReflectionFunction($compiled); - if (!$reflection->isInternal() && !$reflection->isVariadic() && $reflection->getNumberOfParameters() <= $arity) { + if (!$byReference && !$reflection->isInternal() && !$reflection->isVariadic() && $reflection->getNumberOfParameters() <= $arity) { return $compiled; } @@ -79,6 +83,22 @@ function callback(callable $callable, int $arity = 1): \Closure : static fn ($carry, $value) => $compiled($carry, $value); } +/** + * Whether a function takes any of its parameters by reference. + * + * @internal + */ +function takesByReference(\ReflectionFunction $reflection): bool +{ + foreach ($reflection->getParameters() as $parameter) { + if ($parameter->isPassedByReference()) { + return true; + } + } + + return false; +} + /** * Finds the function a callable ultimately calls: decorators are unwrapped, also when they were turned into * closures, and a pipeline gives its first function, which gets all of its arguments. diff --git a/tests/Collections/TransformsTest.php b/tests/Collections/TransformsTest.php index 64e56c3..2ddf18a 100644 --- a/tests/Collections/TransformsTest.php +++ b/tests/Collections/TransformsTest.php @@ -107,6 +107,29 @@ class TransformsTest extends \PHPUnit\Framework\TestCase $this->assertSeq([2], c\filter(fn ($v) => $v % 2 === 0)(self::gen([1, 2, 3]))); } + public function testFilterWithByReferenceCallbackDoesNotWarn() + { + $warnings = []; + set_error_handler(function (int $level, string $message) use (&$warnings) { + $warnings[] = $message; + + return true; + }); + + try { + $array = c\filter(fn (&$v) => $v > 1)([1, 2, 3]); + $seq = c\filter(fn (&$v) => $v > 1)(new Seq([1, 2, 3])); + $both = c\filter(fn (&$v, $k) => $k > 0)([1, 2, 3]); + } finally { + restore_error_handler(); + } + + $this->assertSame([], $warnings); + $this->assertSame([1 => 2, 2 => 3], $array); + $this->assertSeq([2, 3], $seq); + $this->assertSame([1 => 2, 2 => 3], $both); + } + public function testFilterWithInternalPredicate() { $this->assertSame([1 => 'a'], c\filter(is_string(...))([1, 'a']));