Static Analysis Checks: Common Operations (II) (#109)

* SA Check: Filter

* SA Check: First

* SA Check: Head

* SA Check: Last + annotations tweaks

* Revert first annotation removal

docs/pages/api.rst

@@ -711,20 +711,22 @@ Signature: ``Collection::falsy();``
 filter
 ~~~~~~
 
-Filter collection items based on one or more callbacks.
+Filter collection items based on one or more callbacks. Multiple callbacks will be treated as a logical ``AND``.
 
 Interface: `Filterable`_
 
 Signature: ``Collection::filter(callable ...$callbacks);``
 
 .. code-block:: php
 
-    $callback = static function($value): bool {
-        return 0 === $value % 3;
-    };
+    $divisibleBy3 = static fn($value): bool => 0 === $value % 3;
+    $divisibleBy6 = static fn($value): bool => 0 === $value % 6;
+
+    $collection = Collection::fromIterable(range(1, 10))
+        ->filter($divisibleBy3); // [3, 6, 9]
 
     $collection = Collection::fromIterable(range(1, 10))
-        ->filter($callback); // [3, 6, 9]
+        ->filter($divisibleBy3, $divisibleBy6); // [6]
 
 first
 ~~~~~

spec/loophp/collection/CollectionSpec.php

@@ -1062,10 +1062,12 @@ public function it_can_filter(): void
     {
         $input = array_merge([0, false], range(1, 10));
 
-        $callable = static function ($value, $key, $iterator) {
+        $callable = static function ($value) {
             return $value % 2;
         };
 
+        $callableWithKey = static fn (int $value, int $key): bool => $value % 2 === 0 && 4 < $key;
+
         $this::fromIterable($input)
             ->filter($callable)
             ->count()
@@ -1076,6 +1078,10 @@ public function it_can_filter(): void
             ->normalize()
             ->shouldIterateAs([1, 3, 5, 7, 9]);
 
+        $this::fromIterable(range(0, 10))
+            ->filter($callableWithKey)
+            ->shouldIterateAs([6 => 6, 8 => 8, 10 => 10]);
+
         $this::fromIterable(['afooe', 'fooe', 'allo', 'llo'])
             ->filter(
                 static function ($value) {
@@ -1307,6 +1313,10 @@ public function it_can_get_the_first_item(): void
         $this::fromIterable([])
             ->first()
             ->shouldIterateAs([]);
+
+        $this::fromIterable(['foo' => 'bar', 'baz' => 'bar'])
+            ->first()
+            ->shouldIterateAs(['foo' => 'bar']);
     }
 
     public function it_can_get_the_last_item(): void
@@ -1323,6 +1333,10 @@ public function it_can_get_the_last_item(): void
             ->last()
             ->shouldIterateAs([]);
 
+        $this::fromIterable(['foo' => 'bar', 'baz' => 'bar'])
+            ->last()
+            ->shouldIterateAs(['baz' => 'bar']);
+
         $input = [
             ['a'],
             ['b', 'a'],
@@ -1505,11 +1519,17 @@ static function ($value, $key) {
 
     public function it_can_head(): void
     {
-        $input = range('A', 'E');
+        $this::fromIterable(range(1, 10))
+            ->head()
+            ->shouldIterateAs([0 => 1]);
 
-        $this::fromIterable($input)
+        $this::fromIterable([])
             ->head()
-            ->shouldIterateAs([0 => 'A']);
+            ->shouldIterateAs([]);
+
+        $this::fromIterable(['foo' => 'bar', 'baz' => 'bar'])
+            ->head()
+            ->shouldIterateAs(['foo' => 'bar']);
     }
 
     public function it_can_if_then_else(): void

src/Contract/Operation/Filterable.php

@@ -9,6 +9,7 @@
 
 namespace loophp\collection\Contract\Operation;
 
+use Iterator;
 use loophp\collection\Contract\Collection;
 
 /**
@@ -20,7 +21,7 @@ interface Filterable
     /**
      * Filter collection items based on one or more callbacks.
      *
-     * @param callable ...$callbacks
+     * @param callable(T, TKey, Iterator<TKey, T>): bool ...$callbacks
      *
      * @return Collection<TKey, T>
      */

src/Operation/Head.php

@@ -21,15 +21,15 @@
 final class Head extends AbstractOperation
 {
     /**
-     * @return Closure(Iterator<TKey, T>):Generator<TKey, T, mixed, EmptyIterator|mixed>
+     * @return Closure(Iterator<TKey, T>): Generator<TKey, T>
      */
     public function __invoke(): Closure
     {
         return
             /**
              * @param Iterator<TKey, T> $iterator
              *
-             * @return Generator<TKey, T, mixed, EmptyIterator|void>
+             * @return Generator<TKey, T>
              */
             static function (Iterator $iterator): Generator {
                 $isEmpty = true;

src/Operation/Last.php

@@ -11,6 +11,7 @@
 
 use Closure;
 use EmptyIterator;
+use Generator;
 use Iterator;
 
 /**
@@ -20,17 +21,17 @@
 final class Last extends AbstractOperation
 {
     /**
-     * @return Closure(Iterator<TKey, T>): Iterator<TKey, T>
+     * @return Closure(Iterator<TKey, T>): Generator<TKey, T>
      */
     public function __invoke(): Closure
     {
         return
             /**
              * @param Iterator<TKey, T> $iterator
              *
-             * @return Iterator<TKey, T>
+             * @return Generator<TKey, T>
              */
-            static function (Iterator $iterator): Iterator {
+            static function (Iterator $iterator): Generator {
                 $isEmpty = true;
 
                 foreach ($iterator as $key => $current) {

tests/static-analysis/filter.php

@@ -0,0 +1,48 @@
+<?php
+
+/**
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+include __DIR__ . '/../../vendor/autoload.php';
+
+use loophp\collection\Collection;
+use loophp\collection\Contract\Collection as CollectionInterface;
+
+/**
+ * @param CollectionInterface<int, int> $collection
+ */
+function filter_checkList(CollectionInterface $collection): void
+{
+}
+/**
+ * @param CollectionInterface<string, string> $collection
+ */
+function filter_checkMap(CollectionInterface $collection): void
+{
+}
+
+$intValueCallback = static fn (int $value): bool => $value % 2 === 0;
+$intValueCallback2 = static fn (int $value): bool => 2 < $value;
+$intKeyValueCallback1 = static fn (int $value, int $key): bool => $value % 2 === 0 && $key % 2 === 0;
+$intKeyValueCallback2 = static fn (int $value, int $key): bool => 2 < $value && 2 < $key;
+
+$stringValueCallback = static fn (string $value): bool => 'bar' === $value;
+$stringValueCallback2 = static fn (string $value): bool => '' === $value;
+$stringKeyValueCallback1 = static fn (string $value, string $key): bool => 'bar' !== $value && 'foo' !== $key;
+$stringKeyValueCallback2 = static fn (string $value, string $key): bool => 'bar' !== $value && '' === $key;
+
+filter_checkList(Collection::fromIterable([1, 2, 3])->filter());
+filter_checkList(Collection::fromIterable([1, 2, 3])->filter($intValueCallback));
+filter_checkList(Collection::fromIterable([1, 2, 3])->filter($intValueCallback, $intValueCallback2));
+filter_checkList(Collection::fromIterable([1, 2, 3])->filter($intKeyValueCallback1));
+filter_checkList(Collection::fromIterable([1, 2, 3])->filter($intKeyValueCallback1, $intKeyValueCallback2));
+
+filter_checkMap(Collection::fromIterable(['foo' => 'bar', 'baz' => ''])->filter());
+filter_checkMap(Collection::fromIterable(['foo' => 'bar', 'baz' => ''])->filter($stringValueCallback));
+filter_checkMap(Collection::fromIterable(['foo' => 'bar', 'baz' => ''])->filter($stringValueCallback, $stringValueCallback2));
+filter_checkMap(Collection::fromIterable(['foo' => 'bar', 'baz' => ''])->filter($stringKeyValueCallback1));
+filter_checkMap(Collection::fromIterable(['foo' => 'bar', 'baz' => ''])->filter($stringKeyValueCallback1, $stringKeyValueCallback2));

tests/static-analysis/first.php

@@ -0,0 +1,65 @@
+<?php
+
+/**
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+include __DIR__ . '/../../vendor/autoload.php';
+
+use loophp\collection\Collection;
+use loophp\collection\Contract\Collection as CollectionInterface;
+
+/**
+ * @param CollectionInterface<int, int> $collection
+ */
+function first_checkList(CollectionInterface $collection): void
+{
+}
+/**
+ * @param CollectionInterface<string, string> $collection
+ */
+function first_checkMap(CollectionInterface $collection): void
+{
+}
+function first_checkIntElement(int $value): void
+{
+}
+function first_checkNullableInt(?int $value): void
+{
+}
+function first_checkStringElement(string $value): void
+{
+}
+function first_checkNullableString(?string $value): void
+{
+}
+
+first_checkList(Collection::fromIterable([1, 2, 3])->first());
+first_checkMap(Collection::fromIterable(['foo' => 'bar', 'baz' => 'bar'])->first());
+
+first_checkList(Collection::empty()->first());
+first_checkMap(Collection::empty()->first());
+
+first_checkNullableInt(Collection::fromIterable([1, 2, 3])->first()->current());
+first_checkNullableString(Collection::fromIterable(['foo' => 'bar', 'baz' => 'bar'])->first()->current());
+
+// This retrieval method doesn't cause static analysis complaints
+// but is not always reliable because of that.
+first_checkIntElement(Collection::fromIterable([1, 2, 3])->first()->all()[0]);
+first_checkStringElement(Collection::fromIterable(['foo' => 'bar', 'baz' => 'bar'])->first()->all()['foo']);
+first_checkStringElement(Collection::fromIterable(['foo' => 'bar', 'baz' => 'bar'])->first()->all()['baz']);
+
+// VALID failures - `current` returns T|null
+/** @psalm-suppress PossiblyNullArgument @phpstan-ignore-next-line */
+first_checkIntElement(Collection::fromIterable([1, 2, 3])->first()->current());
+/** @psalm-suppress PossiblyNullArgument @phpstan-ignore-next-line */
+first_checkStringElement(Collection::fromIterable(['foo' => 'bar'])->first()->current());
+
+// VALID failures - these keys don't exist
+/** @psalm-suppress InvalidArrayOffset */
+first_checkIntElement(Collection::fromIterable([1, 2, 3])->first()->all()[4]);
+/** @psalm-suppress InvalidArrayOffset @phpstan-ignore-next-line */
+first_checkStringElement(Collection::fromIterable(['foo' => 'bar', 'baz' => 'bar'])->first()->all()[0]);

tests/static-analysis/head.php

@@ -0,0 +1,65 @@
+<?php
+
+/**
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+include __DIR__ . '/../../vendor/autoload.php';
+
+use loophp\collection\Collection;
+use loophp\collection\Contract\Collection as CollectionInterface;
+
+/**
+ * @param CollectionInterface<int, int> $collection
+ */
+function head_checkList(CollectionInterface $collection): void
+{
+}
+/**
+ * @param CollectionInterface<string, string> $collection
+ */
+function head_checkMap(CollectionInterface $collection): void
+{
+}
+function head_checkIntElement(int $value): void
+{
+}
+function head_checkNullableInt(?int $value): void
+{
+}
+function head_checkStringElement(string $value): void
+{
+}
+function head_checkNullableString(?string $value): void
+{
+}
+
+head_checkList(Collection::fromIterable([1, 2, 3])->head());
+head_checkMap(Collection::fromIterable(['foo' => 'bar', 'baz' => 'bar'])->head());
+
+head_checkList(Collection::empty()->head());
+head_checkMap(Collection::empty()->head());
+
+head_checkNullableInt(Collection::fromIterable([1, 2, 3])->head()->current());
+head_checkNullableString(Collection::fromIterable(['foo' => 'bar', 'baz' => 'bar'])->head()->current());
+
+// This retrieval method doesn't cause static analysis complaints
+// but is not always reliable because of that.
+head_checkIntElement(Collection::fromIterable([1, 2, 3])->head()->all()[0]);
+head_checkStringElement(Collection::fromIterable(['foo' => 'bar', 'baz' => 'bar'])->head()->all()['foo']);
+head_checkStringElement(Collection::fromIterable(['foo' => 'bar', 'baz' => 'bar'])->head()->all()['baz']);
+
+// VALID failures - `current` returns T|null
+/** @psalm-suppress PossiblyNullArgument @phpstan-ignore-next-line */
+head_checkIntElement(Collection::fromIterable([1, 2, 3])->head()->current());
+/** @psalm-suppress PossiblyNullArgument @phpstan-ignore-next-line */
+head_checkStringElement(Collection::fromIterable(['foo' => 'bar'])->head()->current());
+
+// VALID failures - these keys don't exist
+/** @psalm-suppress InvalidArrayOffset */
+head_checkIntElement(Collection::fromIterable([1, 2, 3])->head()->all()[4]);
+/** @psalm-suppress InvalidArrayOffset @phpstan-ignore-next-line */
+head_checkStringElement(Collection::fromIterable(['foo' => 'bar', 'baz' => 'bar'])->head()->all()[0]);