Skip to content

Commit

Permalink
Deprecate not passing Criteria constants to orderBy()
Browse files Browse the repository at this point in the history
The signature of orderBy() is needlessly hard to understand for static
analysis. Also, if somebody makes a typo and writes "dsc", they will end
up with Criteria::ASC.
  • Loading branch information
greg0ire committed Feb 16, 2024
1 parent 3a5ceb7 commit 74be93f
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
12 changes: 12 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,18 @@ awareness about deprecated code.
- Use of our low-overhead runtime deprecation API, details:
https://github.com/doctrine/deprecations/

# Upgrade to 2.2

## Deprecated not using Criteria::ASC or Criteria::DESC for ordering

When calling `Criteria::orderBy()`, `Criteria::ASC` or `Criteria::DESC` should
be preferred.

```diff
-$criteria = Criteria::create()->orderBy(['foo' => 'asc']);
+$criteria = Criteria::create()->orderBy(['foo' => Criteria::ASC]);
```

# Upgrade to 2.0

## BC breaking changes
Expand Down
14 changes: 13 additions & 1 deletion src/Criteria.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use function array_map;
use function func_num_args;
use function in_array;
use function strtoupper;

/**
Expand Down Expand Up @@ -173,7 +174,18 @@ public function getOrderings()
public function orderBy(array $orderings)
{
$this->orderings = array_map(
static fn (string $ordering): string => strtoupper($ordering) === self::ASC ? self::ASC : self::DESC,
static function (string $ordering): string {
if (! in_array($ordering, [self::ASC, self::DESC], true)) {
Deprecation::trigger(
'doctrine/collections',
'https://github.com/doctrine/collections/pull/368',
'Passing anything other than Criteria::ASC or Criteria::DESC to %s() is deprecated. Pass one of these constants instead.',
__METHOD__,
);
}

return strtoupper($ordering) === self::ASC ? self::ASC : self::DESC;
},
$orderings,
);

Expand Down
12 changes: 12 additions & 0 deletions tests/Common/Collections/CriteriaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,16 @@ public function testExpr(): void
{
self::assertInstanceOf(ExpressionBuilder::class, Criteria::expr());
}

public function testPassingSloppyOrderingArgumentsIsDeprecated(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/368');
$criteria = Criteria::create()->orderBy(['username' => 'asc']);
}

public function testPassingConstantsIsTheRightWay(): void
{
$this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/368');
$criteria = Criteria::create()->orderBy(['username' => Criteria::ASC]);
}
}

0 comments on commit 74be93f

Please sign in to comment.