Merge pull request #401 from greg0ire/cleanup-order

Cleanup deprecations
doctrine · Feb 26, 2024 · 3bab217 · 3bab217
2 parents 9ccf128 + e1a2122
commit 3bab217
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 147 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -8,7 +8,30 @@ awareness about deprecated code.
 
 # Upgrade to 3.0
 
-## BC breaking changes
+## Deprecated null first result
+
+Passing null as `$firstResult` to
+`Doctrine\Common\Collections\Criteria::__construct()` and to
+`Doctrine\Common\Collections\Criteria::setFirstResult()` is no longer possible.
+Use `0` instead.
+
+## Removed 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 removed in favor of
+  `Order::Ascending` and `Order::Descending`, respectively.
+- `Criteria::getOrderings()` is removed in favor of `Criteria::orderings()`,
+  which returns `array<string, Order>`.
+- `Criteria::orderBy()` no longer accepts `array<string, string>`, pass
+  `array<string, Order>` instead.
+- `Criteria::__construct()` no longer accepts `array<string, string>` as
+  $orderings, pass `array<string, Order>` instead.
+
+## Signature changes
 
 Native return types have been added. The new signatures are already described
 below [in the section about upgrading to 2.0](#upgrade-to-20).

diff --git a/src/Criteria.php b/src/Criteria.php
@@ -6,11 +6,6 @@
 
 use Doctrine\Common\Collections\Expr\CompositeExpression;
 use Doctrine\Common\Collections\Expr\Expression;
-use Doctrine\Deprecations\Deprecation;
-
-use function array_map;
-use function func_num_args;
-use function strtoupper;
 
 /**
  * Criteria for filtering Selectable collections.
@@ -19,12 +14,6 @@
  */
 class Criteria
 {
-    /** @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, Order> */
@@ -56,25 +45,16 @@ public static function expr(): ExpressionBuilder
     /**
      * Construct a new Criteria.
      *
-     * @param array<string, string|Order>|null $orderings
+     * @param array<string, Order>|null $orderings
      */
     public function __construct(
         private Expression|null $expression = null,
         array|null $orderings = null,
-        int|null $firstResult = null,
+        int $firstResult = 0,
         int|null $maxResults = null,
     ) {
         $this->expression = $expression;
 
-        if ($firstResult === null && func_num_args() > 2) {
-            Deprecation::trigger(
-                'doctrine/collections',
-                'https://github.com/doctrine/collections/pull/311',
-                'Passing null as $firstResult to the constructor of %s is deprecated. Pass 0 instead or omit the argument.',
-                self::class,
-            );
-        }
-
         $this->setFirstResult($firstResult);
         $this->setMaxResults($maxResults);
 
@@ -145,29 +125,6 @@ public function getWhereExpression(): Expression|null
         return $this->expression;
     }
 
-    /**
-     * 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.
      *
@@ -186,35 +143,13 @@ public function orderings(): array
      * @see Order::Ascending
      * @see Order::Descending
      *
-     * @param array<string, string|Order> $orderings
+     * @param array<string, Order> $orderings
      *
      * @return $this
      */
     public function orderBy(array $orderings): static
     {
-        $this->orderings = array_map(
-            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,
-        );
+        $this->orderings = $orderings;
 
         return $this;
     }
@@ -230,21 +165,12 @@ public function getFirstResult(): int|null
     /**
      * Set the number of first result that this Criteria should return.
      *
-     * @param int|null $firstResult The value to set.
+     * @param int $firstResult The value to set.
      *
      * @return $this
      */
-    public function setFirstResult(int|null $firstResult): static
+    public function setFirstResult(int $firstResult): static
     {
-        if ($firstResult === null) {
-            Deprecation::triggerIfCalledFromOutside(
-                'doctrine/collections',
-                'https://github.com/doctrine/collections/pull/311',
-                'Passing null to %s() is deprecated, pass 0 instead.',
-                __METHOD__,
-            );
-        }
-
         $this->firstResult = $firstResult;
 
         return $this;

diff --git a/tests/ArrayCollectionTestCase.php b/tests/ArrayCollectionTestCase.php
@@ -326,15 +326,6 @@ public function testMatchingWithSortingPreserveKeys(): void
             $this->markTestSkipped('Collection does not support Selectable interface');
         }
 
-        self::assertSame(
-            [
-                'object2' => $object2,
-                'object1' => $object1,
-            ],
-            $collection
-                ->matching(new Criteria(null, ['sortField' => Criteria::ASC]))
-                ->toArray(),
-        );
         self::assertSame(
             [
                 'object2' => $object2,
@@ -352,7 +343,7 @@ public function testMatchingWithSortingPreserveKeys(): void
      *
      * @dataProvider provideSlices
      */
-    public function testMatchingWithSlicingPreserveKeys(array $array, array $slicedArray, int|null $firstResult, int|null $maxResult): void
+    public function testMatchingWithSlicingPreserveKeys(array $array, array $slicedArray, int $firstResult, int|null $maxResult): void
     {
         $collection = $this->buildCollection($array);
 
@@ -426,7 +417,7 @@ public static function provideSlices(): array
                     'a' => 1,
                     'b' => 2,
                 ],
-                null,
+                0,
                 2,
             ],
         ];

diff --git a/tests/CriteriaTest.php b/tests/CriteriaTest.php
@@ -9,13 +9,10 @@
 use Doctrine\Common\Collections\Expr\CompositeExpression;
 use Doctrine\Common\Collections\ExpressionBuilder;
 use Doctrine\Common\Collections\Order;
-use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
 use PHPUnit\Framework\TestCase;
 
 class CriteriaTest extends TestCase
 {
-    use VerifyDeprecations;
-
     public function testCreate(): void
     {
         $criteria = Criteria::create();
@@ -29,36 +26,11 @@ public function testConstructor(): void
         $criteria = new Criteria($expr, ['foo' => Order::Ascending], 10, 20);
 
         self::assertSame($expr, $criteria->getWhereExpression());
-        self::assertSame(['foo' => Order::Ascending->value], $criteria->getOrderings());
-        self::assertSame(10, $criteria->getFirstResult());
-        self::assertSame(20, $criteria->getMaxResults());
-    }
-
-    public function testDeprecatedNullOffset(): void
-    {
-        $expr = new Comparison('field', '=', 'value');
-
-        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/311');
-        $criteria = new Criteria($expr, ['foo' => Order::Ascending], null, 20);
-
-        self::assertSame($expr, $criteria->getWhereExpression());
-        self::assertSame(['foo' => 'ASC'], $criteria->getOrderings());
         self::assertSame(['foo' => Order::Ascending], $criteria->orderings());
-        self::assertNull($criteria->getFirstResult());
+        self::assertSame(10, $criteria->getFirstResult());
         self::assertSame(20, $criteria->getMaxResults());
     }
 
-    public function testDefaultConstructor(): void
-    {
-        $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/311');
-        $criteria = new Criteria();
-
-        self::assertNull($criteria->getWhereExpression());
-        self::assertSame([], $criteria->getOrderings());
-        self::assertNull($criteria->getFirstResult());
-        self::assertNull($criteria->getMaxResults());
-    }
-
     public function testWhere(): void
     {
         $expr     = new Comparison('field', '=', 'value');
@@ -133,30 +105,4 @@ public function testExpr(): void
     {
         self::assertInstanceOf(ExpressionBuilder::class, Criteria::expr());
     }
-
-    public function testPassingNonOrderEnumToOrderByIsDeprecated(): void
-    {
-        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/389');
-        $criteria = Criteria::create()->orderBy(['foo' => 'ASC']);
-    }
-
-    public function testConstructingCriteriaWithNonOrderEnumIsDeprecated(): void
-    {
-        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/389');
-        $criteria = new Criteria(null, ['foo' => 'ASC']);
-    }
-
-    public function testUsingOrderEnumIsTheRightWay(): void
-    {
-        $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/389');
-        Criteria::create()->orderBy(['foo' => Order::Ascending]);
-        new Criteria(null, ['foo' => Order::Ascending]);
-    }
-
-    public function testCallingGetOrderingsIsDeprecated(): void
-    {
-        $criteria = Criteria::create()->orderBy(['foo' => Order::Ascending]);
-        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/389');
-        $criteria->getOrderings();
-    }
 }