Remove shameful hack in LimitSubqueryOutputWalker - replace with significantly less shameful hack

lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php

@@ -15,7 +15,9 @@ namespace Doctrine\ORM\Tools\Pagination;
 
 use Doctrine\ORM\Query\AST\OrderByClause;
 use Doctrine\ORM\Query\AST\PartialObjectExpression;
+use Doctrine\ORM\Query\AST\PathExpression;
 use Doctrine\ORM\Query\AST\SelectExpression;
+use Doctrine\ORM\Query\Expr\OrderBy;
 use Doctrine\ORM\Query\SqlWalker;
 use Doctrine\ORM\Query\AST\SelectStatement;
 
@@ -62,6 +64,11 @@ class LimitSubqueryOutputWalker extends SqlWalker
      */
     private $em;
 
+    /**
+     * @var array
+     */
+    private $orderByPathExpressions = [];
+
     /**
      * The quote strategy.
      *
@@ -101,17 +108,21 @@ class LimitSubqueryOutputWalker extends SqlWalker
      * Walks down a SelectStatement AST node, wrapping it in a SELECT DISTINCT.
      *
      * @param SelectStatement $AST
+     * @param bool $addMissingItemsFromOrderByToSelect
      *
      * @return string
      *
      * @throws \RuntimeException
      */
-    public function walkSelectStatement(SelectStatement $AST)
+    public function walkSelectStatement(SelectStatement $AST, $addMissingItemsFromOrderByToSelect = true)
     {
-        // In the case of ordering a query by columns from joined tables, we
+        // We don't want to call this recursively!
-        // must add those columns to the select clause of the query BEFORE
+        if ($AST->orderByClause instanceof OrderByClause && $addMissingItemsFromOrderByToSelect) {
-        // the SQL is generated.
+            // In the case of ordering a query by columns from joined tables, we
-        $this->addMissingItemsFromOrderByToSelect($AST);
+            // must add those columns to the select clause of the query BEFORE
+            // the SQL is generated.
+            $this->addMissingItemsFromOrderByToSelect($AST);
+        }
 
         // Remove order by clause from the inner query
         // It will be re-appended in the outer select generated by this method
@@ -218,24 +229,32 @@ class LimitSubqueryOutputWalker extends SqlWalker
      */
     private function addMissingItemsFromOrderByToSelect(SelectStatement $AST)
     {
-        // This block dumps the order by clause node using Node::dump().
+        $this->orderByPathExpressions = [];
-        // It then finds all PathExpressions within and captures the
+
-        // identificationVariable and field name of each.
+        // We need to do this in another walker because otherwise we'll end up
-        $orderByDump = (string)$AST->orderByClause;
+        // polluting the state of this one.
+        $walker = clone $this;
+
+        // This will populate $orderByPathExpressions via
+        // LimitSubqueryOutputWalker::walkPathExpression, which will be called
+        // as the select statement is walked. We'll end up with an array of all
+        // path expressions referenced in the query.
+        $walker->walkSelectStatement($AST, false);
+        $orderByPathExpressions = $walker->getOrderByPathExpressions();
+
+        // Get a map of referenced identifiers to field names.
         $selects = [];
-        if (preg_match_all('/PathExpression\([^\)]+"identificationVariable": \'([^\']*)\'[^\)]+"field": \'([^\']*)\'[^\)]+\)/i', $orderByDump, $matches, PREG_SET_ORDER)) {
+        foreach ($orderByPathExpressions as $pathExpression) {
-            foreach($matches as $match) {
+            $idVar = $pathExpression->identificationVariable;
-                $idVar = $match[1];
+            $field = $pathExpression->field;
-                $field = $match[2];
+            if (!isset($selects[$idVar])) {
-                if (!isset($selects[$idVar])) {
+                $selects[$idVar] = [];
-                    $selects[$idVar] = [];
-                }
-                $selects[$idVar][$field] = true;
             }
+            $selects[$idVar][$field] = true;
         }
 
         // Loop the select clause of the AST and exclude items from $select
-        // that are already being selected.
+        // that are already being selected in the query.
         foreach ($AST->selectClause->selectExpressions as $selectExpression) {
             if ($selectExpression instanceof SelectExpression) {
                 $idVar = $selectExpression->expression;
@@ -348,4 +367,26 @@ class LimitSubqueryOutputWalker extends SqlWalker
 
         return array($selectListAdditions, $orderByItems);
     }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function walkPathExpression($pathExpr)
+    {
+        if (!in_array($pathExpr, $this->orderByPathExpressions)) {
+            $this->orderByPathExpressions[] = $pathExpr;
+        }
+
+        return parent::walkPathExpression($pathExpr);
+    }
+
+    /**
+     * getter for $orderByPathExpressions
+     *
+     * @return array
+     */
+    public function getOrderByPathExpressions()
+    {
+        return $this->orderByPathExpressions;
+    }
 }