From b81939c238ecf43268e52e963d034943c2a0766a Mon Sep 17 00:00:00 2001 From: Alexey Kopytko Date: Sat, 15 Apr 2023 09:40:50 +0900 Subject: [PATCH 1/6] Improve empty tracking --- src/Standard.php | 78 +++++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/src/Standard.php b/src/Standard.php index 8e1486a..beb1b1a 100644 --- a/src/Standard.php +++ b/src/Standard.php @@ -75,6 +75,11 @@ public function __construct(?iterable $input = null) $this->pipeline = $input; } + private function empty(): bool + { + return [] === $this->pipeline || null === $this->pipeline; + } + /** * Appends the contents of an interable to the end of the pipeline. */ @@ -143,7 +148,7 @@ private function willReplace(?iterable $values = null): bool } // No shortcuts are applicable if the pipeline was initialized. - if ([] !== $this->pipeline && null !== $this->pipeline) { + if (!$this->empty()) { return false; } @@ -230,15 +235,13 @@ public function unpack(?callable $func = null): self */ public function chunk(int $length, bool $preserve_keys = false): self { - if (null === $this->pipeline) { - // No-op: null. + // No-op: an empty array or null. + if ($this->empty()) { return $this; } - if ([] === $this->pipeline) { - // No-op: an empty array. - return $this; - } + // Static analyzer hint + assert(null !== $this->pipeline && [] !== $this->pipeline); // Array shortcut if (is_array($this->pipeline)) { @@ -385,13 +388,8 @@ private static function applyOnce(iterable $previous, callable $func): iterable */ public function filter(?callable $func = null): self { - if (null === $this->pipeline) { - // No-op: null. - return $this; - } - - if ([] === $this->pipeline) { - // No-op: an empty array. + // No-op: an empty array or null. + if ($this->empty()) { return $this; } @@ -504,13 +502,8 @@ public function getIterator(): Traversable */ public function toArray(bool $preserve_keys = false): array { - if (null === $this->pipeline) { - // With non-primed pipeline just return an empty array. - return []; - } - - if ([] === $this->pipeline) { - // Empty array is empty. + // No-op: an empty array or null. + if ($this->empty()) { return []; } @@ -537,15 +530,13 @@ public function toArray(bool $preserve_keys = false): array */ public function count(): int { - if (null === $this->pipeline) { + if ($this->empty()) { // With non-primed pipeline just return zero. return 0; } - if ([] === $this->pipeline) { - // Empty array is empty. - return 0; - } + // Static analyzer hint + assert(null !== $this->pipeline && [] !== $this->pipeline); $result = 0; @@ -582,11 +573,14 @@ private static function generatorFromIterable(iterable $input): Generator */ public function slice(int $offset, ?int $length = null) { - if (null === $this->pipeline) { + if ($this->empty()) { // With non-primed pipeline just move along. return $this; } + // Static analyzer hint + assert(null !== $this->pipeline && [] !== $this->pipeline); + if (0 === $length) { // We're not consuming anything assuming total laziness. $this->pipeline = null; @@ -773,11 +767,17 @@ private static function toIterators(iterable ...$inputs): array */ public function reservoir(int $size, ?callable $weightFunc = null): array { - if (null === $this->pipeline) { + if ($this->empty()) { return []; } + // Static analyzer hint + assert(null !== $this->pipeline && [] !== $this->pipeline); + if ($size <= 0) { + // Discard the state to emulate a full consumption + $this->pipeline = null; + return []; } @@ -886,13 +886,12 @@ private static function random(): float */ public function min() { - if (null === $this->pipeline) { + if ($this->empty()) { return null; } - if ([] === $this->pipeline) { - return null; - } + // Static analyzer hint + assert(null !== $this->pipeline && [] !== $this->pipeline); if (is_array($this->pipeline)) { /** @psalm-suppress ArgumentTypeCoercion */ @@ -928,13 +927,12 @@ public function min() */ public function max() { - if (null === $this->pipeline) { + if ($this->empty()) { return null; } - if ([] === $this->pipeline) { - return null; - } + // Static analyzer hint + assert(null !== $this->pipeline && [] !== $this->pipeline); if (is_array($this->pipeline)) { /** @psalm-suppress ArgumentTypeCoercion */ @@ -960,15 +958,13 @@ public function max() */ public function flip() { - if (null === $this->pipeline) { + if ($this->empty()) { // No-op: null. return $this; } - if ([] === $this->pipeline) { - // No-op: an empty array. - return $this; - } + // Static analyzer hint + assert(null !== $this->pipeline && [] !== $this->pipeline); if (is_array($this->pipeline)) { $this->pipeline = array_flip($this->pipeline); From a9fd9114b835a8b3d2202a22ab46538a9babb9b7 Mon Sep 17 00:00:00 2001 From: Alexey Kopytko Date: Sat, 15 Apr 2023 13:31:14 +0900 Subject: [PATCH 2/6] Tweak infection.json.dist --- infection.json.dist | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/infection.json.dist b/infection.json.dist index ac52786..092ac57 100644 --- a/infection.json.dist +++ b/infection.json.dist @@ -1,11 +1,14 @@ { - "timeout": 2, + "timeout": 5, "source": { "directories": [ "src" ] }, "mutators": { + "global-ignoreSourceCodeByRegex": [ + "assert.*" + ], "@default": true, "IdenticalEqual": false, "NotIdenticalNotEqual": false From c15f70366b46295d1800958f2086fea55bcf3ed4 Mon Sep 17 00:00:00 2001 From: Alexey Kopytko Date: Sat, 15 Apr 2023 13:36:10 +0900 Subject: [PATCH 3/6] Update src/Standard.php --- src/Standard.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Standard.php b/src/Standard.php index beb1b1a..bb03dd9 100644 --- a/src/Standard.php +++ b/src/Standard.php @@ -775,7 +775,7 @@ public function reservoir(int $size, ?callable $weightFunc = null): array assert(null !== $this->pipeline && [] !== $this->pipeline); if ($size <= 0) { - // Discard the state to emulate a full consumption + // Discard the state to emulate full consumption $this->pipeline = null; return []; From e6fa097e2f8702d68d36295e20d6eff0ba3f0521 Mon Sep 17 00:00:00 2001 From: Alexey Kopytko Date: Sat, 15 Apr 2023 14:42:18 +0900 Subject: [PATCH 4/6] Use uninitialized state --- src/Standard.php | 69 ++++++++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/src/Standard.php b/src/Standard.php index bb03dd9..a516aeb 100644 --- a/src/Standard.php +++ b/src/Standard.php @@ -55,17 +55,30 @@ class Standard implements IteratorAggregate, Countable { /** - * Pre-primed pipeline. This is not a full `iterable` per se because we exclude IteratorAggregate before assigning a value. + * Pre-primed pipeline. * - * @var ?iterable + * This is not a full `iterable` per se because we exclude IteratorAggregate before assigning a value. */ - private ?iterable $pipeline; + private iterable $pipeline; /** * Contructor with an optional source of data. */ public function __construct(?iterable $input = null) { + if (null !== $input) { + $this->replace($input); + } + } + + private function replace(iterable $input): void + { + if (is_array($input)) { + $this->pipeline = $input; + + return; + } + // IteratorAggregate is a nuance best we avoid dealing with. // For example, CallbackFilterIterator needs a plain Iterator. while ($input instanceof IteratorAggregate) { @@ -77,7 +90,12 @@ public function __construct(?iterable $input = null) private function empty(): bool { - return [] === $this->pipeline || null === $this->pipeline; + return !isset($this->pipeline) || [] === $this->pipeline; + } + + private function discard(): void + { + $this->pipeline = null; } /** @@ -91,7 +109,6 @@ public function append(?iterable $values = null): self } // Static analyzer hints - assert(null !== $this->pipeline); assert(null !== $values); return $this->join($this->pipeline, $values); @@ -118,7 +135,6 @@ public function prepend(?iterable $values = null): self } // Static analyzer hints - assert(null !== $this->pipeline); assert(null !== $values); return $this->join($values, $this->pipeline); @@ -152,15 +168,8 @@ private function willReplace(?iterable $values = null): bool return false; } - // Install an array as it is. - if (is_array($values)) { - $this->pipeline = $values; - - return true; - } - - // Else we use ownself to handle edge cases. - $this->pipeline = new self($values); + // Handle edge cases + $this->replace($values); return true; } @@ -285,7 +294,7 @@ public function map(?callable $func = null): self } // That's the standard case for any next stages. - if (is_iterable($this->pipeline)) { + if (isset($this->pipeline) && is_iterable($this->pipeline)) { $this->pipeline = self::apply($this->pipeline, $func); return $this; @@ -348,13 +357,13 @@ public function cast(?callable $func = null): self } // We got an array, that's what we need. Moving along. - if (is_array($this->pipeline)) { + if (is_array($this->pipeline ?? null)) { $this->pipeline = array_map($func, $this->pipeline); return $this; } - if (is_iterable($this->pipeline)) { + if (is_iterable($this->pipeline ?? null)) { $this->pipeline = self::applyOnce($this->pipeline, $func); return $this; @@ -404,7 +413,7 @@ public function filter(?callable $func = null): self assert($this->pipeline instanceof Iterator); - /** @psalm-suppress ArgumentTypeCoercion */ + /** @psalm-suppress1 ArgumentTypeCoercion */ $this->pipeline = new CallbackFilterIterator($this->pipeline, $func); return $this; @@ -455,13 +464,17 @@ public function reduce(?callable $func = null, $initial = null) */ public function fold($initial, ?callable $func = null) { + if ($this->empty()) { + return $initial; + } + $func = self::resolveReducer($func); if (is_array($this->pipeline)) { return array_reduce($this->pipeline, $func, $initial); } - foreach ($this as $value) { + foreach ($this->pipeline as $value) { $initial = $func($initial, $value); } @@ -486,15 +499,15 @@ private static function resolveReducer(?callable $func): callable public function getIterator(): Traversable { - if ($this->pipeline instanceof Traversable) { - return $this->pipeline; + if (!isset($this->pipeline)) { + return new EmptyIterator(); } - if (null !== $this->pipeline) { - return new ArrayIterator($this->pipeline); + if ($this->pipeline instanceof Traversable) { + return $this->pipeline; } - return new EmptyIterator(); + return new ArrayIterator($this->pipeline); } /** @@ -583,7 +596,7 @@ public function slice(int $offset, ?int $length = null) if (0 === $length) { // We're not consuming anything assuming total laziness. - $this->pipeline = null; + $this->discard(); return $this; } @@ -705,7 +718,7 @@ private static function head(iterable $input, int $length): Generator */ public function zip(iterable ...$inputs) { - if (null === $this->pipeline) { + if (!isset($this->pipeline)) { $this->pipeline = array_shift($inputs); } @@ -776,7 +789,7 @@ public function reservoir(int $size, ?callable $weightFunc = null): array if ($size <= 0) { // Discard the state to emulate full consumption - $this->pipeline = null; + $this->discard(); return []; } From c35fd1a3f6f4d6d6d26cbbac88927a0e258fbcf3 Mon Sep 17 00:00:00 2001 From: Alexey Kopytko Date: Sat, 15 Apr 2023 15:01:14 +0900 Subject: [PATCH 5/6] Fix things --- .php-cs-fixer.dist.php | 1 + psalm.xml.dist | 3 +++ src/Standard.php | 48 ++++++++++++++++++------------------------ tests/ZipTest.php | 7 ++++++ 4 files changed, 31 insertions(+), 28 deletions(-) diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index f4319d3..971f5e0 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -60,6 +60,7 @@ 'random_api_migration' => false, 'blank_line_between_import_groups' => false, 'blank_line_before_statement' => ['statements' => ['break', 'continue', 'declare', 'return', 'throw', 'try']], + 'no_unset_on_property' => false, ]) ->setFinder( PhpCsFixer\Finder::create() diff --git a/psalm.xml.dist b/psalm.xml.dist index ba31bda..69b36b4 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -22,5 +22,8 @@ + + + diff --git a/src/Standard.php b/src/Standard.php index a516aeb..78ff50a 100644 --- a/src/Standard.php +++ b/src/Standard.php @@ -39,7 +39,6 @@ use function assert; use function count; use function is_array; -use function is_iterable; use function is_string; use function iterator_to_array; use function max; @@ -88,14 +87,20 @@ private function replace(iterable $input): void $this->pipeline = $input; } + /** + * @psalm-suppress TypeDoesNotContainType + */ private function empty(): bool { return !isset($this->pipeline) || [] === $this->pipeline; } + /** + * @phan-suppress PhanTypeObjectUnsetDeclaredProperty + */ private function discard(): void { - $this->pipeline = null; + unset($this->pipeline); } /** @@ -249,9 +254,6 @@ public function chunk(int $length, bool $preserve_keys = false): self return $this; } - // Static analyzer hint - assert(null !== $this->pipeline && [] !== $this->pipeline); - // Array shortcut if (is_array($this->pipeline)) { $this->pipeline = array_chunk($this->pipeline, $length, $preserve_keys); @@ -294,7 +296,7 @@ public function map(?callable $func = null): self } // That's the standard case for any next stages. - if (isset($this->pipeline) && is_iterable($this->pipeline)) { + if (isset($this->pipeline)) { $this->pipeline = self::apply($this->pipeline, $func); return $this; @@ -348,6 +350,8 @@ private static function apply(iterable $previous, callable $func): iterable * * @param ?callable $func a callback must return a value * + * @psalm-suppress RedundantCondition + * * @return $this */ public function cast(?callable $func = null): self @@ -357,13 +361,13 @@ public function cast(?callable $func = null): self } // We got an array, that's what we need. Moving along. - if (is_array($this->pipeline ?? null)) { + if (isset($this->pipeline) && is_array($this->pipeline)) { $this->pipeline = array_map($func, $this->pipeline); return $this; } - if (is_iterable($this->pipeline ?? null)) { + if (isset($this->pipeline)) { $this->pipeline = self::applyOnce($this->pipeline, $func); return $this; @@ -413,7 +417,7 @@ public function filter(?callable $func = null): self assert($this->pipeline instanceof Iterator); - /** @psalm-suppress1 ArgumentTypeCoercion */ + /** @psalm-suppress ArgumentTypeCoercion */ $this->pipeline = new CallbackFilterIterator($this->pipeline, $func); return $this; @@ -548,9 +552,6 @@ public function count(): int return 0; } - // Static analyzer hint - assert(null !== $this->pipeline && [] !== $this->pipeline); - $result = 0; foreach ($this->pipeline as $value) { @@ -591,9 +592,6 @@ public function slice(int $offset, ?int $length = null) return $this; } - // Static analyzer hint - assert(null !== $this->pipeline && [] !== $this->pipeline); - if (0 === $length) { // We're not consuming anything assuming total laziness. $this->discard(); @@ -718,8 +716,14 @@ private static function head(iterable $input, int $length): Generator */ public function zip(iterable ...$inputs) { + if ([] === $inputs) { + return $this; + } + if (!isset($this->pipeline)) { - $this->pipeline = array_shift($inputs); + $input = array_shift($inputs); + /** @var iterable $input */ + $this->pipeline = $input; } if ([] === $inputs) { @@ -784,9 +788,6 @@ public function reservoir(int $size, ?callable $weightFunc = null): array return []; } - // Static analyzer hint - assert(null !== $this->pipeline && [] !== $this->pipeline); - if ($size <= 0) { // Discard the state to emulate full consumption $this->discard(); @@ -903,9 +904,6 @@ public function min() return null; } - // Static analyzer hint - assert(null !== $this->pipeline && [] !== $this->pipeline); - if (is_array($this->pipeline)) { /** @psalm-suppress ArgumentTypeCoercion */ return min($this->pipeline); @@ -944,9 +942,6 @@ public function max() return null; } - // Static analyzer hint - assert(null !== $this->pipeline && [] !== $this->pipeline); - if (is_array($this->pipeline)) { /** @psalm-suppress ArgumentTypeCoercion */ return max($this->pipeline); @@ -976,9 +971,6 @@ public function flip() return $this; } - // Static analyzer hint - assert(null !== $this->pipeline && [] !== $this->pipeline); - if (is_array($this->pipeline)) { $this->pipeline = array_flip($this->pipeline); diff --git a/tests/ZipTest.php b/tests/ZipTest.php index 116a256..fe3eda4 100644 --- a/tests/ZipTest.php +++ b/tests/ZipTest.php @@ -148,6 +148,13 @@ public function testZipNothing(): void $this->assertSame([], $pipeline->zip([])->toArray()); } + public function testZipNothingNothing(): void + { + $pipeline = new Standard(); + + $this->assertSame([], $pipeline->zip()->toArray()); + } + public function testExample(): void { $iterable = range(5, 7); From 1b8e8a4507b7b31f6408e9dc6d41c7b366b365b9 Mon Sep 17 00:00:00 2001 From: Alexey Kopytko Date: Sat, 15 Apr 2023 21:01:08 +0900 Subject: [PATCH 6/6] Tweak comment --- src/Standard.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Standard.php b/src/Standard.php index 78ff50a..703f1cc 100644 --- a/src/Standard.php +++ b/src/Standard.php @@ -173,7 +173,7 @@ private function willReplace(?iterable $values = null): bool return false; } - // Handle edge cases + // Handle edge cases there $this->replace($values); return true;