Merge branch '3.0.x' into 3.1.x

* 3.0.x: PHPStan 1.10.59 (doctrine#11320) Address deprecations from Collection 2.2 (doctrine#11315) Fix sql walker phpdoc
derrabus · Feb 29, 2024 · 26f7588 · 26f7588
2 parents c02ddd6 + 20a6efd
commit 26f7588
Show file tree

Hide file tree

Showing 15 changed files with 138 additions and 36 deletions.
diff --git a/composer.json b/composer.json
@@ -38,7 +38,7 @@
     "require-dev": {
         "doctrine/coding-standard": "^12.0",
         "phpbench/phpbench": "^1.0",
-        "phpstan/phpstan": "1.10.35",
+        "phpstan/phpstan": "1.10.59",
         "phpunit/phpunit": "^10.4.0",
         "psr/log": "^1 || ^2 || ^3",
         "squizlabs/php_codesniffer": "3.7.2",

diff --git a/phpcs.xml.dist b/phpcs.xml.dist
@@ -274,4 +274,9 @@
         <!-- https://github.com/doctrine/orm/issues/8537 -->
         <exclude-pattern>src/QueryBuilder.php</exclude-pattern>
     </rule>
+
+    <rule ref="SlevomatCodingStandard.PHP.UselessParentheses">
+        <!-- We need those parentheses to make enum access seem like valid syntax on PHP 7 -->
+        <exclude-pattern>src/Mapping/Driver/XmlDriver.php</exclude-pattern>
+    </rule>
 </ruleset>
diff --git a/phpstan.neon b/phpstan.neon
@@ -30,7 +30,7 @@ parameters:
             message: '~^Result of method Doctrine\\DBAL\\Connection::commit\(\) \(void\) is used\.$~'
             path: src/UnitOfWork.php
         -
-            message: '~^Strict comparison using === between void and false will always evaluate to false\.$~'
+            message: '~^Strict comparison using === between null and false will always evaluate to false\.$~'
             path: src/UnitOfWork.php
         -
             message: '~^Variable \$e on left side of \?\? always exists and is not nullable\.$~'

diff --git a/src/Cache/Persister/Entity/AbstractEntityPersister.php b/src/Cache/Persister/Entity/AbstractEntityPersister.php
@@ -17,6 +17,7 @@
 use Doctrine\ORM\Cache\TimestampCacheKey;
 use Doctrine\ORM\Cache\TimestampRegion;
 use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\Internal\CriteriaOrderings;
 use Doctrine\ORM\Mapping\AssociationMapping;
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Mapping\ClassMetadataFactory;
@@ -33,6 +34,8 @@
 
 abstract class AbstractEntityPersister implements CachedEntityPersister
 {
+    use CriteriaOrderings;
+
     protected UnitOfWork $uow;
     protected ClassMetadataFactory $metadataFactory;
 
@@ -426,7 +429,7 @@ public function count(array|Criteria $criteria = []): int
      */
     public function loadCriteria(Criteria $criteria): array
     {
-        $orderBy     = $criteria->getOrderings();
+        $orderBy     = self::getCriteriaOrderings($criteria);
         $limit       = $criteria->getMaxResults();
         $offset      = $criteria->getFirstResult();
         $query       = $this->persister->getSelectSQL($criteria);

diff --git a/src/Internal/CriteriaOrderings.php b/src/Internal/CriteriaOrderings.php
@@ -0,0 +1,54 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\ORM\Internal;
+
+use Doctrine\Common\Collections\Criteria;
+use Doctrine\Common\Collections\Order;
+
+use function array_map;
+use function class_exists;
+use function method_exists;
+use function strtoupper;
+
+trait CriteriaOrderings
+{
+    /**
+     * @return array<string, string>
+     *
+     * @psalm-suppress DeprecatedMethod We need to call the deprecated API if the new one does not exist yet.
+     */
+    private static function getCriteriaOrderings(Criteria $criteria): array
+    {
+        if (! method_exists(Criteria::class, 'orderings')) {
+            return $criteria->getOrderings();
+        }
+
+        return array_map(
+            static function (Order $order): string {
+                return $order->value;
+            },
+            $criteria->orderings(),
+        );
+    }
+
+    /**
+     * @param array<string, string> $orderings
+     *
+     * @return array<string, string>|array<string, Order>
+     */
+    private static function mapToOrderEnumIfAvailable(array $orderings): array
+    {
+        if (! class_exists(Order::class)) {
+            return $orderings;
+        }
+
+        return array_map(
+            static function (string $order): Order {
+                return Order::from(strtoupper($order));
+            },
+            $orderings,
+        );
+    }
+}
diff --git a/src/Mapping/DefaultTypedFieldMapper.php b/src/Mapping/DefaultTypedFieldMapper.php
@@ -4,6 +4,7 @@
 
 namespace Doctrine\ORM\Mapping;
 
+use BackedEnum;
 use DateInterval;
 use DateTime;
 use DateTimeImmutable;
@@ -16,6 +17,7 @@
 use function array_merge;
 use function assert;
 use function enum_exists;
+use function is_a;
 
 /** @psalm-type ScalarName = 'array'|'bool'|'float'|'int'|'string' */
 final class DefaultTypedFieldMapper implements TypedFieldMapper
@@ -52,18 +54,18 @@ public function validateAndComplete(array $mapping, ReflectionProperty $field):
             && ($type instanceof ReflectionNamedType)
         ) {
             if (! $type->isBuiltin() && enum_exists($type->getName())) {
-                $mapping['enumType'] = $type->getName();
-
                 $reflection = new ReflectionEnum($type->getName());
                 if (! $reflection->isBacked()) {
                     throw MappingException::backedEnumTypeRequired(
                         $field->class,
                         $mapping['fieldName'],
-                        $mapping['enumType'],
+                        $type->getName(),
                     );
                 }
 
-                $type = $reflection->getBackingType();
+                assert(is_a($type->getName(), BackedEnum::class, true));
+                $mapping['enumType'] = $type->getName();
+                $type                = $reflection->getBackingType();
 
                 assert($type instanceof ReflectionNamedType);
             }

diff --git a/src/Mapping/Driver/XmlDriver.php b/src/Mapping/Driver/XmlDriver.php
@@ -5,6 +5,7 @@
 namespace Doctrine\ORM\Mapping\Driver;
 
 use Doctrine\Common\Collections\Criteria;
+use Doctrine\Common\Collections\Order;
 use Doctrine\ORM\Mapping\Builder\EntityListenerBuilder;
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Mapping\MappingException;
@@ -17,6 +18,7 @@
 use SimpleXMLElement;
 
 use function assert;
+use function class_exists;
 use function constant;
 use function count;
 use function defined;
@@ -403,9 +405,10 @@ public function loadMetadataForClass($className, PersistenceClassMetadata $metad
                 if (isset($oneToManyElement->{'order-by'})) {
                     $orderBy = [];
                     foreach ($oneToManyElement->{'order-by'}->{'order-by-field'} ?? [] as $orderByField) {
+                        /** @psalm-suppress DeprecatedConstant */
                         $orderBy[(string) $orderByField['name']] = isset($orderByField['direction'])
                             ? (string) $orderByField['direction']
-                            : Criteria::ASC;
+                            : (class_exists(Order::class) ? (Order::Ascending)->value : Criteria::ASC);
                     }
 
                     $mapping['orderBy'] = $orderBy;
@@ -531,9 +534,10 @@ public function loadMetadataForClass($className, PersistenceClassMetadata $metad
                 if (isset($manyToManyElement->{'order-by'})) {
                     $orderBy = [];
                     foreach ($manyToManyElement->{'order-by'}->{'order-by-field'} ?? [] as $orderByField) {
+                        /** @psalm-suppress DeprecatedConstant */
                         $orderBy[(string) $orderByField['name']] = isset($orderByField['direction'])
                             ? (string) $orderByField['direction']
-                            : Criteria::ASC;
+                            : (class_exists(Order::class) ? (Order::Ascending)->value : Criteria::ASC);
                     }
 
                     $mapping['orderBy'] = $orderBy;

diff --git a/src/PersistentCollection.php b/src/PersistentCollection.php
@@ -9,6 +9,7 @@
 use Doctrine\Common\Collections\Collection;
 use Doctrine\Common\Collections\Criteria;
 use Doctrine\Common\Collections\Selectable;
+use Doctrine\ORM\Internal\CriteriaOrderings;
 use Doctrine\ORM\Mapping\AssociationMapping;
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Mapping\ToManyAssociationMapping;
@@ -40,6 +41,8 @@
  */
 final class PersistentCollection extends AbstractLazyCollection implements Selectable
 {
+    use CriteriaOrderings;
+
     /**
      * A snapshot of the collection at the moment it was fetched from the database.
      * This is used to create a diff of the collection at commit time.
@@ -585,7 +588,9 @@ public function matching(Criteria $criteria): Collection
 
         $criteria = clone $criteria;
         $criteria->where($expression);
-        $criteria->orderBy($criteria->getOrderings() ?: $association->orderBy());
+        $criteria->orderBy(self::mapToOrderEnumIfAvailable(
+            self::getCriteriaOrderings($criteria) ?: $association->orderBy(),
+        ));
 
         $persister = $this->getUnitOfWork()->getEntityPersister($association->targetEntity);
 

diff --git a/src/Persisters/Collection/ManyToManyPersister.php b/src/Persisters/Collection/ManyToManyPersister.php
@@ -9,6 +9,7 @@
 use Doctrine\Common\Collections\Expr\Comparison;
 use Doctrine\DBAL\Exception as DBALException;
 use Doctrine\DBAL\LockMode;
+use Doctrine\ORM\Internal\CriteriaOrderings;
 use Doctrine\ORM\Mapping\AssociationMapping;
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Mapping\InverseSideMapping;
@@ -32,6 +33,8 @@
  */
 class ManyToManyPersister extends AbstractCollectionPersister
 {
+    use CriteriaOrderings;
+
     public function delete(PersistentCollection $collection): void
     {
         $mapping = $this->getMapping($collection);
@@ -732,7 +735,7 @@ private function expandCriteriaParameters(Criteria $criteria): array
 
     private function getOrderingSql(Criteria $criteria, ClassMetadata $targetClass): string
     {
-        $orderings = $criteria->getOrderings();
+        $orderings = self::getCriteriaOrderings($criteria);
         if ($orderings) {
             $orderBy = [];
             foreach ($orderings as $name => $direction) {

diff --git a/src/Persisters/Entity/BasicEntityPersister.php b/src/Persisters/Entity/BasicEntityPersister.php
@@ -16,6 +16,7 @@
 use Doctrine\DBAL\Types\Type;
 use Doctrine\DBAL\Types\Types;
 use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\Internal\CriteriaOrderings;
 use Doctrine\ORM\Mapping\AssociationMapping;
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Mapping\JoinColumnMapping;
@@ -97,6 +98,7 @@
  */
 class BasicEntityPersister implements EntityPersister
 {
+    use CriteriaOrderings;
     use LockSqlHelper;
 
     /** @var array<string,string> */
@@ -842,7 +844,7 @@ public function count(array|Criteria $criteria = []): int
      */
     public function loadCriteria(Criteria $criteria): array
     {
-        $orderBy = $criteria->getOrderings();
+        $orderBy = self::getCriteriaOrderings($criteria);
         $limit   = $criteria->getMaxResults();
         $offset  = $criteria->getFirstResult();
         $query   = $this->getSelectSQL($criteria, null, null, $limit, $offset, $orderBy);

diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php
@@ -8,6 +8,7 @@
 use Doctrine\Common\Collections\Criteria;
 use Doctrine\DBAL\ArrayParameterType;
 use Doctrine\DBAL\ParameterType;
+use Doctrine\ORM\Internal\CriteriaOrderings;
 use Doctrine\ORM\Internal\NoUnknownNamedArguments;
 use Doctrine\ORM\Internal\QueryType;
 use Doctrine\ORM\Query\Expr;
@@ -41,6 +42,7 @@
  */
 class QueryBuilder implements Stringable
 {
+    use CriteriaOrderings;
     use NoUnknownNamedArguments;
 
     /**
@@ -1187,22 +1189,20 @@ public function addCriteria(Criteria $criteria): static
             }
         }
 
-        if ($criteria->getOrderings()) {
-            foreach ($criteria->getOrderings() as $sort => $order) {
-                $hasValidAlias = false;
-                foreach ($allAliases as $alias) {
-                    if (str_starts_with($sort . '.', $alias . '.')) {
-                        $hasValidAlias = true;
-                        break;
-                    }
-                }
-
-                if (! $hasValidAlias) {
-                    $sort = $allAliases[0] . '.' . $sort;
+        foreach (self::getCriteriaOrderings($criteria) as $sort => $order) {
+            $hasValidAlias = false;
+            foreach ($allAliases as $alias) {
+                if (str_starts_with($sort . '.', $alias . '.')) {
+                    $hasValidAlias = true;
+                    break;
                 }
+            }
 
-                $this->addOrderBy($sort, $order);
+            if (! $hasValidAlias) {
+                $sort = $allAliases[0] . '.' . $sort;
             }
+
+            $this->addOrderBy($sort, $order);
         }
 
         // Overwrite limits only if they was set in criteria

diff --git a/tests/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php b/tests/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php
@@ -6,6 +6,7 @@
 
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\Common\Collections\Criteria;
+use Doctrine\Common\Collections\Order;
 use Doctrine\ORM\Mapping\AssociationMapping;
 use Doctrine\ORM\PersistentCollection;
 use Doctrine\ORM\UnitOfWork;
@@ -16,6 +17,7 @@
 use PHPUnit\Framework\Attributes\Group;
 
 use function assert;
+use function class_exists;
 
 /**
  * Basic many-to-many association tests.
@@ -435,7 +437,7 @@ public function testManyToManyOrderByIsNotIgnored(): void
         $user = $this->_em->find($user::class, $user->id);
 
         $criteria = Criteria::create()
-            ->orderBy(['name' => Criteria::ASC]);
+            ->orderBy(['name' => class_exists(Order::class) ? Order::Ascending : Criteria::ASC]);
 
         self::assertEquals(
             ['A', 'B', 'C', 'Developers_0'],
@@ -475,7 +477,7 @@ public function testManyToManyOrderByHonorsFieldNameColumnNameAliases(): void
         $user = $this->_em->find($user::class, $user->id);
 
         $criteria = Criteria::create()
-            ->orderBy(['name' => Criteria::ASC]);
+            ->orderBy(['name' => class_exists(Order::class) ? Order::Ascending : Criteria::ASC]);
 
         self::assertEquals(
             ['A', 'B', 'C'],

diff --git a/tests/Tests/ORM/Functional/Ticket/GH7767Test.php b/tests/Tests/ORM/Functional/Ticket/GH7767Test.php
@@ -6,6 +6,8 @@
 
 use Doctrine\Common\Collections\Collection;
 use Doctrine\Common\Collections\Criteria;
+use Doctrine\Common\Collections\Order;
+use Doctrine\Common\Collections\Selectable;
 use Doctrine\ORM\Mapping\Column;
 use Doctrine\ORM\Mapping\Entity;
 use Doctrine\ORM\Mapping\GeneratedValue;
@@ -17,6 +19,7 @@
 use PHPUnit\Framework\Attributes\Group;
 
 use function assert;
+use function class_exists;
 
 #[Group('GH7767')]
 class GH7767Test extends OrmFunctionalTestCase
@@ -54,7 +57,9 @@ public function testMatchingOverrulesCollectionOrdering(): void
         $parent = $this->_em->find(GH7767ParentEntity::class, 1);
         assert($parent instanceof GH7767ParentEntity);
 
-        $children = $parent->getChildren()->matching(Criteria::create()->orderBy(['position' => 'DESC']));
+        $children = $parent->getChildren()->matching(
+            Criteria::create()->orderBy(['position' => class_exists(Order::class) ? Order::Descending : 'DESC']),
+        );
 
         self::assertEquals(300, $children[0]->position);
         self::assertEquals(200, $children[1]->position);
@@ -70,7 +75,7 @@ class GH7767ParentEntity
     #[GeneratedValue]
     private int $id;
 
-    /** @psalm-var Collection<int, GH7767ChildEntity> */
+    /** @psalm-var Collection<int, GH7767ChildEntity>&Selectable<int, GH7767ChildEntity> */
     #[OneToMany(targetEntity: GH7767ChildEntity::class, mappedBy: 'parent', fetch: 'EXTRA_LAZY', cascade: ['persist'])]
     #[OrderBy(['position' => 'ASC'])]
     private $children;
@@ -80,7 +85,7 @@ public function addChild(int $position): void
         $this->children[] = new GH7767ChildEntity($this, $position);
     }
 
-    /** @psalm-return Collection<int, GH7767ChildEntity> */
+    /** @psalm-return Collection<int, GH7767ChildEntity>&Selectable<int, GH7767ChildEntity> */
     public function getChildren(): Collection
     {
         return $this->children;