Merge pull request doctrine#6193 from derrabus/improvement/query-buil…

…der-reset

Add deprecation layer and upgrade path for QueryBuilder::reset*() methods

UPGRADE.md

@@ -8,6 +8,23 @@ awareness about deprecated code.
 
 # Upgrade to 3.8
 
+## Deprecated reset methods from `QueryBuilder`
+
+`QueryBuilder::resetQueryParts()` has been deprecated.
+
+Resetting individual query parts through the generic `resetQueryPart()` method has been deprecated as well.
+However, several replacements have been put in place depending on the `$queryPartName` parameter:
+
+| `$queryPartName` | suggested replacement                      |
+|------------------|--------------------------------------------|
+| `'select'`       | Call `select()` with a new set of columns. |
+| `'distinct'`     | `distinct(false)`                          |
+| `'where'`        | `resetWhere()`                             |
+| `'groupBy'`      | `resetGroupBy()`                           |
+| `'having'`       | `resetHaving()`                            |
+| `'orderBy'`      | `resetOrderBy()`                           |
+| `'values'`       | Call `values()` with a new set of values.  |
+
 ## Deprecated getting query parts from `QueryBuilder`
 
 The usage of `QueryBuilder::getQueryPart()` and `::getQueryParts()` is deprecated. The query parts

phpstan.neon.dist

@@ -112,6 +112,12 @@ parameters:
             paths:
                 - src/Platforms/AbstractPlatform.php
 
+        # Deprecated method, will be removed in 4.0.0
+        -
+            message: '~^Variable method call on \$this\(Doctrine\\DBAL\\Query\\QueryBuilder\)\.$~'
+            paths:
+                - src/Query/QueryBuilder.php
+
         # There is no way to make this assertion in the code,
         # and the API doesn't support parametrization of returned column types.
         -

psalm.xml.dist

@@ -504,6 +504,8 @@
                 -->
                 <referencedMethod name="Doctrine\DBAL\Query\QueryBuilder::getQueryPart"/>
                 <referencedMethod name="Doctrine\DBAL\Query\QueryBuilder::getQueryParts"/>
+                <referencedMethod name="Doctrine\DBAL\Query\QueryBuilder::resetQueryPart"/>
+                <referencedMethod name="Doctrine\DBAL\Query\QueryBuilder::resetQueryParts"/>
 
                 <!-- TODO for PHPUnit 10 -->
                 <referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::withConsecutive"/>
@@ -658,6 +660,12 @@
                 <file name="src/Schema/PostgreSQLSchemaManager.php"/>
             </errorLevel>
         </NullableReturnStatement>
+        <ParadoxicalCondition>
+            <errorLevel type="suppress">
+                <!-- False positive in deprecation layer. Can be removed in 4.0.x -->
+                <file name="src/Query/QueryBuilder.php" />
+            </errorLevel>
+        </ParadoxicalCondition>
         <PossiblyInvalidArgument>
             <errorLevel type="suppress">
                 <!-- PgSql objects are represented as resources in PHP 7.4 -->

src/Query/QueryBuilder.php

@@ -17,14 +17,17 @@
 use function array_keys;
 use function array_unshift;
 use function count;
+use function func_get_arg;
 use function func_get_args;
 use function func_num_args;
 use function implode;
 use function is_array;
 use function is_object;
 use function key;
+use function method_exists;
 use function strtoupper;
 use function substr;
+use function ucfirst;
 
 /**
  * QueryBuilder class is responsible to dynamically create SQL queries.
@@ -35,6 +38,8 @@
  * The query builder does no validation whatsoever if certain features even work with the
  * underlying database vendor. Limit queries and joins are NOT applied to UPDATE and DELETE statements
  * even if some vendors such as MySQL support it.
+ *
+ * @method $this distinct(bool $distinct = true) Adds or removes DISTINCT to/from the query.
  */
 class QueryBuilder
 {
@@ -677,7 +682,7 @@ public function select($select = null/*, string ...$selects*/)
     }
 
     /**
-     * Adds DISTINCT to the query.
+     * Adds or removes DISTINCT to/from the query.
      *
      * <code>
      *     $qb = $conn->createQueryBuilder()
@@ -688,9 +693,10 @@ public function select($select = null/*, string ...$selects*/)
      *
      * @return $this This QueryBuilder instance.
      */
-    public function distinct(): self
+    public function distinct(/* bool $distinct = true */): self
     {
-        $this->sqlParts['distinct'] = true;
+        $this->sqlParts['distinct'] = func_num_args() < 1 || func_get_arg(0);
+        $this->state                = self::STATE_DIRTY;
 
         return $this;
     }
@@ -1335,37 +1341,126 @@ public function getQueryParts()
     /**
      * Resets SQL parts.
      *
+     * @deprecated Use the dedicated reset*() methods instead.
+     *
      * @param string[]|null $queryPartNames
      *
      * @return $this This QueryBuilder instance.
      */
     public function resetQueryParts($queryPartNames = null)
     {
+        Deprecation::trigger(
+            'doctrine/dbal',
+            'TODO',
+            '%s() is deprecated, instead use dedicated reset methods for the parts that shall be reset.',
+            __METHOD__,
+        );
+
         $queryPartNames ??= array_keys($this->sqlParts);
 
         foreach ($queryPartNames as $queryPartName) {
-            $this->resetQueryPart($queryPartName);
+            $this->sqlParts[$queryPartName] = self::SQL_PARTS_DEFAULTS[$queryPartName];
         }
 
+        $this->state = self::STATE_DIRTY;
+
         return $this;
     }
 
     /**
      * Resets a single SQL part.
      *
+     * @deprecated Use the dedicated reset*() methods instead.
+     *
      * @param string $queryPartName
      *
      * @return $this This QueryBuilder instance.
      */
     public function resetQueryPart($queryPartName)
     {
+        if ($queryPartName === 'distinct') {
+            Deprecation::trigger(
+                'doctrine/dbal',
+                'TODO',
+                'Calling %s() with "distinct" is deprecated, call distinct(false) instead.',
+                __METHOD__,
+            );
+
+            return $this->distinct(false);
+        }
+
+        $newMethodName = 'reset' . ucfirst($queryPartName);
+        if (array_key_exists($queryPartName, self::SQL_PARTS_DEFAULTS) && method_exists($this, $newMethodName)) {
+            Deprecation::trigger(
+                'doctrine/dbal',
+                'TODO',
+                'Calling %s() with "%s" is deprecated, call %s() instead.',
+                __METHOD__,
+                $queryPartName,
+                $newMethodName,
+            );
+
+            return $this->$newMethodName();
+        }
+
+        Deprecation::trigger(
+            'doctrine/dbal',
+            'TODO',
+            'Calling %s() with "%s" is deprecated without replacement.',
+            __METHOD__,
+            $queryPartName,
+            $newMethodName,
+        );
+
         $this->sqlParts[$queryPartName] = self::SQL_PARTS_DEFAULTS[$queryPartName];
 
         $this->state = self::STATE_DIRTY;
 
         return $this;
     }
 
+    /**
+     * Resets the WHERE conditions for the query.
+     *
+     * @return $this This QueryBuilder instance.
+     */
+    public function resetWhere(): self
+    {
+        $this->sqlParts['where'] = self::SQL_PARTS_DEFAULTS['where'];
+
+        $this->state = self::STATE_DIRTY;
+
+        return $this;
+    }
+
+    /**
+     * Resets the grouping for the query.
+     *
+     * @return $this This QueryBuilder instance.
+     */
+    public function resetGroupBy(): self
+    {
+        $this->sqlParts['groupBy'] = self::SQL_PARTS_DEFAULTS['groupBy'];
+
+        $this->state = self::STATE_DIRTY;
+
+        return $this;
+    }
+
+    /**
+     * Resets the HAVING conditions for the query.
+     *
+     * @return $this This QueryBuilder instance.
+     */
+    public function resetHaving(): self
+    {
+        $this->sqlParts['having'] = self::SQL_PARTS_DEFAULTS['having'];
+
+        $this->state = self::STATE_DIRTY;
+
+        return $this;
+    }
+
     /**
      * Resets the ordering for the query.
      *