Make phpdoc accurate

When transforming these phpdoc types into native types, things break down. They are correct according to the EBNF, but in practice, there are so-called phase 2 optimizations that allow using ConditionalPrimary, ConditionalFactor and ConditionalTerm instances in places where ConditionalExpression is used.
doctrine · Oct 21, 2023 · 293299a · 293299a
1 parent 42af7ca
commit 293299a
Show file tree

Hide file tree

Showing 12 changed files with 41 additions and 60 deletions.
diff --git a/lib/Doctrine/ORM/Query/AST/ConditionalFactor.php b/lib/Doctrine/ORM/Query/AST/ConditionalFactor.php
@@ -9,7 +9,7 @@
  *
  * @link    www.doctrine-project.org
  */
-class ConditionalFactor extends Node
+class ConditionalFactor extends Node implements Phase2OptimizableConditional
 {
     /** @var bool */
     public $not = false;

diff --git a/lib/Doctrine/ORM/Query/AST/ConditionalPrimary.php b/lib/Doctrine/ORM/Query/AST/ConditionalPrimary.php
@@ -9,12 +9,12 @@
  *
  * @link    www.doctrine-project.org
  */
-class ConditionalPrimary extends Node
+class ConditionalPrimary extends Node implements Phase2OptimizableConditional
 {
     /** @var Node|null */
     public $simpleConditionalExpression;
 
-    /** @var ConditionalExpression|null */
+    /** @var ConditionalExpression|Phase2OptimizableConditional|null */
     public $conditionalExpression;
 
     /** @return bool */

diff --git a/lib/Doctrine/ORM/Query/AST/ConditionalTerm.php b/lib/Doctrine/ORM/Query/AST/ConditionalTerm.php
@@ -9,7 +9,7 @@
  *
  * @link    www.doctrine-project.org
  */
-class ConditionalTerm extends Node
+class ConditionalTerm extends Node implements Phase2OptimizableConditional
 {
     /** @var mixed[] */
     public $conditionalFactors = [];

diff --git a/lib/Doctrine/ORM/Query/AST/HavingClause.php b/lib/Doctrine/ORM/Query/AST/HavingClause.php
@@ -6,10 +6,10 @@
 
 class HavingClause extends Node
 {
-    /** @var ConditionalExpression */
+    /** @var ConditionalExpression|Phase2OptimizableConditional */
     public $conditionalExpression;
 
-    /** @param ConditionalExpression $conditionalExpression */
+    /** @param ConditionalExpression|Phase2OptimizableConditional $conditionalExpression */
     public function __construct($conditionalExpression)
     {
         $this->conditionalExpression = $conditionalExpression;

diff --git a/lib/Doctrine/ORM/Query/AST/Join.php b/lib/Doctrine/ORM/Query/AST/Join.php
@@ -25,7 +25,7 @@ class Join extends Node
     /** @var Node|null */
     public $joinAssociationDeclaration = null;
 
-    /** @var ConditionalExpression|null */
+    /** @var ConditionalExpression|Phase2OptimizableConditional|null */
     public $conditionalExpression = null;
 
     /**

diff --git a/lib/Doctrine/ORM/Query/AST/Phase2OptimizableConditional.php b/lib/Doctrine/ORM/Query/AST/Phase2OptimizableConditional.php
@@ -0,0 +1,17 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\ORM\Query\AST;
+
+/**
+ * Marks types that can be used in place of a ConditionalExpression as a phase
+ * 2 optimization.
+ *
+ * @internal
+ *
+ * @psalm-inheritors ConditionalPrimary|ConditionalFactor|ConditionalTerm
+ */
+interface Phase2OptimizableConditional
+{
+}
diff --git a/lib/Doctrine/ORM/Query/AST/WhenClause.php b/lib/Doctrine/ORM/Query/AST/WhenClause.php
@@ -11,15 +11,15 @@
  */
 class WhenClause extends Node
 {
-    /** @var ConditionalExpression */
+    /** @var ConditionalExpression|Phase2OptimizableConditional */
     public $caseConditionExpression;
 
     /** @var mixed */
     public $thenScalarExpression = null;
 
     /**
-     * @param ConditionalExpression $caseConditionExpression
-     * @param mixed                 $thenScalarExpression
+     * @param ConditionalExpression|Phase2OptimizableConditional $caseConditionExpression
+     * @param mixed                                              $thenScalarExpression
      */
     public function __construct($caseConditionExpression, $thenScalarExpression)
     {

diff --git a/lib/Doctrine/ORM/Query/AST/WhereClause.php b/lib/Doctrine/ORM/Query/AST/WhereClause.php
@@ -11,10 +11,10 @@
  */
 class WhereClause extends Node
 {
-    /** @var ConditionalExpression|ConditionalTerm */
+    /** @var ConditionalExpression|Phase2OptimizableConditional */
     public $conditionalExpression;
 
-    /** @param ConditionalExpression $conditionalExpression */
+    /** @param ConditionalExpression|Phase2OptimizableConditional $conditionalExpression */
     public function __construct($conditionalExpression)
     {
         $this->conditionalExpression = $conditionalExpression;

diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php
@@ -1010,9 +1010,9 @@ private function generateRangeVariableDeclarationSQL(
     /**
      * Walks down a JoinAssociationDeclaration AST node, thereby generating the appropriate SQL.
      *
-     * @param AST\JoinAssociationDeclaration $joinAssociationDeclaration
-     * @param int                            $joinType
-     * @param AST\ConditionalExpression      $condExpr
+     * @param AST\JoinAssociationDeclaration                             $joinAssociationDeclaration
+     * @param int                                                        $joinType
+     * @param AST\ConditionalExpression|AST\Phase2OptimizableConditional $condExpr
      * @psalm-param AST\Join::JOIN_TYPE_* $joinType
      *
      * @return string
@@ -2048,7 +2048,7 @@ public function walkWhereClause($whereClause)
     /**
      * Walk down a ConditionalExpression AST node, thereby generating the appropriate SQL.
      *
-     * @param AST\ConditionalExpression $condExpr
+     * @param AST\ConditionalExpression|AST\Phase2OptimizableConditional $condExpr
      *
      * @return string
      *
@@ -2068,7 +2068,7 @@ public function walkConditionalExpression($condExpr)
     /**
      * Walks down a ConditionalTerm AST node, thereby generating the appropriate SQL.
      *
-     * @param AST\ConditionalTerm $condTerm
+     * @param AST\ConditionalTerm|AST\ConditionalFactor|AST\ConditionalPrimary $condTerm
      *
      * @return string
      *
@@ -2088,7 +2088,7 @@ public function walkConditionalTerm($condTerm)
     /**
      * Walks down a ConditionalFactor AST node, thereby generating the appropriate SQL.
      *
-     * @param AST\ConditionalFactor $factor
+     * @param AST\ConditionalFactor|AST\ConditionalPrimary $factor
      *
      * @return string The SQL.
      *

diff --git a/lib/Doctrine/ORM/Tools/Pagination/WhereInWalker.php b/lib/Doctrine/ORM/Tools/Pagination/WhereInWalker.php
@@ -6,7 +6,6 @@
 
 use Doctrine\ORM\Query\AST\ArithmeticExpression;
 use Doctrine\ORM\Query\AST\ConditionalExpression;
-use Doctrine\ORM\Query\AST\ConditionalFactor;
 use Doctrine\ORM\Query\AST\ConditionalPrimary;
 use Doctrine\ORM\Query\AST\ConditionalTerm;
 use Doctrine\ORM\Query\AST\InListExpression;
@@ -96,10 +95,7 @@ public function walkSelectStatement(SelectStatement $AST)
                         ),
                     ]
                 );
-            } elseif (
-                $AST->whereClause->conditionalExpression instanceof ConditionalExpression
-                || $AST->whereClause->conditionalExpression instanceof ConditionalFactor
-            ) {
+            } else {
                 $tmpPrimary                              = new ConditionalPrimary();
                 $tmpPrimary->conditionalExpression       = $AST->whereClause->conditionalExpression;
                 $AST->whereClause->conditionalExpression = new ConditionalTerm(

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -440,6 +440,11 @@ parameters:
 			count: 1
 			path: lib/Doctrine/ORM/Query/SqlWalker.php
 
+		-
+			message: "#^Parameter \\#1 \\$condTerm of method Doctrine\\\\ORM\\\\Query\\\\SqlWalker\\:\\:walkConditionalTerm\\(\\) expects Doctrine\\\\ORM\\\\Query\\\\AST\\\\ConditionalFactor\\|Doctrine\\\\ORM\\\\Query\\\\AST\\\\ConditionalPrimary\\|Doctrine\\\\ORM\\\\Query\\\\AST\\\\ConditionalTerm, Doctrine\\\\ORM\\\\Query\\\\AST\\\\Phase2OptimizableConditional given\\.$#"
+			count: 1
+			path: lib/Doctrine/ORM/Query/SqlWalker.php
+
 		-
 			message: "#^Result of && is always false\\.$#"
 			count: 1
@@ -595,16 +600,6 @@ parameters:
 			count: 1
 			path: lib/Doctrine/ORM/Tools/Export/Driver/YamlExporter.php
 
-		-
-			message: "#^Instanceof between \\*NEVER\\* and Doctrine\\\\ORM\\\\Query\\\\AST\\\\ConditionalFactor will always evaluate to false\\.$#"
-			count: 1
-			path: lib/Doctrine/ORM/Tools/Pagination/WhereInWalker.php
-
-		-
-			message: "#^Instanceof between Doctrine\\\\ORM\\\\Query\\\\AST\\\\ConditionalExpression and Doctrine\\\\ORM\\\\Query\\\\AST\\\\ConditionalPrimary will always evaluate to false\\.$#"
-			count: 1
-			path: lib/Doctrine/ORM/Tools/Pagination/WhereInWalker.php
-
 		-
 			message: "#^Else branch is unreachable because ternary operator condition is always true\\.$#"
 			count: 1

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -2022,18 +2022,13 @@
     </PossiblyFalseArgument>
     <PossiblyInvalidArgument>
       <code>$AST</code>
-      <code>$conditionalExpression</code>
       <code>$expr</code>
       <code>$pathExp</code>
-      <code><![CDATA[$this->ConditionalExpression()]]></code>
-      <code><![CDATA[$this->ConditionalExpression()]]></code>
       <code><![CDATA[$this->lexer->getLiteral($token)]]></code>
       <code><![CDATA[$this->lexer->getLiteral($token)]]></code>
       <code><![CDATA[$this->lexer->getLiteral($token)]]></code>
     </PossiblyInvalidArgument>
     <PossiblyInvalidPropertyAssignmentValue>
-      <code><![CDATA[$this->ConditionalExpression()]]></code>
-      <code><![CDATA[$this->ConditionalExpression()]]></code>
       <code><![CDATA[$this->SimpleArithmeticExpression()]]></code>
     </PossiblyInvalidPropertyAssignmentValue>
     <PossiblyNullArgument>
@@ -2145,11 +2140,6 @@
     <ImplicitToStringCast>
       <code>$expr</code>
     </ImplicitToStringCast>
-    <InvalidArgument>
-      <code>$condExpr</code>
-      <code>$condTerm</code>
-      <code>$factor</code>
-    </InvalidArgument>
     <InvalidNullableReturnType>
       <code>string</code>
     </InvalidNullableReturnType>
@@ -2158,7 +2148,6 @@
     </MoreSpecificImplementedParamType>
     <PossiblyInvalidArgument>
       <code><![CDATA[$aggExpression->pathExpression]]></code>
-      <code><![CDATA[$whereClause->conditionalExpression]]></code>
     </PossiblyInvalidArgument>
     <PossiblyNullArgument>
       <code><![CDATA[$AST->whereClause]]></code>
@@ -2194,7 +2183,6 @@
     </PossiblyUndefinedArrayOffset>
     <RedundantConditionGivenDocblockType>
       <code>$whereClause !== null</code>
-      <code><![CDATA[($factor->not ? 'NOT ' : '') . $this->walkConditionalPrimary($factor->conditionalPrimary)]]></code>
     </RedundantConditionGivenDocblockType>
   </file>
   <file src="lib/Doctrine/ORM/Query/TreeWalkerAdapter.php">
@@ -2633,21 +2621,6 @@
       <code>$orderByClause</code>
     </PropertyNotSetInConstructor>
   </file>
-  <file src="lib/Doctrine/ORM/Tools/Pagination/WhereInWalker.php">
-    <DocblockTypeContradiction>
-      <code><![CDATA[$AST->whereClause->conditionalExpression instanceof ConditionalExpression
-                || $AST->whereClause->conditionalExpression instanceof ConditionalFactor]]></code>
-      <code><![CDATA[$AST->whereClause->conditionalExpression instanceof ConditionalFactor]]></code>
-      <code><![CDATA[$AST->whereClause->conditionalExpression instanceof ConditionalPrimary]]></code>
-    </DocblockTypeContradiction>
-    <PossiblyInvalidPropertyAssignmentValue>
-      <code><![CDATA[$AST->whereClause->conditionalExpression]]></code>
-    </PossiblyInvalidPropertyAssignmentValue>
-    <RedundantConditionGivenDocblockType>
-      <code><![CDATA[$AST->whereClause->conditionalExpression instanceof ConditionalExpression
-                || $AST->whereClause->conditionalExpression instanceof ConditionalFactor]]></code>
-    </RedundantConditionGivenDocblockType>
-  </file>
   <file src="lib/Doctrine/ORM/Tools/SchemaTool.php">
     <ArgumentTypeCoercion>
       <code>$classes</code>