sort: improve sort operation (#334)

composer.json

@@ -37,7 +37,7 @@
     ],
     "require": {
         "php": ">= 8.1",
-        "loophp/iterators": "^3"
+        "loophp/iterators": "^3.1"
     },
     "require-dev": {
         "ext-pcov": "*",

docs/pages/api.rst

@@ -2135,18 +2135,21 @@ Signature: ``Collection::slice(int $offset, ?int $length = -1): Collection;``
 sort
 ~~~~
 
-Sort a collection using a callback. If no callback is provided, it will sort using natural order.
+Sort a collection using a callback. If no callback is provided, it will sort
+using natural order, ascending.
 
-By default, it will sort by values and using a callback. If you want to sort by keys, you can pass a parameter to change
-the behaviour or use twice the flip operation. See the example below.
+By default, it will sort by values and using the default callback. If you want
+to sort by keys, you can pass a parameter to change the behaviour.
 
-Since version 7.4, sorting is `stable` by default. Stable sort algorithms sort equal
-elements in the same order that they appear in the input.
+Since version 7.4, sorting is `stable` by default. Stable sort algorithms sort
+equal elements in the same order that they appear in the input.
 
 Interface: `Sortable`_
 
 Signature: ``Collection::sort(int $type = Sortable::BY_VALUES, ?callable $callback = null): Collection;``
 
+Callback signature: ``Closure(mixed $right, mixed $left, mixed $rightKey, mixed $leftKey): int``
+
 .. literalinclude:: code/operations/sort.php
   :language: php
 

docs/pages/code/operations/sort.php

@@ -17,14 +17,21 @@
 $collection = Collection::fromIterable(['z', 'y', 'x'])
     ->sort(
         Sortable::BY_VALUES,
-        static fn ($left, $right): int => $left <=> $right
+        static fn (string $left, string $right): int => $left <=> $right
+    ); // [0 => 'z', 1 => 'y', 2 => 'x']
+
+// Example 3 -> Regular values sorting with a custom callback, inverted
+$collection = Collection::fromIterable(['z', 'y', 'x'])
+    ->sort(
+        Sortable::BY_VALUES,
+        static fn (string $left, string $right): int => $right <=> $left
     ); // [2 => 'x', 1 => 'y', 0 => 'z']
 
-// Example 3 -> Regular keys sorting (no callback is needed here)
+// Example 4 -> Regular keys sorting (no callback is needed here)
 $collection = Collection::fromIterable([3 => 'z', 2 => 'y', 1 => 'x'])
     ->sort(Sortable::BY_KEYS); // [1 => 'x', 2 => 'y', 3 => 'z']
 
-// Example 4 -> Regular keys sorting using the flip() operation twice
+// Example 5 -> Regular keys sorting using the flip() operation twice
 $collection = Collection::fromIterable([3 => 'z', 2 => 'y', 1 => 'x'])
     ->flip() // Exchange values and keys
     ->sort() // Sort the values (which are now the keys)

src/Collection.php

@@ -735,7 +735,7 @@ public function slice(int $offset, int $length = -1): CollectionInterface
         return new self((new Operation\Slice())()($offset)($length), [$this]);
     }
 
-    public function sort(int $type = OperationInterface\Sortable::BY_VALUES, ?callable $callback = null): CollectionInterface
+    public function sort(int $type = OperationInterface\Sortable::BY_VALUES, null|callable|Closure $callback = null): CollectionInterface
     {
         return new self((new Operation\Sort())()($type)($callback), [$this]);
     }

src/CollectionDecorator.php

@@ -606,7 +606,7 @@ public function slice(int $offset, int $length = -1): static
         return new static($this->innerCollection->slice($offset, $length));
     }
 
-    public function sort(int $type = Operation\Sortable::BY_VALUES, ?callable $callback = null): static
+    public function sort(int $type = Operation\Sortable::BY_VALUES, null|callable|Closure $callback = null): static
     {
         return new static($this->innerCollection->sort($type, $callback));
     }

src/Contract/Operation/Sortable.php

@@ -4,6 +4,7 @@
 
 namespace loophp\collection\Contract\Operation;
 
+use Closure;
 use loophp\collection\Contract\Collection;
 
 /**
@@ -23,7 +24,9 @@ interface Sortable
      *
      * @see https://loophp-collection.readthedocs.io/en/stable/pages/api.html#sort
      *
+     * @param null|callable|Closure(T, T, TKey, TKey): int $callback
+     *
      * @return Collection<TKey, T>
      */
-    public function sort(int $type = Sortable::BY_VALUES, ?callable $callback = null): Collection;
+    public function sort(int $type = Sortable::BY_VALUES, null|callable|Closure $callback = null): Collection;
 }

src/Operation/Matching.php

@@ -43,7 +43,7 @@ static function (Criteria $criteria): Closure {
                     $next = null;
 
                     foreach (array_reverse($orderings) as $field => $ordering) {
-                        $next = ClosureExpressionVisitor::sortByField($field, Criteria::DESC === $ordering ? -1 : 1, $next);
+                        $next = ClosureExpressionVisitor::sortByField($field, Criteria::ASC === $ordering ? -1 : 1, $next);
                     }
 
                     $pipes[] = (new Sort())()(Sortable::BY_VALUES)($next);

src/Operation/Scale.php

@@ -60,7 +60,6 @@ static function (float|int $v) use ($lowerBound, $upperBound, $wantedLowerBound,
 
                                 $filter = (new Filter())()(
                                     static fn (float|int $item): bool => $item > $lowerBound,
-
                                     static fn (float|int $item): bool => $item <= $upperBound
                                 );
 

src/Operation/Sort.php

@@ -8,7 +8,7 @@
 use Exception;
 use Generator;
 use loophp\collection\Contract\Operation;
-use loophp\iterators\SortIterator;
+use loophp\iterators\SortIterableAggregate;
 
 /**
  * @immutable
@@ -19,76 +19,72 @@
 final class Sort extends AbstractOperation
 {
     /**
-     * @return Closure(int): Closure(null|(callable(T|TKey, T|TKey): int)): Closure(iterable<TKey, T>): Generator<TKey, T>
+     * @return Closure(int): Closure(null|(Closure(T, T, TKey, TKey): int)): Closure(iterable<TKey, T>): Generator<TKey, T>
      */
     public function __invoke(): Closure
     {
         return
             /**
-             * @return Closure(null|(callable(T|TKey, T|TKey): int)): Closure(iterable<TKey, T>): Generator<TKey, T>
+             * @return Closure(null|Closure(T, T, TKey, TKey): int): Closure(iterable<TKey, T>): Generator<TKey, T>
              */
             static fn (int $type = Operation\Sortable::BY_VALUES): Closure =>
-            /**
-             * @param null|(callable(T|TKey, T|TKey): int) $callback
-             *
-             * @return Closure(iterable<TKey, T>): Generator<TKey, T>
-             */
-            static function (?callable $callback = null) use ($type): Closure {
-                $callback ??=
-                    /**
-                     * @param T|TKey $left
-                     * @param T|TKey $right
-                     */
-                    static fn (mixed $left, mixed $right): int => $left <=> $right;
+                /**
+                 * @param null|(Closure(T, T, TKey, TKey): int)|(callable(T, T, TKey, TKey): int) $callback
+                 *
+                 * @return Closure(iterable<TKey, T>): Generator<TKey, T>
+                 */
+                static function (null|callable|Closure $callback = null) use ($type): Closure {
+                    if (Operation\Sortable::BY_VALUES !== $type && Operation\Sortable::BY_KEYS !== $type) {
+                        throw new Exception('Invalid sort type.');
+                    }
+
+                    $callback ??=
+                        /**
+                         * @param T $left
+                         * @param T $right
+                         * @param TKey $leftKey
+                         * @param TKey $rightKey
+                         */
+                        static fn (mixed $left, mixed $right, mixed $leftKey, mixed $rightKey): int => $right <=> $left;
+
+                    if (!($callback instanceof Closure)) {
+                        trigger_deprecation(
+                            'loophp/collection',
+                            '7.4',
+                            'Passing a callable as argument is deprecated and will be removed in 8.0. Use a closure instead.',
+                            self::class
+                        );
 
-                return
-                    /**
-                     * @param iterable<TKey, T> $iterable
-                     *
-                     * @return Generator<TKey, T>
-                     */
-                    static function (iterable $iterable) use ($type, $callback): Generator {
-                        if (Operation\Sortable::BY_VALUES !== $type && Operation\Sortable::BY_KEYS !== $type) {
-                            throw new Exception('Invalid sort type.');
-                        }
+                        $callback = Closure::fromCallable($callback);
+                    }
 
-                        $operations = Operation\Sortable::BY_VALUES === $type ?
-                            [
-                                'before' => [(new Pack())()],
-                                'after' => [(new Unpack())()],
-                            ] :
-                            [
-                                'before' => [(new Flip())(), (new Pack())()],
-                                'after' => [(new Unpack())(), (new Flip())()],
-                            ];
+                    $operations = Operation\Sortable::BY_VALUES === $type ?
+                        [
+                            'before' => [],
+                            'after' => [],
+                        ] :
+                        [
+                            'before' => [(new Flip())()],
+                            'after' => [(new Flip())()],
+                        ];
 
-                        $sortCallback =
-                            /**
-                             * @param callable(T|TKey, T|TKey): int $callback
-                             *
-                             * @return Closure(array{0:TKey|T, 1:T|TKey}, array{0:TKey|T, 1:T|TKey}): int
-                             */
-                            static fn (callable $callback): Closure =>
-                            /**
-                             * @param array{0:TKey|T, 1:T|TKey} $left
-                             * @param array{0:TKey|T, 1:T|TKey} $right
-                             */
-                            static fn (array $left, array $right): int => (0 === $return = $callback($right[1], $left[1])) ? ($right[0] <=> $left[0]) : $return;
+                    $sortedIterator =
+                        /**
+                         * @param iterable<TKey, T> $iterable
+                         *
+                         * @return SortIterableAggregate<TKey, T>
+                         */
+                        static fn (iterable $iterable): SortIterableAggregate => new SortIterableAggregate($iterable, $callback);
 
-                        $sortedIterator =
-                            /**
-                             * @param iterable<TKey, T> $iterable
-                             *
-                             * @return SortIterator<TKey, T>
-                             */
-                            static fn (iterable $iterable): SortIterator => new SortIterator($iterable, $sortCallback($callback));
+                    /** @var Closure(iterable<TKey, T>): Generator<TKey, T> $sort */
+                    $sort = (new Pipe())()(
+                        ...$operations['before'],
+                        ...[$sortedIterator],
+                        ...$operations['after']
+                    );
 
-                        yield from (new Pipe())()(
-                            ...$operations['before'],
-                            ...[$sortedIterator],
-                            ...$operations['after']
-                        )($iterable);
-                    };
-            };
+                    // Point free style.
+                    return $sort;
+                };
     }
 }

tests/unit/Traits/GenericCollectionProviders.php

@@ -4068,7 +4068,7 @@ public static function sortOperationProvider()
             $operation,
             [
                 Operation\Sortable::BY_VALUES,
-                static fn ($left, $right): int => $right <=> $left,
+                static fn (string $left, string $right): int => $left <=> $right,
             ],
             $input,
             array_combine(range('A', 'E'), range('E', 'A')),