From b2550cf80b04db485c7fb706ac8ee2080d1c60fa Mon Sep 17 00:00:00 2001
From: Kacper Donat <kadet1090@gmail.com>
Date: Wed, 28 Oct 2026 10:05:33 +0100
Subject: [PATCH] pass by-value only to native filter paths

---
 src/Collections/transforms.php       |  9 +++++++--
 src/Internal/functions.php           | 14 +++++---------
 tests/Collections/TransformsTest.php | 16 ++++++++++++++++
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/src/Collections/transforms.php b/src/Collections/transforms.php
index d536e99..253baa9 100644
--- a/src/Collections/transforms.php
+++ b/src/Collections/transforms.php
@@ -22,6 +22,7 @@ namespace Kadet\Functional\Collections;
 
 use Kadet\Functional\Collections\Lazy as lazy;
 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\iterated;
 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);
 
-    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) {
             return $input->filter(static fn ($key, $value) => $fn($value, $key));
         }
 
         $values = iterated($input);
         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);
         }
diff --git a/src/Internal/functions.php b/src/Internal/functions.php
index b03d1b4..3e86d70 100644
--- a/src/Internal/functions.php
+++ b/src/Internal/functions.php
@@ -64,17 +64,13 @@ 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 $byReference ? static fn (...$args) => $compiled(...$args) : $compiled;
+        return $compiled;
     }
 
     // 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;
     }
 
@@ -88,9 +84,9 @@ function callback(callable $callable, int $arity = 1): \Closure
  *
  * @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()) {
             return true;
         }
diff --git a/tests/Collections/TransformsTest.php b/tests/Collections/TransformsTest.php
index 2ddf18a..dccff5e 100644
--- a/tests/Collections/TransformsTest.php
+++ b/tests/Collections/TransformsTest.php
@@ -130,6 +130,22 @@ class TransformsTest extends \PHPUnit\Framework\TestCase
         $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()
     {
         $this->assertSame([1 => 'a'], c\filter(is_string(...))([1, 'a']));