Fixes & improvements

docs/pages/api.rst

@@ -560,7 +560,7 @@ how values are accessed and compared to each other.
 The first parameter is the comparator. This is a curried function which takes
 first the left part, then the right part and then returns a boolean.
 
-The second parameter is the accessor. This binary function take the value and
+The second parameter is the accessor. This binary function takes the value and
 the key of the current iterated value and then return the value to compare.
 This is useful when you want to compare objects.
 

docs/pages/code/operations/distinct.php

@@ -14,11 +14,11 @@
 
 include __DIR__ . '/../../../../vendor/autoload.php';
 
-// Example 1 -> Using the default callback, with scalar values
+// Example 1 -> Using the default callbacks, with scalar values
 $collection = Collection::fromIterable(['a', 'b', 'a', 'c'])
     ->distinct(); // [0 => 'a', 1 => 'b', 3 => 'c']
 
-// Example 2 -> Using one custom callback, with object values
+// Example 2 -> Using a custom comparator callback, with object values
 final class User
 {
     private string $name;
@@ -46,7 +46,36 @@ public function name(): string
         static fn (User $left): Closure => static fn (User $right): bool => $left->name() === $right->name()
     ); // [0 => User<foo>, 1 => User<bar>, 3 => User<a>]
 
-// Example 3 -> Using two custom callbacks, with object values
+// Example 3 -> Using a custom accessor callback, with object values
+final class Person
+{
+    private string $name;
+
+    public function __construct(string $name)
+    {
+        $this->name = $name;
+    }
+
+    public function name(): string
+    {
+        return $this->name;
+    }
+}
+
+$users = [
+    new Person('foo'),
+    new Person('bar'),
+    new Person('foo'),
+    new Person('a'),
+];
+
+$collection = Collection::fromIterable($users)
+    ->distinct(
+        null,
+        static fn (Person $person): string => $person->name()
+    ); // [0 => Person<foo>, 1 => Person<bar>, 3 => Person<a>]
+
+// Example 4 -> Using both accessor and comparator callbacks, with object values
 final class Cat
 {
     private string $name;

spec/loophp/collection/CollectionSpec.php

@@ -765,6 +765,10 @@ public function it_can_distinct(): void
             ->distinct()
             ->shouldIterateAs([0 => 1, 2 => 2, 4 => 3, 6 => $stdclass]);
 
+        $this::fromIterable(['foo' => 'f', 'bar' => 'b', 'baz' => 'f'])
+            ->distinct()
+            ->shouldIterateAs(['foo' => 'f', 'bar' => 'b']);
+
         $cat = static fn (string $name) => new class($name) {
             private string $name;
 
@@ -792,21 +796,21 @@ public function name(): string
 
         $this::fromIterable($cats)
             ->distinct(
-                static fn ($left) => static fn ($right) => $left->name() === $right->name()
+                static fn (object $left) => static fn (object $right) => $left->name() === $right->name()
             )
             ->shouldIterateAs([$cat1, $cat2, $cat3]);
 
         $this::fromIterable($cats)
             ->distinct(
-                static fn ($left) => static fn ($right) => $left === $right,
-                static fn ($cat): string => $cat->name()
+                static fn (string $left) => static fn (string $right) => $left === $right,
+                static fn (object $cat): string => $cat->name()
             )
             ->shouldIterateAs([$cat1, $cat2, $cat3]);
 
         $this::fromIterable($cats)
             ->distinct(
                 null,
-                static fn ($cat): string => $cat->name()
+                static fn (object $cat): string => $cat->name()
             )
             ->shouldIterateAs([$cat1, $cat2, $cat3]);
     }

src/Collection.php

@@ -277,13 +277,13 @@ public function distinct(?callable $comparatorCallback = null, ?callable $access
 
         $comparatorCallback ??=
             /**
-             * @param mixed $left
+             * @param T $left
              *
-             * @return Closure(mixed): bool
+             * @return Closure(T): bool
              */
             static fn ($left): Closure =>
                 /**
-                 * @param mixed $right
+                 * @param T $right
                  */
                 static fn ($right): bool => $left === $right;
 

src/Contract/Operation/Distinctable.php

@@ -19,8 +19,10 @@
 interface Distinctable
 {
     /**
-     * @param null|callable(mixed): (Closure(mixed): bool) $comparatorCallback
-     * @param null|callable(T, TKey): mixed $accessorCallback
+     * @template U
+     *
+     * @param null|callable(U): (Closure(U): bool) $comparatorCallback
+     * @param null|callable(T, TKey): U $accessorCallback
      *
      * @return Collection<TKey, T>
      */

src/Operation/Distinct.php

@@ -22,47 +22,60 @@
 final class Distinct extends AbstractOperation
 {
     /**
-     * @return Closure(callable(mixed): (Closure(mixed): bool)): Closure(callable(T, TKey): mixed): Closure(Iterator<TKey, T>): Generator<TKey, T>
+     * @template U
+     *
+     * @return Closure(callable(U): Closure(U): bool): Closure(callable(T, TKey): U): Closure(Iterator<TKey, T>): Generator<TKey, T>
      */
     public function __invoke(): Closure
     {
         return
             /**
-             * @param callable(mixed): (Closure(mixed): bool) $comparatorCallback
+             * @param callable(U): (Closure(U): bool) $comparatorCallback
              *
-             * @return Closure(callable(T, TKey): mixed): Closure(Iterator<TKey, T>): Generator<TKey, T>
+             * @return Closure(callable(T, TKey): U): Closure(Iterator<TKey, T>): Generator<TKey, T>
              */
             static fn (callable $comparatorCallback): Closure =>
                 /**
-                 * @param callable(T, TKey): mixed $accessorCallback
+                 * @param callable(T, TKey): U $accessorCallback
                  *
                  * @return Closure(Iterator<TKey, T>): Generator<TKey, T>
                  */
                 static function (callable $accessorCallback) use ($comparatorCallback): Closure {
+                    /**
+                     * @param callable(T, TKey): U $accessorCallback
+                     *
+                     * @return Closure(callable(U): Closure(U): bool): Closure(list<array{0: TKey, 1: T}>, array{0: TKey, 1: T}): list<array{0: TKey, 1: T}>
+                     */
                     $foldLeftCallbackBuilder =
-                        static fn (callable $accessorCallback): Closure => static fn (callable $comparatorCallback): Closure =>
+                        static fn (callable $accessorCallback): Closure =>
                             /**
-                             * @param list<array{0: TKey, 1: T}> $seen
-                             * @param array{0: TKey, 1: T} $value
+                             * @param callable(U): (Closure(U): bool) $comparatorCallback
+                             *
+                             * @return Closure(list<array{0: TKey, 1: T}>, array{0: TKey, 1: T}): list<array{0: TKey, 1: T}>
                              */
-                            static function (array $seen, array $value) use ($accessorCallback, $comparatorCallback): array {
-                                $isSeen = false;
-                                $comparator = $comparatorCallback($accessorCallback($value[1], $value[0]));
+                            static fn (callable $comparatorCallback): Closure =>
+                                /**
+                                 * @param list<array{0: TKey, 1: T}> $seen
+                                 * @param array{0: TKey, 1: T} $value
+                                 */
+                                static function (array $seen, array $value) use ($accessorCallback, $comparatorCallback): array {
+                                    $isSeen = false;
+                                    $comparator = $comparatorCallback($accessorCallback($value[1], $value[0]));
 
-                                foreach ($seen as $item) {
-                                    if (true === $comparator($accessorCallback($item[1], $item[0]))) {
-                                        $isSeen = true;
+                                    foreach ($seen as $item) {
+                                        if (true === $comparator($accessorCallback($item[1], $item[0]))) {
+                                            $isSeen = true;
 
-                                        break;
+                                            break;
+                                        }
                                     }
-                                }
 
-                                if (false === $isSeen) {
-                                    $seen[] = $value;
-                                }
+                                    if (false === $isSeen) {
+                                        $seen[] = $value;
+                                    }
 
-                                return $seen;
-                            };
+                                    return $seen;
+                                };
 
                     /** @var Closure(Iterator<TKey, T>): Generator<TKey, T> $pipe */
                     $pipe = Pipe::of()(

tests/static-analysis/distinct.php

@@ -15,51 +15,68 @@
 /**
  * @param CollectionInterface<int, int> $collection
  */
-function distinct_checkList(CollectionInterface $collection): void
+function distinct_checkIntList(CollectionInterface $collection): void
 {
 }
 /**
- * @param CollectionInterface<string, string> $collection
+ * @param CollectionInterface<int, stdClass> $collection
  */
-function distinct_checkMap(CollectionInterface $collection): void
+function distinct_checkObjectList(CollectionInterface $collection): void
 {
 }
-function distinct_checkIntElement(int $value): void
-{
-}
-function distinct_checkNullableInt(?int $value): void
-{
-}
-function distinct_checkStringElement(string $value): void
-{
-}
-function distinct_checkNullableString(?string $value): void
+/**
+ * @param CollectionInterface<string, string> $collection
+ */
+function distinct_checkMap(CollectionInterface $collection): void
 {
 }
 
-distinct_checkList(Collection::fromIterable([1, 2, 3, 1])->distinct());
-distinct_checkMap(Collection::fromIterable(['a' => 'foo', 'b' => 'bar', 'c' => 'baz', 'd' => 'bar'])->distinct());
+$cat = static function (string $name): stdClass {
+    $instance = new stdClass();
+    $instance->name = $name;
+
+    return $instance;
+};
+
+$cats = [
+    $cat1 = $cat('izumi'),
+    $cat2 = $cat('nakano'),
+    $cat3 = $cat('booba'),
+    $cat3,
+];
+
+$accessor = static fn (stdClass $object): string => $object->name;
+$stringComparator = static fn (string $left): Closure => static fn (string $right): bool => $left === $right;
+$objectComparator = static fn (stdClass $left): Closure => static fn (stdClass $right): bool => $left->name === $right->name;
+
+distinct_checkIntList(Collection::fromIterable([11, 12, 11, 13])->distinct());
+distinct_checkMap(Collection::fromIterable(['foo' => 'f', 'bar' => 'b', 'baz' => 'f'])->distinct());
+
+distinct_checkObjectList(Collection::fromIterable($cats)->distinct());
+distinct_checkObjectList(Collection::fromIterable($cats)->distinct($objectComparator));
+distinct_checkObjectList(Collection::fromIterable($cats)->distinct($stringComparator, $accessor));
+distinct_checkObjectList(Collection::fromIterable($cats)->distinct(null, $accessor));
 
-distinct_checkList(Collection::empty()->distinct());
-distinct_checkMap(Collection::empty()->distinct());
+// VALID failures
 
-distinct_checkNullableInt(Collection::fromIterable([1, 2, 3, 1])->distinct()->current());
-distinct_checkNullableString(Collection::fromIterable(['foo' => 'bar', 'baz' => 'bar'])->head()->current());
+// `distinct` does not change the collection types TKey, T
+/** @psalm-suppress InvalidScalarArgument @phpstan-ignore-next-line */
+distinct_checkIntList(Collection::fromIterable(['a', 'b', 'c'])->distinct());
+/** @psalm-suppress InvalidScalarArgument @phpstan-ignore-next-line */
+distinct_checkMap(Collection::fromIterable(['foo' => 1, 'bar' => 2, 'baz' => 'f'])->distinct());
 
-// This retrieval method doesn't cause static analysis complaints
-// but is not always reliable because of that.
-distinct_checkIntElement(Collection::fromIterable([1, 2, 3, 1])->distinct()->all()[0]);
-distinct_checkStringElement(Collection::fromIterable(['a' => 'foo', 'b' => 'bar', 'c' => 'baz', 'd' => 'bar'])->distinct()->all()['a']);
-distinct_checkStringElement(Collection::fromIterable(['a' => 'foo', 'b' => 'bar', 'c' => 'baz', 'd' => 'bar'])->distinct()->all()['b']);
+// mixing object comparator parameter types
+$objectComparator = static fn (stdClass $left): Closure => static fn (string $right): bool => $left->name === $right;
+/** @psalm-suppress InvalidArgument @phpstan-ignore-next-line */
+distinct_checkObjectList(Collection::fromIterable($cats)->distinct($objectComparator));
 
-// VALID failures - `current` returns T|null
-/** @psalm-suppress PossiblyNullArgument @phpstan-ignore-next-line */
-distinct_checkIntElement(Collection::fromIterable([1, 2, 3, 1])->distinct()->current());
-/** @psalm-suppress PossiblyNullArgument @phpstan-ignore-next-line */
-distinct_checkStringElement(Collection::fromIterable(['foo' => 'bar', 'bar'])->distinct()->current());
+// using wrong type parameter for accessor callback
+$accessor = static fn (string $object): string => $object;
+/** @psalm-suppress InvalidArgument */
+distinct_checkObjectList(Collection::fromIterable($cats)->distinct(null, $accessor));
 
-// VALID failures - these keys don't exist
-/** @psalm-suppress InvalidArrayOffset */
-distinct_checkIntElement(Collection::fromIterable([1, 2, 3, 1])->distinct()->all()[4]);
-/** @psalm-suppress InvalidArrayOffset @phpstan-ignore-next-line */
-distinct_checkStringElement(Collection::fromIterable(['foo' => 'bar', 'baz' => 'bar'])->distinct()->all()['plop']);
+// comparator parameter types need to match accessor return type
+$accessor = static fn (stdClass $object): string => $object->name;
+$objectComparator = static fn (stdClass $left): Closure => static fn (stdClass $right): bool => $left->name === $right->name;
+/** @psalm-suppress InvalidArgument @phpstan-ignore-next-line */
+distinct_checkObjectList(Collection::fromIterable($cats)->distinct($objectComparator, $accessor));