diff --git a/CHANGELOG.md b/CHANGELOG.md index 357110d..baaccd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## 5.1.0 - 2023-10-11 + +### Changed + +- Registered cleanup callbacks for lazy `Sequence`s and `Set`s are all called now for composed structures, instead of the last one + ## 5.0.0 - 2023-09-16 ### Changed diff --git a/README.md b/README.md index a18c096..da253ff 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A set of classes to wrap PHP primitives to build immutable data. -[Documentation](docs/) +[Documentation](docs/README.md) ## Installation diff --git a/src/RegisterCleanup.php b/src/RegisterCleanup.php new file mode 100644 index 0000000..f13c63b --- /dev/null +++ b/src/RegisterCleanup.php @@ -0,0 +1,66 @@ +cleanup = $cleanup; + } + + /** + * @param callable(): void $cleanup + */ + public function __invoke(callable $cleanup): void + { + $this->cleanup = $cleanup; + } + + /** + * @internal + * @psalm-pure + */ + public static function noop(): self + { + return new self(static fn() => null); + } + + /** + * @internal + */ + public function push(): self + { + return $this->child = self::noop(); + } + + /** + * @internal + */ + public function pop(): void + { + $this->child = null; + } + + /** + * @internal + */ + public function cleanup(): void + { + if ($this->child) { + $this->child->cleanup(); + } + + ($this->cleanup)(); + } +} diff --git a/src/Sequence.php b/src/Sequence.php index f48d447..4e1040c 100644 --- a/src/Sequence.php +++ b/src/Sequence.php @@ -79,7 +79,6 @@ public static function defer(\Generator $generator): self * * @template V * @psalm-pure - * @psalm-type RegisterCleanup = callable(callable(): void): void * * @param callable(RegisterCleanup): \Generator $generator * diff --git a/src/Sequence/Lazy.php b/src/Sequence/Lazy.php index 3e76b86..66c9439 100644 --- a/src/Sequence/Lazy.php +++ b/src/Sequence/Lazy.php @@ -10,13 +10,13 @@ Set, Maybe, SideEffect, + RegisterCleanup, }; /** * @template T * @implements Implementation * @psalm-immutable - * @psalm-type RegisterCleanup = callable(callable(): void): void */ final class Lazy implements Implementation { @@ -42,7 +42,7 @@ public function __invoke($element): Implementation $values = $this->values; return new self( - static function(callable $registerCleanup) use ($values, $element): \Generator { + static function(RegisterCleanup $registerCleanup) use ($values, $element): \Generator { foreach ($values($registerCleanup) as $value) { yield $value; } @@ -78,7 +78,7 @@ public function iterator(): \Iterator // the generator to cleanup its ressources, so we pass an empty function // that does nothing /** @psalm-suppress ImpureFunctionCall */ - return ($this->values)(self::bypassCleanup()); + return ($this->values)(RegisterCleanup::noop()); } /** @@ -88,16 +88,12 @@ public function get(int $index): Maybe { return Maybe::defer(function() use ($index) { $iteration = 0; - $cleanup = self::noCleanup(); - /** @psalm-suppress ImpureFunctionCall */ - $generator = ($this->values)(static function(callable $userDefinedCleanup) use (&$cleanup) { - $cleanup = $userDefinedCleanup; - }); + $register = RegisterCleanup::noop(); + $generator = ($this->values)($register); foreach ($generator as $value) { if ($index === $iteration) { - /** @psalm-suppress MixedFunctionCall Due to the reference in the closure above */ - $cleanup(); + $register->cleanup(); return Maybe::just($value); } @@ -131,7 +127,7 @@ public function distinct(): Implementation $values = $this->values; return new self( - static function(callable $registerCleanup) use ($values): \Generator { + static function(RegisterCleanup $registerCleanup) use ($values): \Generator { /** @var list */ $uniques = []; @@ -154,7 +150,7 @@ public function drop(int $size): Implementation $values = $this->values; return new self( - static function(callable $registerCleanup) use ($values, $size): \Generator { + static function(RegisterCleanup $registerCleanup) use ($values, $size): \Generator { $dropped = 0; foreach ($values($registerCleanup) as $value) { @@ -200,7 +196,7 @@ public function filter(callable $predicate): Implementation $values = $this->values; return new self( - static function(callable $registerCleanup) use ($values, $predicate): \Generator { + static function(RegisterCleanup $registerCleanup) use ($values, $predicate): \Generator { foreach ($values($registerCleanup) as $value) { if ($predicate($value)) { yield $value; @@ -273,16 +269,14 @@ public function last(): Maybe */ public function contains($element): bool { - $cleanup = self::noCleanup(); + $register = RegisterCleanup::noop(); /** @psalm-suppress ImpureFunctionCall */ - $generator = ($this->values)(static function(callable $userDefinedCleanup) use (&$cleanup) { - $cleanup = $userDefinedCleanup; - }); + $generator = ($this->values)($register); foreach ($generator as $value) { if ($value === $element) { - /** @psalm-suppress MixedFunctionCall Due to the reference in the closure above */ - $cleanup(); + /** @psalm-suppress ImpureMethodCall */ + $register->cleanup(); return true; } @@ -300,16 +294,12 @@ public function indexOf($element): Maybe { return Maybe::defer(function() use ($element) { $index = 0; - $cleanup = self::noCleanup(); - /** @psalm-suppress ImpureFunctionCall */ - $generator = ($this->values)(static function(callable $userDefinedCleanup) use (&$cleanup) { - $cleanup = $userDefinedCleanup; - }); + $register = RegisterCleanup::noop(); + $generator = ($this->values)($register); foreach ($generator as $value) { if ($value === $element) { - /** @psalm-suppress MixedFunctionCall Due to the reference in the closure above */ - $cleanup(); + $register->cleanup(); /** @var Maybe<0|positive-int> */ return Maybe::just($index); @@ -334,7 +324,7 @@ public function indices(): Implementation /** @var Implementation<0|positive-int> */ return new self( - static function(callable $registerCleanup) use ($values): \Generator { + static function(RegisterCleanup $registerCleanup) use ($values): \Generator { $index = 0; foreach ($values($registerCleanup) as $_) { @@ -356,7 +346,7 @@ public function map(callable $function): Implementation $values = $this->values; return new self( - static function(callable $registerCleanup) use ($values, $function): \Generator { + static function(RegisterCleanup $registerCleanup) use ($values, $function): \Generator { foreach ($values($registerCleanup) as $value) { yield $function($value); } @@ -378,11 +368,11 @@ public function flatMap(callable $map, callable $exfiltrate): self $values = $this->values; return new self( - static function(callable $registerCleanup) use ($values, $map, $exfiltrate): \Generator { + static function(RegisterCleanup $registerCleanup) use ($values, $map, $exfiltrate): \Generator { foreach ($values($registerCleanup) as $value) { $generator = self::open( $exfiltrate($map($value)), - $registerCleanup, + $registerCleanup->push(), ); foreach ($generator as $inner) { @@ -403,7 +393,7 @@ public function pad(int $size, $element): Implementation $values = $this->values; return new self( - static function(callable $registerCleanup) use ($values, $size, $element): \Generator { + static function(RegisterCleanup $registerCleanup) use ($values, $size, $element): \Generator { foreach ($values($registerCleanup) as $value) { yield $value; --$size; @@ -447,7 +437,7 @@ public function take(int $size): Implementation $values = $this->values; return new self( - static function(callable $registerCleanup) use ($values, $size): \Generator { + static function(RegisterCleanup $register) use ($values, $size): \Generator { $taken = 0; // We intercept the registering of the cleanup function here // because this generator can be stopped when we reach the number @@ -455,17 +445,12 @@ static function(callable $registerCleanup) use ($values, $size): \Generator { // the parent sequence may not need to cleanup as it could // iterate over the whole generator but this inner one still // needs to free resources correctly - $cleanup = self::noCleanup(); - $middleware = static function(callable $userDefinedCleanup) use (&$cleanup, $registerCleanup): void { - /** @var callable(): void $userDefinedCleanup */ - $cleanup = $userDefinedCleanup; - $registerCleanup($userDefinedCleanup); - }; + $middleware = $register->push(); foreach ($values($middleware) as $value) { if ($taken >= $size) { - /** @psalm-suppress MixedFunctionCall Due to the reference in the closure above */ - $cleanup(); + $middleware->cleanup(); + $register->pop(); return; } @@ -499,7 +484,7 @@ public function append(Implementation $sequence): Implementation $values = $this->values; return new self( - static function(callable $registerCleanup) use ($values, $sequence): \Generator { + static function(RegisterCleanup $registerCleanup) use ($values, $sequence): \Generator { foreach ($values($registerCleanup) as $value) { yield $value; } @@ -542,7 +527,7 @@ static function() use ($values, $function): \Generator { // bypass the registering of cleanup function as we iterate over // the whole generator - foreach ($values(self::bypassCleanup()) as $value) { + foreach ($values(RegisterCleanup::noop()) as $value) { $loaded[] = $value; } @@ -595,7 +580,7 @@ static function() use ($values): \Generator { // bypass the registering of cleanup function as we iterate over // the whole generator - foreach ($values(self::bypassCleanup()) as $value) { + foreach ($values(RegisterCleanup::noop()) as $value) { \array_unshift($reversed, $value); } @@ -620,7 +605,7 @@ public function toSequence(): Sequence $values = $this->values; return Sequence::lazy( - static function(callable $registerCleanup) use ($values): \Generator { + static function(RegisterCleanup $registerCleanup) use ($values): \Generator { foreach ($values($registerCleanup) as $value) { yield $value; } @@ -636,7 +621,7 @@ public function toSet(): Set $values = $this->values; return Set::lazy( - static function(callable $registerCleanup) use ($values): \Generator { + static function(RegisterCleanup $registerCleanup) use ($values): \Generator { foreach ($values($registerCleanup) as $value) { yield $value; } @@ -647,17 +632,13 @@ static function(callable $registerCleanup) use ($values): \Generator { public function find(callable $predicate): Maybe { return Maybe::defer(function() use ($predicate) { - $cleanup = self::noCleanup(); - /** @psalm-suppress ImpureFunctionCall */ - $generator = ($this->values)(static function(callable $userDefinedCleanup) use (&$cleanup) { - $cleanup = $userDefinedCleanup; - }); + $register = RegisterCleanup::noop(); + $generator = ($this->values)($register); foreach ($generator as $value) { /** @psalm-suppress ImpureFunctionCall */ if ($predicate($value) === true) { - /** @psalm-suppress MixedFunctionCall Due to the reference in the closure above */ - $cleanup(); + $register->cleanup(); return Maybe::just($value); } @@ -671,7 +652,7 @@ public function find(callable $predicate): Maybe public function match(callable $wrap, callable $match, callable $empty) { /** @psalm-suppress ImpureFunctionCall */ - $generator = ($this->values)(self::bypassCleanup()); + $generator = ($this->values)(RegisterCleanup::noop()); return (new Defer($generator))->match($wrap, $match, $empty); } @@ -689,17 +670,23 @@ public function zip(Implementation $sequence): Implementation /** @var Implementation */ return new self( - static function(callable $registerCleanup) use ($values, $sequence) { - $other = self::open($sequence, $registerCleanup); + static function(RegisterCleanup $register) use ($values, $sequence) { + $otherRegister = $register->push(); + $other = self::open($sequence, $otherRegister); - foreach ($values($registerCleanup) as $value) { + foreach ($values($register) as $value) { if (!$other->valid()) { + $register->pop(); + $register->cleanup(); + return; } yield [$value, $other->current()]; $other->next(); } + + $otherRegister->cleanup(); }, ); } @@ -716,7 +703,7 @@ public function safeguard($carry, callable $assert): self $values = $this->values; return new self( - static function(callable $registerCleanup) use ($values, $carry, $assert): \Generator { + static function(RegisterCleanup $registerCleanup) use ($values, $carry, $assert): \Generator { foreach ($values($registerCleanup) as $value) { $carry = $assert($carry, $value); @@ -736,7 +723,7 @@ static function(callable $registerCleanup) use ($values, $carry, $assert): \Gene */ public function aggregate(callable $map, callable $exfiltrate): self { - return new self(function(callable $registerCleanup) use ($map, $exfiltrate) { + return new self(function(RegisterCleanup $registerCleanup) use ($map, $exfiltrate) { $aggregate = new Aggregate($this->iterator()); /** @psalm-suppress MixedArgument */ $values = $aggregate(static fn($a, $b) => self::open( @@ -766,7 +753,7 @@ public function memoize(): Implementation public function dropWhile(callable $condition): self { /** @psalm-suppress ImpureFunctionCall */ - return new self(function($registerCleanup) use ($condition) { + return new self(function(RegisterCleanup $registerCleanup) use ($condition) { /** @psalm-suppress ImpureFunctionCall */ $generator = ($this->values)($registerCleanup); @@ -809,24 +796,19 @@ public function takeWhile(callable $condition): self $values = $this->values; return new self( - static function(callable $registerCleanup) use ($values, $condition): \Generator { + static function(RegisterCleanup $register) use ($values, $condition): \Generator { // We intercept the registering of the cleanup function here // because this generator can be stopped when we reach the number // of elements to take so we have to cleanup here. In this case // the parent sequence may not need to cleanup as it could // iterate over the whole generator but this inner one still // needs to free resources correctly - $cleanup = self::noCleanup(); - $middleware = static function(callable $userDefinedCleanup) use (&$cleanup, $registerCleanup): void { - /** @var callable(): void $userDefinedCleanup */ - $cleanup = $userDefinedCleanup; - $registerCleanup($userDefinedCleanup); - }; + $middleware = $register->push(); foreach ($values($middleware) as $value) { if (!$condition($value)) { - /** @psalm-suppress MixedFunctionCall Due to the reference in the closure above */ - $cleanup(); + $middleware->cleanup(); + $register->pop(); return; } @@ -855,13 +837,12 @@ private function load(): Implementation * @template A * * @param Implementation $sequence - * @param RegisterCleanup $registerCleanup * * @return \Iterator */ private static function open( Implementation $sequence, - callable $registerCleanup, + RegisterCleanup $registerCleanup, ): \Iterator { if ($sequence instanceof self) { return ($sequence->values)($registerCleanup); @@ -869,28 +850,4 @@ private static function open( return $sequence->iterator(); } - - /** - * @psalm-pure - * - * @return callable(): void - */ - private static function noCleanup(): callable - { - return static function(): void { - // nothing to do - }; - } - - /** - * @psalm-pure - * - * @return RegisterCleanup - */ - private static function bypassCleanup(): callable - { - return static function(): void { - // nothing to do - }; - } } diff --git a/src/Set.php b/src/Set.php index c9f0ea0..a7b1522 100644 --- a/src/Set.php +++ b/src/Set.php @@ -76,7 +76,6 @@ public static function defer(\Generator $generator): self * * @template V * @psalm-pure - * @psalm-type RegisterCleanup = callable(callable(): void): void * * @param callable(RegisterCleanup): \Generator $generator * diff --git a/src/Set/Lazy.php b/src/Set/Lazy.php index 4cf3e98..790de79 100644 --- a/src/Set/Lazy.php +++ b/src/Set/Lazy.php @@ -10,13 +10,13 @@ Str, Maybe, SideEffect, + RegisterCleanup, }; /** * @template T * @implements Implementation * @psalm-immutable - * @psalm-type RegisterCleanup = callable(callable(): void): void */ final class Lazy implements Implementation { diff --git a/tests/SequenceTest.php b/tests/SequenceTest.php index a7babe4..8ceb191 100644 --- a/tests/SequenceTest.php +++ b/tests/SequenceTest.php @@ -1303,6 +1303,143 @@ public function testTakeWhile() $this->assertFalse($reachedEnd); } + public function testCallAllCleanupsWhenFlatMappingComposedLazySequences() + { + $cleanups = []; + $sequence = Sequence::lazy(static function($register) use (&$cleanups) { + $register(static function() use (&$cleanups) { + $cleanups[] = 'parent'; + }); + + yield Sequence::lazy(static function($register) use (&$cleanups) { + $register(static function() use (&$cleanups) { + $cleanups = ['child1']; + }); + + yield 1; + yield 2; + }); + yield Sequence::lazy(static function($register) use (&$cleanups) { + $register(static function() use (&$cleanups) { + $cleanups = ['child2']; + }); + + yield 3; + yield 4; + }); + }) + ->flatMap(static fn($sequence) => $sequence); + + $this->assertSame([1], $sequence->take(1)->toList()); + $this->assertSame(['child1', 'parent'], $cleanups); + + $cleanups = []; + + $this->assertSame([1, 2], $sequence->take(2)->toList()); + $this->assertSame(['child2', 'parent'], $cleanups); + + $cleanups = []; + + $this->assertSame([1, 2, 3], $sequence->take(3)->toList()); + $this->assertSame(['child2', 'parent'], $cleanups); + + $cleanups = []; + + $this->assertSame([1, 2, 3, 4], $sequence->toList()); + $this->assertSame([], $cleanups); + } + + public function testCallCleanupWhenAppendingLazySequences() + { + $cleanups = []; + $sequence1 = Sequence::lazy(static function($register) use (&$cleanups) { + $register(static function() use (&$cleanups) { + $cleanups[] = 1; + }); + + yield 1; + yield 2; + }); + $sequence2 = Sequence::lazy(static function($register) use (&$cleanups) { + $register(static function() use (&$cleanups) { + $cleanups[] = 2; + }); + + yield 3; + yield 4; + }); + $sequence = $sequence1->append($sequence2); + + $this->assertSame([1], $sequence->take(1)->toList()); + $this->assertSame([1], $cleanups); + + $cleanups = []; + + $this->assertSame([1, 2, 3], $sequence->take(3)->toList()); + $this->assertSame([2], $cleanups); + + $cleanups = []; + + $this->assertSame([1, 2, 3, 4], $sequence->toList()); + $this->assertSame([], $cleanups); + } + + public function testCallCleanupWhenZippingLazySequences() + { + $cleanups = []; + $sequence1 = Sequence::lazy(static function($register) use (&$cleanups) { + $register(static function() use (&$cleanups) { + $cleanups[] = 1; + }); + + yield 1; + yield 2; + yield 3; + }); + $sequence2 = Sequence::lazy(static function($register) use (&$cleanups) { + $register(static function() use (&$cleanups) { + $cleanups[] = 2; + }); + + yield 3; + yield 4; + yield 5; + yield 6; + }); + $sequence = $sequence1->zip($sequence2); + + $this->assertSame([[1, 3], [2, 4], [3, 5]], $sequence->toList()); + $this->assertSame([2], $cleanups); + + $cleanups = []; + + $this->assertSame([[1, 3], [2, 4]], $sequence->take(2)->toList()); + $this->assertSame([2, 1], $cleanups); + + $cleanups = []; + $sequence1 = Sequence::lazy(static function($register) use (&$cleanups) { + $register(static function() use (&$cleanups) { + $cleanups[] = 1; + }); + + yield 1; + yield 2; + yield 3; + }); + $sequence2 = Sequence::lazy(static function($register) use (&$cleanups) { + $register(static function() use (&$cleanups) { + $cleanups[] = 2; + }); + + yield 3; + yield 4; + }); + $sequence = $sequence1->zip($sequence2); + + $this->assertSame([[1, 3], [2, 4]], $sequence->toList()); + $this->assertSame([1], $cleanups); + } + public function get($map, $index) { return $map->get($index)->match(