Merge branch '2.12.x' into 3.0.x

* 2.12.x: Deprecate custom ObjectRepository implementations (doctrine#9533) Fix types on walkLiteral() and walkLikeExpression() (doctrine#9566)
derrabus · Apr 6, 2022 · 953c633 · 953c633
2 parents 476a6ba + d9e8e83
commit 953c633
Show file tree

Hide file tree

Showing 16 changed files with 137 additions and 86 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -251,6 +251,14 @@ Use `toIterable()` instead.
 
 # Upgrade to 2.12
 
+## Deprecate custom repository classes that don't extend `EntityRepository`
+
+Although undocumented, it is currently possible to configure a custom repository
+class that implements `ObjectRepository` but does not extend the
+`EntityRepository` base class.
+
+This is now deprecated. Please extend `EntityRepository` instead.
+
 ## Deprecated more APIs related to entity namespace aliases
 
 ```diff

diff --git a/lib/Doctrine/ORM/Configuration.php b/lib/Doctrine/ORM/Configuration.php
@@ -5,6 +5,7 @@
 namespace Doctrine\ORM;
 
 use Doctrine\Common\Proxy\AbstractProxyFactory;
+use Doctrine\Deprecations\Deprecation;
 use Doctrine\ORM\Cache\CacheConfiguration;
 use Doctrine\ORM\Exception\InvalidEntityRepository;
 use Doctrine\ORM\Internal\Hydration\AbstractHydrator;
@@ -22,8 +23,9 @@
 use Doctrine\Persistence\Mapping\Driver\MappingDriver;
 use Doctrine\Persistence\ObjectRepository;
 use Psr\Cache\CacheItemPoolInterface;
-use ReflectionClass;
 
+use function class_exists;
+use function is_a;
 use function strtolower;
 
 /**
@@ -446,27 +448,36 @@ public function getFilterClassName($name)
      * Sets default repository class.
      *
      * @param string $className
+     * @psalm-param class-string<EntityRepository> $className
      *
      * @return void
      *
      * @throws InvalidEntityRepository If $classname is not an ObjectRepository.
      */
     public function setDefaultRepositoryClassName($className)
     {
-        $reflectionClass = new ReflectionClass($className);
-
-        if (! $reflectionClass->implementsInterface(ObjectRepository::class)) {
+        if (! class_exists($className) || ! is_a($className, ObjectRepository::class, true)) {
             throw InvalidEntityRepository::fromClassName($className);
         }
 
+        if (! is_a($className, EntityRepository::class, true)) {
+            Deprecation::trigger(
+                'doctrine/orm',
+                'https://github.com/doctrine/orm/pull/9533',
+                'Configuring %s as default repository class is deprecated because it does not extend %s.',
+                $className,
+                EntityRepository::class
+            );
+        }
+
         $this->_attributes['defaultRepositoryClassName'] = $className;
     }
 
     /**
      * Get default repository class.
      *
      * @return string
-     * @psalm-return class-string
+     * @psalm-return class-string<EntityRepository>
      */
     public function getDefaultRepositoryClassName()
     {

diff --git a/lib/Doctrine/ORM/Exception/InvalidEntityRepository.php b/lib/Doctrine/ORM/Exception/InvalidEntityRepository.php
@@ -4,15 +4,15 @@
 
 namespace Doctrine\ORM\Exception;
 
-use Doctrine\Persistence\ObjectRepository;
+use Doctrine\ORM\EntityRepository;
 use LogicException;
 
 final class InvalidEntityRepository extends LogicException implements ConfigurationException
 {
     public static function fromClassName(string $className): self
     {
         return new self(
-            "Invalid repository class '" . $className . "'. It must be a " . ObjectRepository::class . '.'
+            "Invalid repository class '" . $className . "'. It must be a " . EntityRepository::class . '.'
         );
     }
 }
diff --git a/lib/Doctrine/ORM/Query/AST/LikeExpression.php b/lib/Doctrine/ORM/Query/AST/LikeExpression.php
@@ -4,6 +4,8 @@
 
 namespace Doctrine\ORM\Query\AST;
 
+use Doctrine\ORM\Query\AST\Functions\FunctionNode;
+
 /**
  * LikeExpression ::= StringExpression ["NOT"] "LIKE" string ["ESCAPE" char]
  *
@@ -12,21 +14,21 @@
 class LikeExpression extends Node
 {
     /** @var bool */
-    public $not;
+    public $not = false;
 
-    /** @var Node */
+    /** @var Node|string */
     public $stringExpression;
 
-    /** @var InputParameter */
+    /** @var InputParameter|FunctionNode|PathExpression|Literal */
     public $stringPattern;
 
     /** @var Literal|null */
     public $escapeChar;
 
     /**
-     * @param Node           $stringExpression
-     * @param InputParameter $stringPattern
-     * @param Literal|null   $escapeChar
+     * @param Node|string                                        $stringExpression
+     * @param InputParameter|FunctionNode|PathExpression|Literal $stringPattern
+     * @param Literal|null                                       $escapeChar
      */
     public function __construct($stringExpression, $stringPattern, $escapeChar = null)
     {

diff --git a/lib/Doctrine/ORM/Query/AST/Literal.php b/lib/Doctrine/ORM/Query/AST/Literal.php
@@ -10,7 +10,10 @@ class Literal extends Node
     public const BOOLEAN = 2;
     public const NUMERIC = 3;
 
-    /** @var int */
+    /**
+     * @var int
+     * @psalm-var self::*
+     */
     public $type;
 
     /** @var mixed */
@@ -19,6 +22,7 @@ class Literal extends Node
     /**
      * @param int   $type
      * @param mixed $value
+     * @psalm-param self::* $type
      */
     public function __construct($type, $value)
     {

diff --git a/lib/Doctrine/ORM/Query/QueryException.php b/lib/Doctrine/ORM/Query/QueryException.php
@@ -7,6 +7,7 @@
 use Doctrine\ORM\Exception\ORMException;
 use Doctrine\ORM\Query\AST\PathExpression;
 use Exception;
+use Stringable;
 
 class QueryException extends Exception implements ORMException
 {
@@ -134,7 +135,7 @@ public static function invalidPathExpression($pathExpr)
     }
 
     /**
-     * @param string $literal
+     * @param string|Stringable $literal
      *
      * @return QueryException
      */

diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php
@@ -17,6 +17,7 @@
 use Doctrine\ORM\Query;
 use Doctrine\ORM\Utility\HierarchyDiscriminatorResolver;
 use Doctrine\ORM\Utility\PersisterHelper;
+use InvalidArgumentException;
 use LogicException;
 
 use function array_diff;
@@ -2161,9 +2162,15 @@ public function walkBetweenExpression($betweenExpr)
     public function walkLikeExpression($likeExpr)
     {
         $stringExpr = $likeExpr->stringExpression;
-        $leftExpr   = is_string($stringExpr) && isset($this->queryComponents[$stringExpr]['resultVariable'])
-            ? $this->walkResultVariable($stringExpr)
-            : $stringExpr->dispatch($this);
+        if (is_string($stringExpr)) {
+            if (! isset($this->queryComponents[$stringExpr]['resultVariable'])) {
+                throw new LogicException(sprintf('No result variable found for string expression "%s".', $stringExpr));
+            }
+
+            $leftExpr = $this->walkResultVariable($stringExpr);
+        } else {
+            $leftExpr = $stringExpr->dispatch($this);
+        }
 
         $sql = $leftExpr . ($likeExpr->not ? ' NOT' : '') . ' LIKE ';
 
@@ -2332,6 +2339,10 @@ public function walkStringPrimary($stringPrimary)
      */
     public function walkResultVariable($resultVariable)
     {
+        if (! isset($this->scalarResultAliasMap[$resultVariable])) {
+            throw new InvalidArgumentException(sprintf('Unknown result variable: %s', $resultVariable));
+        }
+
         $resultAlias = $this->scalarResultAliasMap[$resultVariable];
 
         if (is_array($resultAlias)) {

diff --git a/lib/Doctrine/ORM/Query/TreeWalker.php b/lib/Doctrine/ORM/Query/TreeWalker.php
@@ -335,7 +335,7 @@ public function walkInstanceOfExpression($instanceOfExpr);
     /**
      * Walks down a literal that represents an AST node, thereby generating the appropriate SQL.
      *
-     * @param mixed $literal
+     * @param AST\Literal $literal
      *
      * @return string The SQL.
      */

diff --git a/lib/Doctrine/ORM/Repository/DefaultRepositoryFactory.php b/lib/Doctrine/ORM/Repository/DefaultRepositoryFactory.php
@@ -4,7 +4,9 @@
 
 namespace Doctrine\ORM\Repository;
 
+use Doctrine\Deprecations\Deprecation;
 use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\EntityRepository;
 use Doctrine\Persistence\ObjectRepository;
 
 use function spl_object_id;
@@ -18,6 +20,7 @@ final class DefaultRepositoryFactory implements RepositoryFactory
      * The list of EntityRepository instances.
      *
      * @var ObjectRepository[]
+     * @psalm-var array<string, ObjectRepository>
      */
     private $repositoryList = [];
 
@@ -49,6 +52,17 @@ private function createRepository(
         $repositoryClassName = $metadata->customRepositoryClassName
             ?: $entityManager->getConfiguration()->getDefaultRepositoryClassName();
 
-        return new $repositoryClassName($entityManager, $metadata);
+        $repository = new $repositoryClassName($entityManager, $metadata);
+        if (! $repository instanceof EntityRepository) {
+            Deprecation::trigger(
+                'doctrine/orm',
+                'https://github.com/doctrine/orm/pull/9533',
+                'Configuring %s as repository class is deprecated because it does not extend %s.',
+                $repositoryClassName,
+                EntityRepository::class
+            );
+        }
+
+        return $repository;
     }
 }
diff --git a/lib/Doctrine/ORM/Repository/RepositoryFactory.php b/lib/Doctrine/ORM/Repository/RepositoryFactory.php
@@ -18,7 +18,7 @@ interface RepositoryFactory
      * @param EntityManagerInterface $entityManager The EntityManager instance.
      * @param class-string<T>        $entityName    The name of the entity.
      *
-     * @return ObjectRepository<T>
+     * @return ObjectRepository<T> This type will change to {@see EntityRepository} in 3.0.
      *
      * @template T of object
      */

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -681,7 +681,7 @@ parameters:
 			path: lib/Doctrine/ORM/Query/Parser.php
 
 		-
-			message: "#^Parameter \\#2 \\$stringPattern of class Doctrine\\\\ORM\\\\Query\\\\AST\\\\LikeExpression constructor expects Doctrine\\\\ORM\\\\Query\\\\AST\\\\InputParameter, Doctrine\\\\ORM\\\\Query\\\\AST\\\\Node given\\.$#"
+			message: "#^Parameter \\#2 \\$stringPattern of class Doctrine\\\\ORM\\\\Query\\\\AST\\\\LikeExpression constructor expects Doctrine\\\\ORM\\\\Query\\\\AST\\\\Functions\\\\FunctionNode\\|Doctrine\\\\ORM\\\\Query\\\\AST\\\\InputParameter\\|Doctrine\\\\ORM\\\\Query\\\\AST\\\\Literal\\|Doctrine\\\\ORM\\\\Query\\\\AST\\\\PathExpression, Doctrine\\\\ORM\\\\Query\\\\AST\\\\Node given\\.$#"
 			count: 1
 			path: lib/Doctrine/ORM/Query/Parser.php
 
@@ -692,11 +692,6 @@ parameters:
 
 		-
 			message: "#^Call to function is_string\\(\\) with Doctrine\\\\ORM\\\\Query\\\\AST\\\\Node will always evaluate to false\\.$#"
-			count: 2
-			path: lib/Doctrine/ORM/Query/SqlWalker.php
-
-		-
-			message: "#^Else branch is unreachable because previous condition is always true\\.$#"
 			count: 1
 			path: lib/Doctrine/ORM/Query/SqlWalker.php
 
@@ -705,21 +700,6 @@ parameters:
 			count: 1
 			path: lib/Doctrine/ORM/Query/SqlWalker.php
 
-		-
-			message: "#^Elseif branch is unreachable because previous condition is always true\\.$#"
-			count: 2
-			path: lib/Doctrine/ORM/Query/SqlWalker.php
-
-		-
-			message: "#^Instanceof between \\*NEVER\\* and Doctrine\\\\ORM\\\\Query\\\\AST\\\\Functions\\\\FunctionNode will always evaluate to false\\.$#"
-			count: 1
-			path: lib/Doctrine/ORM/Query/SqlWalker.php
-
-		-
-			message: "#^Instanceof between \\*NEVER\\* and Doctrine\\\\ORM\\\\Query\\\\AST\\\\PathExpression will always evaluate to false\\.$#"
-			count: 1
-			path: lib/Doctrine/ORM/Query/SqlWalker.php
-
 		-
 			message: "#^Method Doctrine\\\\ORM\\\\Query\\\\SqlWalker\\:\\:walkConditionalPrimary\\(\\) should return string but return statement is missing\\.$#"
 			count: 1
@@ -732,7 +712,7 @@ parameters:
 
 		-
 			message: "#^Result of && is always false\\.$#"
-			count: 2
+			count: 1
 			path: lib/Doctrine/ORM/Query/SqlWalker.php
 
 		-
@@ -1309,4 +1289,3 @@ parameters:
 			message: "#^Access to an undefined property Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\:\\:\\$subClasses\\.$#"
 			count: 1
 			path: lib/Doctrine/ORM/Utility/HierarchyDiscriminatorResolver.php
-