Merge 2.2.x up into 3.0.x

.github/workflows/documentation.yml

@@ -33,15 +33,12 @@ jobs:
         run: "rm composer.json"
 
       - name: "Require phpdocumentor/guides-cli"
-        run: "composer require --dev phpdocumentor/guides-cli dev-main@dev --no-update"
-
-      - name: "Configure minimum stability"
-        run: "composer config minimum-stability dev"
+        run: "composer require --dev phpdocumentor/guides-cli --no-update"
 
       - name: "Install dependencies with Composer"
         uses: "ramsey/composer-install@v2"
         with:
           dependency-versions: "highest"
 
       - name: "Run guides-cli"
-        run: "vendor/bin/guides -vvv --no-progress --fail-on-log docs/en /tmp/test"
+        run: "vendor/bin/guides -vvv --no-progress --fail-on-log docs/en"

UPGRADE.md

@@ -13,6 +13,22 @@ awareness about deprecated code.
 Native return types have been added. The new signatures are already described
 below [in the section about upgrading to 2.0](#upgrade-to-20).
 
+# Upgrade to 2.2
+
+## Deprecated string representation of sort order
+
+Criteria orderings direction is now represented by the
+`Doctrine\Common\Collection\Order` enum.
+
+As a consequence:
+
+- `Criteria::ASC` and `Criteria::DESC` are deprecated in favor of
+  `Order::Ascending` and `Order::Descending`, respectively.
+- `Criteria::getOrderings()` is deprecated in favor of `Criteria::orderings()`,
+  which returns `array<string, Order>`.
+- `Criteria::orderBy()` accepts `array<string, string|Order>`, but passing
+  anything other than `array<string, Order>` is deprecated.
+
 # Upgrade to 2.0
 
 ## BC breaking changes

composer.json

@@ -41,7 +41,7 @@
         "doctrine/coding-standard": "^12",
         "phpstan/phpstan": "^1.8",
         "phpstan/phpstan-phpunit": "^1.0",
-        "phpunit/phpunit": "^9.5",
+        "phpunit/phpunit": "^10.5",
         "vimeo/psalm": "^5.11"
     },
     "autoload": {
@@ -51,7 +51,7 @@
     },
     "autoload-dev": {
         "psr-4": {
-            "Doctrine\\Tests\\": "tests"
+            "Doctrine\\Tests\\Common\\Collections\\": "tests"
         }
     },
     "config": {

docs/en/expressions.rst

@@ -76,7 +76,7 @@ orderBy
 Sets the ordering of the result of this Criteria.
 
 .. code-block:: php
-    $criteria->orderBy(['name' => Criteria::ASC]);
+    $criteria->orderBy(['name' => Order::Ascending]);
 
 setFirstResult
 --------------

docs/en/sidebar.rst

@@ -1,3 +1,5 @@
+:orphan:
+
 .. toctree::
     :depth: 3
 

phpcs.xml.dist

@@ -32,10 +32,10 @@
     </rule>
 
     <rule ref="Squiz.NamingConventions.ValidVariableName.NotCamelCaps">
-        <exclude-pattern>tests/Common/Collections/ClosureExpressionVisitorTest.php</exclude-pattern>
+        <exclude-pattern>tests/ClosureExpressionVisitorTest.php</exclude-pattern>
     </rule>
     <rule ref="Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps">
-        <exclude-pattern>tests/Common/Collections/ClosureExpressionVisitorTest.php</exclude-pattern>
+        <exclude-pattern>tests/ClosureExpressionVisitorTest.php</exclude-pattern>
     </rule>
 
     <rule ref="SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingNativeTypeHint">

phpunit.xml.dist

@@ -1,9 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
-
 <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
      xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd"
      colors="true"
-     verbose="true"
      beStrictAboutOutputDuringTests="true"
      beStrictAboutTodoAnnotatedTests="true"
 >
@@ -13,9 +11,9 @@
         </testsuite>
     </testsuites>
 
-    <coverage>
+    <source>
         <include>
             <directory>./src/</directory>
         </include>
-    </coverage>
+    </source>
 </phpunit>

src/ArrayCollection.php

@@ -396,12 +396,12 @@ public function matching(Criteria $criteria): Collection
             $filtered = array_filter($filtered, $filter);
         }
 
-        $orderings = $criteria->getOrderings();
+        $orderings = $criteria->orderings();
 
         if ($orderings) {
             $next = null;
             foreach (array_reverse($orderings) as $field => $ordering) {
-                $next = ClosureExpressionVisitor::sortByField($field, $ordering === Criteria::DESC ? -1 : 1, $next);
+                $next = ClosureExpressionVisitor::sortByField($field, $ordering === Order::Descending ? -1 : 1, $next);
             }
 
             uasort($filtered, $next);
@@ -410,7 +410,7 @@ public function matching(Criteria $criteria): Collection
         $offset = $criteria->getFirstResult();
         $length = $criteria->getMaxResults();
 
-        if ($offset || $length) {
+        if ($offset !== null && $offset > 0 || $length !== null && $length > 0) {
             $filtered = array_slice($filtered, (int) $offset, $length, true);
         }
 

src/Criteria.php

@@ -19,12 +19,15 @@
  */
 class Criteria
 {
-    final public const ASC  = 'ASC';
+    /** @deprecated use Order::Ascending instead */
+    final public const ASC = 'ASC';
+
+    /** @deprecated use Order::Descending instead */
     final public const DESC = 'DESC';
 
     private static ExpressionBuilder|null $expressionBuilder = null;
 
-    /** @var array<string, string> */
+    /** @var array<string, Order> */
     private array $orderings = [];
 
     private int|null $firstResult = null;
@@ -53,7 +56,7 @@ public static function expr(): ExpressionBuilder
     /**
      * Construct a new Criteria.
      *
-     * @param array<string, string>|null $orderings
+     * @param array<string, string|Order>|null $orderings
      */
     public function __construct(
         private Expression|null $expression = null,
@@ -145,29 +148,71 @@ public function getWhereExpression(): Expression|null
     /**
      * Gets the current orderings of this Criteria.
      *
+     * @deprecated use orderings() instead
+     *
      * @return array<string, string>
      */
     public function getOrderings(): array
+    {
+        Deprecation::trigger(
+            'doctrine/collections',
+            'https://github.com/doctrine/collections/pull/389',
+            'Calling %s() is deprecated. Use %s::orderings() instead.',
+            __METHOD__,
+            self::class,
+        );
+
+        return array_map(
+            static fn (Order $ordering): string => $ordering->value,
+            $this->orderings,
+        );
+    }
+
+    /**
+     * Gets the current orderings of this Criteria.
+     *
+     * @return array<string, Order>
+     */
+    public function orderings(): array
     {
         return $this->orderings;
     }
 
     /**
      * Sets the ordering of the result of this Criteria.
      *
-     * Keys are field and values are the order, being either ASC or DESC.
+     * Keys are field and values are the order, being a valid Order enum case.
      *
-     * @see Criteria::ASC
-     * @see Criteria::DESC
+     * @see Order::Ascending
+     * @see Order::Descending
      *
-     * @param array<string, string> $orderings
+     * @param array<string, string|Order> $orderings
      *
      * @return $this
      */
     public function orderBy(array $orderings): static
     {
         $this->orderings = array_map(
-            static fn (string $ordering): string => strtoupper($ordering) === self::ASC ? self::ASC : self::DESC,
+            static function (string|Order $ordering): Order {
+                if ($ordering instanceof Order) {
+                    return $ordering;
+                }
+
+                static $triggered = false;
+
+                if (! $triggered) {
+                    Deprecation::trigger(
+                        'doctrine/collections',
+                        'https://github.com/doctrine/collections/pull/389',
+                        'Passing non-Order enum values to %s() is deprecated. Pass Order enum values instead.',
+                        __METHOD__,
+                    );
+                }
+
+                $triggered = true;
+
+                return strtoupper($ordering) === Order::Ascending->value ? Order::Ascending : Order::Descending;
+            },
             $orderings,
         );
 

src/Order.php

@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Common\Collections;
+
+enum Order: string
+{
+    case Ascending  = 'ASC';
+    case Descending = 'DESC';
+}

src/ReadableCollection.php

@@ -223,10 +223,10 @@ public function findFirst(Closure $p): mixed;
      * Applies iteratively the given function to each element in the collection,
      * so as to reduce the collection to a single value.
      *
-     * @psalm-param Closure(TReturn|TInitial|null, T):(TInitial|TReturn) $func
-     * @psalm-param TInitial|null $initial
+     * @psalm-param Closure(TReturn|TInitial, T):TReturn $func
+     * @psalm-param TInitial $initial
      *
-     * @psalm-return TReturn|TInitial|null
+     * @psalm-return TReturn|TInitial
      *
      * @psalm-template TReturn
      * @psalm-template TInitial

tests/Common/Collections/AbstractLazyArrayCollectionTest.php → tests/AbstractLazyArrayCollectionTest.php

@@ -6,7 +6,6 @@
 
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\Common\Collections\Collection;
-use Doctrine\Tests\LazyArrayCollection;
 
 use function assert;
 
@@ -15,7 +14,7 @@
  *
  * @covers \Doctrine\Common\Collections\AbstractLazyCollection
  */
-class AbstractLazyArrayCollectionTest extends BaseArrayCollectionTest
+class AbstractLazyArrayCollectionTest extends ArrayCollectionTestCase
 {
     /**
      * @param mixed[] $elements

tests/Common/Collections/AbstractLazyCollectionTest.php → tests/AbstractLazyCollectionTest.php

@@ -5,7 +5,6 @@
 namespace Doctrine\Tests\Common\Collections;
 
 use Doctrine\Common\Collections\ArrayCollection;
-use Doctrine\Tests\LazyArrayCollection;
 
 use function assert;
 use function is_array;
@@ -17,7 +16,7 @@
  *
  * @covers \Doctrine\Common\Collections\AbstractLazyCollection
  */
-class AbstractLazyCollectionTest extends BaseCollectionTest
+class AbstractLazyCollectionTest extends CollectionTestCase
 {
     protected function setUp(): void
     {

tests/Common/Collections/ArrayCollectionTest.php → tests/ArrayCollectionTest.php

@@ -20,7 +20,7 @@
  *
  * @covers \Doctrine\Common\Collections\ArrayCollection
  */
-class ArrayCollectionTest extends BaseArrayCollectionTest
+class ArrayCollectionTest extends ArrayCollectionTestCase
 {
     /**
      * @param mixed[] $elements

tests/Common/Collections/BaseArrayCollectionTest.php → tests/ArrayCollectionTestCase.php

@@ -6,6 +6,7 @@
 
 use Doctrine\Common\Collections\Collection;
 use Doctrine\Common\Collections\Criteria;
+use Doctrine\Common\Collections\Order;
 use Doctrine\Common\Collections\Selectable;
 use PHPUnit\Framework\TestCase;
 use stdClass;
@@ -20,7 +21,7 @@
 use function next;
 use function reset;
 
-abstract class BaseArrayCollectionTest extends TestCase
+abstract class ArrayCollectionTestCase extends TestCase
 {
     /**
      * @param mixed[] $elements
@@ -183,7 +184,7 @@ public function testIterator(array $elements): void
     }
 
     /** @psalm-return array<string, array{mixed[]}> */
-    public function provideDifferentElements(): array
+    public static function provideDifferentElements(): array
     {
         return [
             'indexed'     => [[1, 2, 3, 4, 5]],
@@ -334,6 +335,15 @@ public function testMatchingWithSortingPreserveKeys(): void
                 ->matching(new Criteria(null, ['sortField' => Criteria::ASC]))
                 ->toArray(),
         );
+        self::assertSame(
+            [
+                'object2' => $object2,
+                'object1' => $object1,
+            ],
+            $collection
+                ->matching(new Criteria(null, ['sortField' => Order::Ascending]))
+                ->toArray(),
+        );
     }
 
     /**
@@ -359,7 +369,7 @@ public function testMatchingWithSlicingPreserveKeys(array $array, array $slicedA
     }
 
     /** @return mixed[][] */
-    public function provideSlices(): array
+    public static function provideSlices(): array
     {
         return [
             'preserve numeric keys' => [
@@ -443,7 +453,7 @@ public function testMultiColumnSortAppliesAllSorts(): void
         self::assertSame(
             $expected,
             $collection
-                ->matching(new Criteria(null, ['foo' => Criteria::DESC, 'bar' => Criteria::DESC]))
+                ->matching(new Criteria(null, ['foo' => Order::Descending, 'bar' => Order::Descending]))
                 ->toArray(),
         );
     }

tests/Common/Collections/CollectionTest.php → tests/CollectionTest.php

@@ -9,13 +9,14 @@
 use Doctrine\Common\Collections\Criteria;
 use Doctrine\Common\Collections\Expr\Expression;
 use Doctrine\Common\Collections\Expr\Value;
+use Doctrine\Common\Collections\Order;
 use RuntimeException;
 use stdClass;
 
 use function count;
 use function is_string;
 
-class CollectionTest extends BaseCollectionTest
+class CollectionTest extends CollectionTestCase
 {
     protected function setUp(): void
     {
@@ -72,7 +73,7 @@ public function testMatchingOrdering(): void
     {
         $this->fillMatchingFixture();
 
-        $col = $this->collection->matching(new Criteria(null, ['foo' => 'DESC']));
+        $col = $this->collection->matching(new Criteria(null, ['foo' => Order::Descending]));
 
         self::assertInstanceOf(Collection::class, $col);
         self::assertNotSame($col, $this->collection);

tests/Common/Collections/BaseCollectionTest.php → tests/CollectionTestCase.php

@@ -16,7 +16,7 @@
 use function is_string;
 use function sprintf;
 
-abstract class BaseCollectionTest extends TestCase
+abstract class CollectionTestCase extends TestCase
 {
     /** @var Collection<mixed> */
     protected Collection $collection;