Simplifying parameter binding by making it all the same, this also

resolves a potential issue that was made evident on cyclic references
not being able to be traversed

lib/Cake/Model/Datasource/Database/Expression/QueryExpression.php

@@ -428,9 +428,10 @@ public function sql() {
  *
  * Callback function receives as only argument an instance of a QueryExpression
  *
+ * @param callable $callable
  * @return void
  */
-	public function traverse($callable) {
+	public function traverse(callable $callable) {
 		foreach ($this->_conditions as $c) {
 			if ($c instanceof self) {
 				$c->traverse($callable);

lib/Cake/Model/Datasource/Database/Expression/ValuesExpression.php

@@ -153,6 +153,12 @@ public function traverse(callable $visitor) {
 		if (!$this->_hasQuery) {
 			return;
 		}
+
+		foreach ($this->_values as $v) {
+			if ($v instanceof ExpressionInterface) {
+				$v->traverse($visitor);
+			}
+		}
 		$this->_values[0]->traverse($visitor);
 	}
 

lib/Cake/Model/Datasource/Database/ExpressionInterface.php

@@ -29,4 +29,15 @@ interface ExpressionInterface {
  */
 	public function sql();
 
+/**
+ * Iterates over each part of the expression recursively for every
+ * level of the expressions tree and executes the $visitor callable
+ * passing as first parameter the instance of the expression currently
+ * being iterated.
+ *
+ * @param callable $visitor
+ * @return void
+ */
+	public function traverse(callable $visitor);
+
 }

lib/Cake/Model/Datasource/Database/Query.php

@@ -240,10 +240,10 @@ public function sql($transform = true) {
  *	});
  * }}}
  *
- * @param callback $visitor a function or callable to be executed for each part
+ * @param callable $visitor a function or callable to be executed for each part
  * @return Query
  */
-	public function traverse($visitor) {
+	public function traverse(callable $visitor) {
 		$this->{'_traverse' . ucfirst($this->_type)}($visitor);
 		return $this;
 	}
@@ -1449,24 +1449,28 @@ protected function _bindParams($statement) {
 			$statement->bind($params, $types);
 		};
 
-		$binder = function($expression, $name) use ($statement, $visitor, &$binder) {
+		$refs = [];
+		$binder = function($expression, $name = null) use ($statement, $visitor, &$binder, &$refs) {
 			if (is_array($expression)) {
 				foreach ($expression as $e) {
 					$binder($e, $name);
 				}
+				return;
 			}
 
-			if ($expression instanceof self) {
-				return $expression->traverse($binder);
-			}
+			if ($expression instanceof ExpressionInterface) {
+				$id = spl_object_hash($expression);
 
-			if ($expression instanceof QueryExpression) {
-				// Visit all expressions and subexpressions to get every bound value
-				$expression->traverse($visitor);
-			}
-			if ($expression instanceof ValuesExpression) {
+				if (isset($refs[$id])) {
+					return;
+				}
+
+				$refs[$id] = 1;
 				$expression->traverse($binder);
-				$visitor($expression);
+
+				if (!($expression instanceof self)) {
+					$visitor($expression);
+				}
 			}
 		};
 

lib/Cake/Test/TestCase/Model/Datasource/Database/QueryTest.php

@@ -1944,21 +1944,11 @@ public function testInsertFailureMixingTypesQueryFirst() {
 	}
 
 /**
- * Assertion for comparing a table's contents with what is in it.
+ * undocumented function
  *
- * @param string $table
- * @param int $count
- * @param array $rows
+ * @group FunctionExpression
  * @return void
  */
-	protected function assertTable($table, $count, $rows) {
-		$result = (new Query($this->connection))->select('*')
-			->from($table)
-			->execute();
-		$this->assertCount($count, $result, 'Row count is incorrect');
-		$this->assertEquals($rows, $result->fetchAll('assoc'));
-	}
-
 	public function testSQLFunctions() {
 		$this->_insertTwoRecords();
 		$query = new Query($this->connection);
@@ -1974,4 +1964,22 @@ public function testSQLFunctions() {
 			->execute();
 		debug($result->fetchAll());
 	}
+
+
+/**
+ * Assertion for comparing a table's contents with what is in it.
+ *
+ * @param string $table
+ * @param int $count
+ * @param array $rows
+ * @return void
+ */
+	protected function assertTable($table, $count, $rows) {
+		$result = (new Query($this->connection))->select('*')
+			->from($table)
+			->execute();
+		$this->assertCount($count, $result, 'Row count is incorrect');
+		$this->assertEquals($rows, $result->fetchAll('assoc'));
+	}
+
 }