chore: Refactor optimization for internal callables (#289)

Author: sanmai

src/Standard.php

@@ -1563,13 +1563,7 @@ public function tap(callable $func): self
     private static function tapValues(iterable $previous, callable $func): Generator
     {
         foreach ($previous as $key => $value) {
-            try {
-                $func($value, $key);
-            } catch (ArgumentCountError) {
-                // Optimization to reduce the number of argument count errors when calling internal callables.
-                $func = self::wrapInternalCallable($func);
-                $func($value);
-            }
+            self::callWithValueKey($func, $value, $key);
 
             yield $key => $value;
         }
@@ -1602,16 +1596,24 @@ private function eachInternal(callable $func): void
         }
 
         foreach ($this->pipeline as $key => $value) {
-            try {
-                $func($value, $key);
-            } catch (ArgumentCountError) {
-                // Optimization to reduce the number of argument count errors when calling internal callables.
-                // This error is thrown when too many arguments are passed to a built-in function (that are sensitive
-                // to extra arguments), so we can wrap it to prevent the errors later. On the other hand, if there
-                // are too little arguments passed, it will blow up just a line later.
-                $func = self::wrapInternalCallable($func);
-                $func($value);
-            }
+            self::callWithValueKey($func, $value, $key);
+        }
+    }
+
+    /**
+     * Reference parameter allows wrapped callable to persist across iterations.
+     */
+    private static function callWithValueKey(callable &$func, mixed $value, mixed $key): void
+    {
+        try {
+            $func($value, $key);
+        } catch (ArgumentCountError) {
+            // Optimization to reduce the number of argument count errors when calling internal callables.
+            // This error is thrown when too many arguments are passed to a built-in function (that are sensitive
+            // to extra arguments), so we can wrap it to prevent the errors later. On the other hand, if there
+            // are too little arguments passed, it will blow up just a line later.
+            $func = self::wrapInternalCallable($func);
+            $func($value);
         }
     }
 

tests/EachTest.php

@@ -61,7 +61,7 @@ public function testUninitialized(): void
     {
         $pipeline = new Standard();
 
-        $pipeline->each(fn($value) => $this->observeValue($value));
+        $pipeline->each($this->observeValue(...));
 
         $this->assertSame([], $this->output);
     }
@@ -70,7 +70,7 @@ public function testEmpty(): void
     {
         $pipeline = take([]);
 
-        $pipeline->each(fn($value) => $this->observeValue($value));
+        $pipeline->each($this->observeValue(...));
 
         $this->assertSame([], $this->output);
     }
@@ -79,7 +79,7 @@ public function testEmptyGenerator(): void
     {
         $pipeline = map(static fn() => yield from []);
 
-        $pipeline->each(fn($value) => $this->observeValue($value));
+        $pipeline->each($this->observeValue(...));
 
         $this->assertSame([], $this->output);
     }
@@ -88,7 +88,7 @@ public function testNotEmpty(): void
     {
         $pipeline = take([1, 2, 3, 4]);
 
-        $pipeline->each(fn(int $value) => $this->observeValue($value));
+        $pipeline->each($this->observeValue(...));
 
         $this->assertSame([1, 2, 3, 4], $this->output);
     }
@@ -123,9 +123,7 @@ public function testInterrupt(Standard $pipeline): void
         });
 
         try {
-            $pipeline->each(function (int $value): void {
-                $this->observeValue($value);
-            });
+            $pipeline->each($this->observeValue(...));
         } catch (LogicException $_) {
             $this->assertSame([1, 2], $this->output);
         }
@@ -142,13 +140,9 @@ public function testNoDiscard(): void
     {
         $pipeline = fromArray([1, 2, 3]);
 
-        $pipeline->each(function (int $value): void {
-            $this->observeValue($value);
-        }, false);
+        $pipeline->each($this->observeValue(...), false);
 
-        $pipeline->each(function (int $value): void {
-            $this->observeValue($value);
-        });
+        $pipeline->each($this->observeValue(...));
 
         $this->assertSame([1, 2, 3, 1, 2, 3], $this->output);
         $this->assertSame([], $pipeline->toList());
@@ -206,8 +200,6 @@ public function testCallableReassigned(): void
         $pipeline = fromArray(['1', '2', '3']);
         $pipeline->each($callback);
 
-        $this->assertSame(4, $callback->callCount, 'Expected 1 initial call that throws + 3 successful calls after wrapping');
-
         $this->assertSame([
             ['1', 0],
             ['1'],

tests/Fixtures/CallableThrower.php

@@ -20,17 +20,17 @@
 
 use ArgumentCountError;
 
+use function count;
+
 class CallableThrower
 {
     public array $args = [];
-    public int $callCount = 0;
 
     public function __invoke(...$args): void
     {
         $this->args[] = $args;
-        $this->callCount++;
 
-        if (1 === $this->callCount) {
+        if (count($args) > 1) {
             throw new ArgumentCountError();
         }
     }