From e1a2122f3b4156fedb984d83ebe3186eb25a792e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Sun, 25 Feb 2024 13:31:37 +0100 Subject: [PATCH] Cleanup deprecations --- UPGRADE.md | 25 ++++++++- src/Criteria.php | 86 +++---------------------------- tests/ArrayCollectionTestCase.php | 13 +---- tests/CriteriaTest.php | 56 +------------------- 4 files changed, 33 insertions(+), 147 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 54203d98..3c31b693 100644 --- 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`. +- `Criteria::orderBy()` no longer accepts `array`, pass + `array` instead. +- `Criteria::__construct()` no longer accepts `array` as + $orderings, pass `array` 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 index ce227374..84d90798 100644 --- 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 */ @@ -56,25 +45,16 @@ public static function expr(): ExpressionBuilder /** * Construct a new Criteria. * - * @param array|null $orderings + * @param array|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 - */ - 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 $orderings + * @param array $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 index 260fe724..06b42950 100644 --- 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 index 8360d640..025ae8be 100644 --- 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(); - } }