Resolve QueryBuilder deprecations (#9953)

doctrine · Aug 4, 2022 · 23f5c01 · 23f5c01
1 parent 1a82e4a
commit 23f5c01
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 144 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,5 +1,39 @@
 # Upgrade to 3.0
 
+## BC BREAK: Removed `QueryBuilder` methods and constants.
+
+The following `QueryBuilder` constants and methods have been removed:
+
+1. `SELECT`,
+2. `DELETE`,
+3. `UPDATE`,
+4. `STATE_DIRTY`,
+5. `STATE_CLEAN`,
+6. `getState()`,
+7. `getType()`.
+
+## BC BREAK: Omitting only the alias argument for `QueryBuilder::update` and `QueryBuilder::delete` is not supported anymore
+
+When building an UPDATE or DELETE query and when passing a class/type to the function, the alias argument must not be omitted.
+
+### Before
+
+```php
+$qb = $em->createQueryBuilder()
+    ->delete('User u')
+    ->where('u.id = :user_id')
+    ->setParameter('user_id', 1);
+```
+
+### After
+
+```php
+$qb = $em->createQueryBuilder()
+    ->delete('User', 'u')
+    ->where('u.id = :user_id')
+    ->setParameter('user_id', 1);
+```
+
 ## BC BREAK: Split output walkers and tree walkers
 
 `SqlWalker` and its child classes don't implement the `TreeWalker` interface

diff --git a/lib/Doctrine/ORM/Internal/QueryType.php b/lib/Doctrine/ORM/Internal/QueryType.php
@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\ORM\Internal;
+
+enum QueryType
+{
+    case Select;
+    case Delete;
+    case Update;
+}
diff --git a/lib/Doctrine/ORM/QueryBuilder.php b/lib/Doctrine/ORM/QueryBuilder.php
@@ -6,7 +6,7 @@
 
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\Common\Collections\Criteria;
-use Doctrine\Deprecations\Deprecation;
+use Doctrine\ORM\Internal\QueryType;
 use Doctrine\ORM\Query\Expr;
 use Doctrine\ORM\Query\Parameter;
 use Doctrine\ORM\Query\QueryExpressionVisitor;
@@ -38,21 +38,6 @@
  */
 class QueryBuilder
 {
-    /** @deprecated */
-    public const SELECT = 0;
-
-    /** @deprecated */
-    public const DELETE = 1;
-
-    /** @deprecated */
-    public const UPDATE = 2;
-
-    /** @deprecated */
-    public const STATE_DIRTY = 0;
-
-    /** @deprecated */
-    public const STATE_CLEAN = 1;
-
     /**
      * The array of DQL parts collected.
      *
@@ -70,19 +55,7 @@ class QueryBuilder
         'orderBy' => [],
     ];
 
-    /**
-     * The type of query this is. Can be select, update or delete.
-     *
-     * @psalm-var self::SELECT|self::DELETE|self::UPDATE
-     */
-    private int $type = self::SELECT;
-
-    /**
-     * The state of the query object. Can be dirty or clean.
-     *
-     * @psalm-var self::STATE_*
-     */
-    private int $state = self::STATE_CLEAN;
+    private QueryType $type = QueryType::Select;
 
     /**
      * The complete DQL string for this query.
@@ -240,25 +213,6 @@ public function setCacheMode(int $cacheMode): static
         return $this;
     }
 
-    /**
-     * Gets the type of the currently built query.
-     *
-     * @deprecated If necessary, track the type of the query being built outside of the builder.
-     *
-     * @psalm-return self::SELECT|self::DELETE|self::UPDATE
-     */
-    public function getType(): int
-    {
-        Deprecation::trigger(
-            'doctrine/dbal',
-            'https://github.com/doctrine/orm/pull/9945',
-            'Relying on the type of the query being built is deprecated.'
-            . ' If necessary, track the type of the query being built outside of the builder.'
-        );
-
-        return $this->type;
-    }
-
     /**
      * Gets the associated EntityManager for this query builder.
      */
@@ -267,25 +221,6 @@ public function getEntityManager(): EntityManagerInterface
         return $this->em;
     }
 
-    /**
-     * Gets the state of this query builder instance.
-     *
-     * @deprecated The builder state is an internal concern.
-     *
-     * @return int Either QueryBuilder::STATE_DIRTY or QueryBuilder::STATE_CLEAN.
-     * @psalm-return self::STATE_*
-     */
-    public function getState(): int
-    {
-        Deprecation::trigger(
-            'doctrine/dbal',
-            'https://github.com/doctrine/orm/pull/9945',
-            'Relying on the query builder state is deprecated as it is an internal concern.'
-        );
-
-        return $this->state;
-    }
-
     /**
      * Gets the complete DQL string formed by the current specifications of this QueryBuilder.
      *
@@ -298,20 +233,11 @@ public function getState(): int
      */
     public function getDQL(): string
     {
-        if ($this->dql !== null && $this->state === self::STATE_CLEAN) {
-            return $this->dql;
-        }
-
-        $dql = match ($this->type) {
-            self::DELETE => $this->getDQLForDelete(),
-            self::UPDATE => $this->getDQLForUpdate(),
-            self::SELECT => $this->getDQLForSelect(),
+        return $this->dql ??= match ($this->type) {
+            QueryType::Select => $this->getDQLForSelect(),
+            QueryType::Delete => $this->getDQLForDelete(),
+            QueryType::Update => $this->getDQLForUpdate(),
         };
-
-        $this->state = self::STATE_CLEAN;
-        $this->dql   = $dql;
-
-        return $dql;
     }
 
     /**
@@ -670,7 +596,7 @@ public function add(string $dqlPartName, string|object|array $dqlPart, bool $app
             $this->dqlParts[$dqlPartName] = $isMultiple ? [$dqlPart] : $dqlPart;
         }
 
-        $this->state = self::STATE_DIRTY;
+        $this->dql = null;
 
         return $this;
     }
@@ -690,7 +616,7 @@ public function add(string $dqlPartName, string|object|array $dqlPart, bool $app
      */
     public function select(mixed ...$select): static
     {
-        $this->type = self::SELECT;
+        $this->type = QueryType::Select;
 
         if ($select === []) {
             return $this;
@@ -733,7 +659,7 @@ public function distinct(bool $flag = true): static
      */
     public function addSelect(mixed ...$select): static
     {
-        $this->type = self::SELECT;
+        $this->type = QueryType::Select;
 
         if ($select === []) {
             return $this;
@@ -760,18 +686,18 @@ public function addSelect(mixed ...$select): static
      */
     public function delete(?string $delete = null, ?string $alias = null): static
     {
-        $this->type = self::DELETE;
+        $this->type = QueryType::Delete;
 
         if (! $delete) {
             return $this;
         }
 
         if (! $alias) {
-            Deprecation::trigger(
-                'doctrine/orm',
-                'https://github.com/doctrine/orm/issues/9733',
-                'Omitting the alias is deprecated and will throw an exception in Doctrine 3.0.'
-            );
+            throw new InvalidArgumentException(sprintf(
+                '%s(): The alias for entity %s must not be omitted.',
+                __METHOD__,
+                $delete
+            ));
         }
 
         return $this->add('from', new Expr\From($delete, $alias));
@@ -795,18 +721,18 @@ public function delete(?string $delete = null, ?string $alias = null): static
      */
     public function update(?string $update = null, ?string $alias = null): static
     {
-        $this->type = self::UPDATE;
+        $this->type = QueryType::Update;
 
         if (! $update) {
             return $this;
         }
 
         if (! $alias) {
-            Deprecation::trigger(
-                'doctrine/orm',
-                'https://github.com/doctrine/orm/issues/9733',
-                'Omitting the alias is deprecated and will throw an exception in Doctrine 3.0.'
-            );
+            throw new InvalidArgumentException(sprintf(
+                '%s(): The alias for entity %s must not be omitted.',
+                __METHOD__,
+                $update
+            ));
         }
 
         return $this->add('from', new Expr\From($update, $alias));
@@ -1386,7 +1312,7 @@ public function resetDQLParts(?array $parts = null): static
     public function resetDQLPart(string $part): static
     {
         $this->dqlParts[$part] = is_array($this->dqlParts[$part]) ? [] : null;
-        $this->state           = self::STATE_DIRTY;
+        $this->dql             = null;
 
         return $this;
     }

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -2062,10 +2062,6 @@
     <PossiblyInvalidIterator occurrences="1">
       <code>$dqlPart</code>
     </PossiblyInvalidIterator>
-    <PossiblyNullArgument occurrences="2">
-      <code>$alias</code>
-      <code>$alias</code>
-    </PossiblyNullArgument>
   </file>
   <file src="lib/Doctrine/ORM/Repository/DefaultRepositoryFactory.php">
     <InvalidReturnStatement occurrences="1">

diff --git a/psalm.xml b/psalm.xml
@@ -30,7 +30,6 @@
         <DeprecatedConstant>
             <errorLevel type="suppress">
                 <file name="lib/Doctrine/ORM/Mapping/Driver/DatabaseDriver.php" />
-                <file name="lib/Doctrine/ORM/QueryBuilder.php"/>
             </errorLevel>
         </DeprecatedConstant>
         <DeprecatedMethod>

diff --git a/tests/Doctrine/Tests/ORM/QueryBuilderTest.php b/tests/Doctrine/Tests/ORM/QueryBuilderTest.php
@@ -6,7 +6,6 @@
 
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\Common\Collections\Criteria;
-use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
 use Doctrine\ORM\Cache;
 use Doctrine\ORM\Query;
 use Doctrine\ORM\Query\Expr\Join;
@@ -29,8 +28,6 @@
  */
 class QueryBuilderTest extends OrmTestCase
 {
-    use VerifyDeprecations;
-
     private EntityManagerMock $entityManager;
 
     protected function setUp(): void
@@ -52,16 +49,7 @@ public function testSelectSetsType(): void
             ->delete(CmsUser::class, 'u')
             ->select('u.id', 'u.username');
 
-        self::assertEquals($qb->getType(), QueryBuilder::SELECT);
-    }
-
-    public function testEmptySelectSetsType(): void
-    {
-        $qb = $this->entityManager->createQueryBuilder()
-            ->delete(CmsUser::class, 'u')
-            ->select();
-
-        self::assertEquals($qb->getType(), QueryBuilder::SELECT);
+        $this->assertValidQueryBuilder($qb, 'SELECT u.id, u.username FROM Doctrine\Tests\Models\CMS\CmsUser u');
     }
 
     public function testDeleteSetsType(): void
@@ -70,7 +58,7 @@ public function testDeleteSetsType(): void
             ->from(CmsUser::class, 'u')
             ->delete();
 
-        self::assertEquals($qb->getType(), QueryBuilder::DELETE);
+        $this->assertValidQueryBuilder($qb, 'DELETE Doctrine\Tests\Models\CMS\CmsUser u');
     }
 
     public function testUpdateSetsType(): void
@@ -79,7 +67,7 @@ public function testUpdateSetsType(): void
             ->from(CmsUser::class, 'u')
             ->update();
 
-        self::assertEquals($qb->getType(), QueryBuilder::UPDATE);
+        $this->assertValidQueryBuilder($qb, 'UPDATE Doctrine\Tests\Models\CMS\CmsUser u');
     }
 
     public function testSimpleSelect(): void
@@ -828,21 +816,6 @@ public function testGetEntityManager(): void
         self::assertEquals($this->entityManager, $qb->getEntityManager());
     }
 
-    public function testInitialStateIsClean(): void
-    {
-        $qb = $this->entityManager->createQueryBuilder();
-        self::assertEquals(QueryBuilder::STATE_CLEAN, $qb->getState());
-    }
-
-    public function testAlteringQueryChangesStateToDirty(): void
-    {
-        $qb = $this->entityManager->createQueryBuilder()
-            ->select('u')
-            ->from(CmsUser::class, 'u');
-
-        self::assertEquals(QueryBuilder::STATE_DIRTY, $qb->getState());
-    }
-
     public function testSelectWithFuncExpression(): void
     {
         $qb   = $this->entityManager->createQueryBuilder();
@@ -1281,24 +1254,21 @@ public function testJoin(): void
         self::assertSame('SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u LEFT JOIN Doctrine\Tests\Models\CMS\CmsArticle a0 INNER JOIN Doctrine\Tests\Models\CMS\CmsArticle a1', $builder->getDQL());
     }
 
-    public function testUpdateDeprecationMessage(): void
+    public function testUpdateWithoutAlias(): void
     {
-        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/9733');
-
-        $qb = $this->entityManager->createQueryBuilder()
-            ->update(CmsUser::class . ' u')
-            ->set('u.username', ':username');
+        $qb = $this->entityManager->createQueryBuilder();
 
-        $this->assertValidQueryBuilder($qb, 'UPDATE Doctrine\Tests\Models\CMS\CmsUser u  SET u.username = :username');
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('Doctrine\ORM\QueryBuilder::update(): The alias for entity Doctrine\Tests\Models\CMS\CmsUser u must not be omitted.');
+        $qb->update(CmsUser::class . ' u');
     }
 
-    public function testDeleteDeprecationMessage(): void
+    public function testDeleteWithoutAlias(): void
     {
-        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/9733');
-
-        $qb = $this->entityManager->createQueryBuilder()
-            ->delete(CmsUser::class . ' u');
+        $qb = $this->entityManager->createQueryBuilder();
 
-        $this->assertValidQueryBuilder($qb, 'DELETE Doctrine\Tests\Models\CMS\CmsUser u ');
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('Doctrine\ORM\QueryBuilder::delete(): The alias for entity Doctrine\Tests\Models\CMS\CmsUser u must not be omitted.');
+        $qb->delete(CmsUser::class . ' u');
     }
 }