[Validator] Changed NodeTraverser to traverse nodes iteratively, not recursively

In this way, the deep method call chains are avoided. Also, it is possible to
avoid the many calls to leaveNode(), which are currently not really needed.

src/Symfony/Component/Validator/Context/ExecutionContext.php

@@ -20,7 +20,7 @@ use Symfony\Component\Validator\ExecutionContextInterface as LegacyExecutionCont
 use Symfony\Component\Validator\Group\GroupManagerInterface;
 use Symfony\Component\Validator\Mapping\PropertyMetadataInterface;
 use Symfony\Component\Validator\Node\Node;
-use Symfony\Component\Validator\Util\NodeStackInterface;
+use Symfony\Component\Validator\NodeVisitor\NodeObserverInterface;
 use Symfony\Component\Validator\Util\PropertyPath;
 use Symfony\Component\Validator\Validator\ValidatorInterface;
 use Symfony\Component\Validator\Violation\ConstraintViolationBuilder;
@@ -33,7 +33,7 @@ use Symfony\Component\Validator\Violation\ConstraintViolationBuilder;
  *
  * @see ExecutionContextInterface
  */
-class ExecutionContext implements ExecutionContextInterface, LegacyExecutionContextInterface, NodeStackInterface
+class ExecutionContext implements ExecutionContextInterface, LegacyExecutionContextInterface, NodeObserverInterface
 {
     /**
      * @var ValidatorInterface
@@ -76,13 +76,6 @@ class ExecutionContext implements ExecutionContextInterface, LegacyExecutionCont
      */
     private $node;
 
-    /**
-     * The trace of nodes from the root node to the current node.
-     *
-     * @var \SplStack
-     */
-    private $nodeStack;
-
     /**
      * Creates a new execution context.
      *
@@ -108,49 +101,18 @@ class ExecutionContext implements ExecutionContextInterface, LegacyExecutionCont
         $this->translator = $translator;
         $this->translationDomain = $translationDomain;
         $this->violations = new ConstraintViolationList();
-        $this->nodeStack = new \SplStack();
     }
 
     /**
      * Sets the values of the context to match the given node.
      *
-     * Internally, all nodes are stored on a stack and can be removed from that
-     * stack using {@link popNode()}.
-     *
      * @param Node $node The currently validated node
      */
-    public function pushNode(Node $node)
+    public function setCurrentNode(Node $node)
     {
-        $this->nodeStack->push($node);
         $this->node = $node;
     }
 
-    /**
-     * Sets the values of the context to match the previous node.
-     *
-     * The current node is removed from the internal stack and returned.
-     *
-     * @return Node|null The currently validated node or null, if no node was
-     *                   on the stack
-     */
-    public function popNode()
-    {
-        // Nothing to do if the stack is empty
-        if (0 === count($this->nodeStack)) {
-            return null;
-        }
-
-        // Remove the current node from the stack
-        $poppedNode = $this->nodeStack->pop();
-
-        // Adjust the current node to the previous node
-        $this->node = count($this->nodeStack) > 0
-            ? $this->nodeStack->top()
-            : null;
-
-        return $poppedNode;
-    }
-
     /**
      * {@inheritdoc}
      */

src/Symfony/Component/Validator/Node/PropertyNode.php

@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Validator\Node;
 
+use Symfony\Component\Validator\Exception\UnexpectedTypeException;
 use Symfony\Component\Validator\Mapping\PropertyMetadataInterface;
 
 /**
@@ -32,6 +33,11 @@ use Symfony\Component\Validator\Mapping\PropertyMetadataInterface;
  */
 class PropertyNode extends Node
 {
+    /**
+     * @var object
+     */
+    public $object;
+
     /**
      * @var PropertyMetadataInterface
      */
@@ -40,6 +46,8 @@ class PropertyNode extends Node
     /**
      * Creates a new property node.
      *
+     * @param object                    $object         The object the property
+     *                                                  belongs to
      * @param mixed                     $value          The property value
      * @param PropertyMetadataInterface $metadata       The property's metadata
      * @param string                    $propertyPath   The property path leading
@@ -49,9 +57,15 @@ class PropertyNode extends Node
      * @param string[]|null             $cascadedGroups The groups in which
      *                                                  cascaded objects should
      *                                                  be validated
+     *
+     * @throws UnexpectedTypeException If $object is not an object
      */
-    public function __construct($value, PropertyMetadataInterface $metadata, $propertyPath, array $groups, $cascadedGroups = null)
+    public function __construct($object, $value, PropertyMetadataInterface $metadata, $propertyPath, array $groups, $cascadedGroups = null)
     {
+        if (!is_object($object)) {
+            throw new UnexpectedTypeException($object, 'object');
+        }
+
         parent::__construct(
             $value,
             $metadata,
@@ -59,6 +73,8 @@ class PropertyNode extends Node
             $groups,
             $cascadedGroups
         );
+
+        $this->object = $object;
     }
 
 }

src/Symfony/Component/Validator/NodeTraverser/NodeTraverser.php

@@ -44,6 +44,7 @@ class NodeTraverser implements NodeTraverserInterface
     public function __construct(MetadataFactoryInterface $metadataFactory)
     {
         $this->visitors = new \SplObjectStorage();
+        $this->nodeQueue = new \SplQueue();
         $this->metadataFactory = $metadataFactory;
     }
 
@@ -73,11 +74,19 @@ class NodeTraverser implements NodeTraverserInterface
             }
         }
 
+        $traversal = new Traversal($context);
+
         foreach ($nodes as $node) {
-            if ($node instanceof ClassNode) {
+            $traversal->nodeQueue->enqueue($node);
-                $this->traverseClassNode($node, $context);
+
-            } else {
+            while (!$traversal->nodeQueue->isEmpty()) {
-                $this->traverseNode($node, $context);
+                $node = $traversal->nodeQueue->dequeue();
+
+                if ($node instanceof ClassNode) {
+                    $this->traverseClassNode($node, $traversal);
+                } else {
+                    $this->traverseNode($node, $traversal);
+                }
             }
         }
 
@@ -91,49 +100,31 @@ class NodeTraverser implements NodeTraverserInterface
         }
     }
 
-    private function enterNode(Node $node, ExecutionContextInterface $context)
+    private function visit(Node $node, ExecutionContextInterface $context)
     {
-        $continueTraversal = true;
-
         foreach ($this->visitors as $visitor) {
-            if (false === $visitor->enterNode($node, $context)) {
+            if (false === $visitor->visit($node, $context)) {
-                $continueTraversal = false;
+                return false;
-
-                // Continue, so that the enterNode() method of all visitors
-                // is called
             }
         }
 
-        return $continueTraversal;
+        return true;
     }
 
-    private function leaveNode(Node $node, ExecutionContextInterface $context)
+    private function traverseNode(Node $node, Traversal $traversal)
     {
-        foreach ($this->visitors as $visitor) {
+        if (false === $this->visit($node, $traversal->context)) {
-            $visitor->leaveNode($node, $context);
-        }
-    }
-
-    private function traverseNode(Node $node, ExecutionContextInterface $context)
-    {
-        $continue = $this->enterNode($node, $context);
-
-        // Visitors have two possibilities to influence the traversal:
-        //
-        // 1. If a visitor's enterNode() method returns false, the traversal is
-        //    skipped entirely.
-        // 2. If a visitor's enterNode() method removes a group from the node,
-        //    that group will be skipped in the subtree of that node.
-
-        if (false === $continue) {
-            $this->leaveNode($node, $context);
-
             return;
         }
 
-        if (null === $node->value) {
+        // Visitors have two possibilities to influence the traversal:
-            $this->leaveNode($node, $context);
+        //
+        // 1. If a visitor's visit() method returns false, the traversal is
+        //    skipped entirely.
+        // 2. If a visitor's visit() method removes a group from the node,
+        //    that group will be skipped in the subtree of that node.
 
+        if (null === $node->value) {
             return;
         }
 
@@ -144,8 +135,6 @@ class NodeTraverser implements NodeTraverserInterface
             : $node->groups;
 
         if (0 === count($cascadedGroups)) {
-            $this->leaveNode($node, $context);
-
             return;
         }
 
@@ -161,11 +150,9 @@ class NodeTraverser implements NodeTraverserInterface
                 $node->propertyPath,
                 $cascadedGroups,
                 $traversalStrategy,
-                $context
+                $traversal
             );
 
-            $this->leaveNode($node, $context);
-
             return;
         }
 
@@ -178,25 +165,19 @@ class NodeTraverser implements NodeTraverserInterface
                 $node->propertyPath,
                 $cascadedGroups,
                 $traversalStrategy,
-                $context
+                $traversal
             );
 
-            $this->leaveNode($node, $context);
-
             return;
         }
 
         // Traverse only if the TRAVERSE bit is set
         if (!($traversalStrategy & TraversalStrategy::TRAVERSE)) {
-            $this->leaveNode($node, $context);
-
             return;
         }
 
         if (!$node->value instanceof \Traversable) {
             if ($traversalStrategy & TraversalStrategy::IGNORE_NON_TRAVERSABLE) {
-                $this->leaveNode($node, $context);
-
                 return;
             }
 
@@ -212,15 +193,15 @@ class NodeTraverser implements NodeTraverserInterface
             $node->propertyPath,
             $cascadedGroups,
             $traversalStrategy,
-            $context
+            $traversal
         );
-
-        $this->leaveNode($node, $context);
     }
 
-    private function traverseClassNode(ClassNode $node, ExecutionContextInterface $context, $traversalStrategy = TraversalStrategy::IMPLICIT)
+    private function traverseClassNode(ClassNode $node, Traversal $traversal, $traversalStrategy = TraversalStrategy::IMPLICIT)
     {
-        $continue = $this->enterNode($node, $context);
+        if (false === $this->visit($node, $traversal->context)) {
+            return;
+        }
 
         // Visitors have two possibilities to influence the traversal:
         //
@@ -229,21 +210,14 @@ class NodeTraverser implements NodeTraverserInterface
         // 2. If a visitor's enterNode() method removes a group from the node,
         //    that group will be skipped in the subtree of that node.
 
-        if (false === $continue) {
-            $this->leaveNode($node, $context);
-
-            return;
-        }
-
         if (0 === count($node->groups)) {
-            $this->leaveNode($node, $context);
-
             return;
         }
 
         foreach ($node->metadata->getConstrainedProperties() as $propertyName) {
             foreach ($node->metadata->getPropertyMetadata($propertyName) as $propertyMetadata) {
-                $propertyNode = new PropertyNode(
+                $traversal->nodeQueue->enqueue(new PropertyNode(
+                    $node->value,
                     $propertyMetadata->getPropertyValue($node->value),
                     $propertyMetadata,
                     $node->propertyPath
@@ -251,9 +225,7 @@ class NodeTraverser implements NodeTraverserInterface
                         : $propertyName,
                     $node->groups,
                     $node->cascadedGroups
-                );
+                ));
-
-                $this->traverseNode($propertyNode, $context);
             }
         }
 
@@ -265,15 +237,11 @@ class NodeTraverser implements NodeTraverserInterface
 
         // Traverse only if the TRAVERSE bit is set
         if (!($traversalStrategy & TraversalStrategy::TRAVERSE)) {
-            $this->leaveNode($node, $context);
-
             return;
         }
 
         if (!$node->value instanceof \Traversable) {
             if ($traversalStrategy & TraversalStrategy::IGNORE_NON_TRAVERSABLE) {
-                $this->leaveNode($node, $context);
-
                 return;
             }
 
@@ -289,13 +257,11 @@ class NodeTraverser implements NodeTraverserInterface
             $node->propertyPath,
             $node->groups,
             $traversalStrategy,
-            $context
+            $traversal
         );
-
-        $this->leaveNode($node, $context);
     }
 
-    private function cascadeObject($object, $propertyPath, array $groups, $traversalStrategy, ExecutionContextInterface $context)
+    private function cascadeObject($object, $propertyPath, array $groups, $traversalStrategy, Traversal $traversal)
     {
         try {
             $classMetadata = $this->metadataFactory->getMetadataFor($object);
@@ -304,14 +270,12 @@ class NodeTraverser implements NodeTraverserInterface
                 // error
             }
 
-            $classNode = new ClassNode(
+            $traversal->nodeQueue->enqueue(new ClassNode(
                 $object,
                 $classMetadata,
                 $propertyPath,
                 $groups
-            );
+            ));
-
-            $this->traverseClassNode($classNode, $context, $traversalStrategy);
         } catch (NoSuchMetadataException $e) {
             // Rethrow if the TRAVERSE bit is not set
             if (!($traversalStrategy & TraversalStrategy::TRAVERSE)) {
@@ -329,12 +293,12 @@ class NodeTraverser implements NodeTraverserInterface
                  $propertyPath,
                  $groups,
                  $traversalStrategy,
-                 $context
+                 $traversal
             );
         }
     }
 
-    private function cascadeEachObjectIn($collection, $propertyPath, array $groups, $traversalStrategy, ExecutionContextInterface $context)
+    private function cascadeEachObjectIn($collection, $propertyPath, array $groups, $traversalStrategy, Traversal $traversal)
     {
         if ($traversalStrategy & TraversalStrategy::RECURSIVE) {
             // Try to traverse nested objects, but ignore if they do not
@@ -357,7 +321,7 @@ class NodeTraverser implements NodeTraverserInterface
                     $propertyPath.'['.$key.']',
                     $groups,
                     $traversalStrategy,
-                    $context
+                    $traversal
                 );
 
                 continue;
@@ -371,7 +335,7 @@ class NodeTraverser implements NodeTraverserInterface
                     $propertyPath.'['.$key.']',
                     $groups,
                     $traversalStrategy,
-                    $context
+                    $traversal
                 );
             }
         }

src/Symfony/Component/Validator/NodeTraverser/Traversal.php

@@ -0,0 +1,31 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Validator\NodeTraverser;
+
+use Symfony\Component\Validator\Context\ExecutionContextInterface;
+
+/**
+ * @since  %%NextVersion%%
+ * @author Bernhard Schussek <bschussek@gmail.com>
+ */
+class Traversal
+{
+    public $context;
+
+    public $nodeQueue;
+
+    public function __construct(ExecutionContextInterface $context)
+    {
+        $this->context = $context;
+        $this->nodeQueue = new \SplQueue();
+    }
+}

src/Symfony/Component/Validator/NodeVisitor/AbstractVisitor.php

@@ -28,11 +28,7 @@ abstract class AbstractVisitor implements NodeVisitorInterface
     {
     }
 
-    public function enterNode(Node $node, ExecutionContextInterface $context)
+    public function visit(Node $node, ExecutionContextInterface $context)
-    {
-    }
-
-    public function leaveNode(Node $node, ExecutionContextInterface $context)
     {
     }
 }

src/Symfony/Component/Validator/NodeVisitor/ContextRefresherVisitor.php

@@ -14,7 +14,6 @@ namespace Symfony\Component\Validator\NodeVisitor;
 use Symfony\Component\Validator\Context\ExecutionContextInterface;
 use Symfony\Component\Validator\Exception\RuntimeException;
 use Symfony\Component\Validator\Node\Node;
-use Symfony\Component\Validator\Util\NodeStackInterface;
 
 /**
  * Updates the current context with the current node of the validation
@@ -25,31 +24,17 @@ use Symfony\Component\Validator\Util\NodeStackInterface;
  */
 class ContextRefresherVisitor extends AbstractVisitor
 {
-    public function enterNode(Node $node, ExecutionContextInterface $context)
+    public function visit(Node $node, ExecutionContextInterface $context)
     {
-        if (!$context instanceof NodeStackInterface) {
+        if (!$context instanceof NodeObserverInterface) {
             throw new RuntimeException(sprintf(
                 'The ContextRefresherVisitor only supports instances of class '.
-                '"Symfony\Component\Validator\Context\NodeStackInterface". '.
+                '"Symfony\Component\Validator\NodeVisitor\NodeObserverInterface". '.
                 'An instance of class "%s" was given.',
                 get_class($context)
             ));
         }
 
-        $context->pushNode($node);
+        $context->setCurrentNode($node);
-    }
-
-    public function leaveNode(Node $node, ExecutionContextInterface $context)
-    {
-        if (!$context instanceof NodeStackInterface) {
-            throw new RuntimeException(sprintf(
-                'The ContextRefresherVisitor only supports instances of class '.
-                '"Symfony\Component\Validator\Context\NodeStackInterface". '.
-                'An instance of class "%s" was given.',
-                get_class($context)
-            ));
-        }
-
-        $context->popNode();
     }
 }

src/Symfony/Component/Validator/NodeVisitor/GroupSequenceResolverVisitor.php

@@ -23,7 +23,7 @@ use Symfony\Component\Validator\Node\Node;
  */
 class GroupSequenceResolverVisitor extends AbstractVisitor
 {
-    public function enterNode(Node $node, ExecutionContextInterface $context)
+    public function visit(Node $node, ExecutionContextInterface $context)
     {
         if (!$node instanceof ClassNode) {
             return;

src/Symfony/Component/Validator/NodeVisitor/NodeObserverInterface.php

@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Symfony\Component\Validator\Util;
+namespace Symfony\Component\Validator\NodeVisitor;
 
 use Symfony\Component\Validator\Node\Node;
 
@@ -17,9 +17,7 @@ use Symfony\Component\Validator\Node\Node;
  * @since  %%NextVersion%%
  * @author Bernhard Schussek <bschussek@gmail.com>
  */
-interface NodeStackInterface
+interface NodeObserverInterface
 {
-    public function pushNode(Node $node);
+    public function setCurrentNode(Node $node);
-
-    public function popNode();
 }

src/Symfony/Component/Validator/NodeVisitor/NodeValidatorVisitor.php

@@ -42,29 +42,24 @@ class NodeValidatorVisitor extends AbstractVisitor implements GroupManagerInterf
 
     private $currentGroup;
 
-    private $objectHashStack;
-
     public function __construct(NodeTraverserInterface $nodeTraverser, ConstraintValidatorFactoryInterface $validatorFactory)
     {
         $this->validatorFactory = $validatorFactory;
         $this->nodeTraverser = $nodeTraverser;
-        $this->objectHashStack = new \SplStack();
     }
 
     public function afterTraversal(array $nodes, ExecutionContextInterface $context)
     {
         $this->validatedObjects = array();
         $this->validatedConstraints = array();
-        $this->objectHashStack = new \SplStack();
     }
 
-    public function enterNode(Node $node, ExecutionContextInterface $context)
+    public function visit(Node $node, ExecutionContextInterface $context)
     {
         if ($node instanceof ClassNode) {
             $objectHash = spl_object_hash($node->value);
-            $this->objectHashStack->push($objectHash);
+        } elseif ($node instanceof PropertyNode) {
-        } elseif ($node instanceof PropertyNode && count($this->objectHashStack) > 0) {
+            $objectHash = spl_object_hash($node->object);
-            $objectHash = $this->objectHashStack->top();
         } else {
             $objectHash = null;
         }
@@ -116,13 +111,6 @@ class NodeValidatorVisitor extends AbstractVisitor implements GroupManagerInterf
         return true;
     }
 
-    public function leaveNode(Node $node, ExecutionContextInterface $context)
-    {
-        if ($node instanceof ClassNode) {
-            $this->objectHashStack->pop();
-        }
-    }
-
     public function getCurrentGroup()
     {
         return $this->currentGroup;

src/Symfony/Component/Validator/NodeVisitor/NodeVisitorInterface.php

@@ -24,7 +24,5 @@ interface NodeVisitorInterface
 
     public function afterTraversal(array $nodes, ExecutionContextInterface $context);
 
-    public function enterNode(Node $node, ExecutionContextInterface $context);
+    public function visit(Node $node, ExecutionContextInterface $context);
-
-    public function leaveNode(Node $node, ExecutionContextInterface $context);
 }

src/Symfony/Component/Validator/NodeVisitor/ObjectInitializerVisitor.php

@@ -43,7 +43,7 @@ class ObjectInitializerVisitor extends AbstractVisitor
         $this->initializers = $initializers;
     }
 
-    public function enterNode(Node $node, ExecutionContextInterface $context)
+    public function visit(Node $node, ExecutionContextInterface $context)
     {
         if (!$node instanceof ClassNode) {
             return;

src/Symfony/Component/Validator/Tests/Context/ExecutionContextTest.php

@@ -55,41 +55,6 @@ class ExecutionContextTest extends \PHPUnit_Framework_TestCase
         );
     }
 
-    public function testPushAndPop()
-    {
-        $metadata = $this->getMock('Symfony\Component\Validator\Mapping\MetadataInterface');
-        $node = new GenericNode('value', $metadata, '', array(), array());
-
-        $this->context->pushNode($node);
-
-        $this->assertSame('value', $this->context->getValue());
-        // the other methods are covered in AbstractValidatorTest
-
-        $this->assertSame($node, $this->context->popNode());
-
-        $this->assertNull($this->context->getValue());
-    }
-
-    public function testPushTwiceAndPop()
-    {
-        $metadata1 = $this->getMock('Symfony\Component\Validator\Mapping\MetadataInterface');
-        $node1 = new GenericNode('value', $metadata1, '', array(), array());
-        $metadata2 = $this->getMock('Symfony\Component\Validator\Mapping\MetadataInterface');
-        $node2 = new GenericNode('other value', $metadata2, '', array(), array());
-
-        $this->context->pushNode($node1);
-        $this->context->pushNode($node2);
-
-        $this->assertSame($node2, $this->context->popNode());
-
-        $this->assertSame('value', $this->context->getValue());
-    }
-
-    public function testPopWithoutPush()
-    {
-        $this->assertNull($this->context->popNode());
-    }
-
     public function testGetGroup()
     {
         $this->groupManager->expects($this->once())

src/Symfony/Component/Validator/Validator/ContextualValidator.php

@@ -141,6 +141,7 @@ class ContextualValidator implements ContextualValidatorInterface
             $propertyValue = $propertyMetadata->getPropertyValue($object);
 
             $nodes[] = new PropertyNode(
+                $object,
                 $propertyValue,
                 $propertyMetadata,
                 PropertyPath::append($this->defaultPropertyPath, $propertyName),
@@ -172,6 +173,7 @@ class ContextualValidator implements ContextualValidatorInterface
 
         foreach ($propertyMetadatas as $propertyMetadata) {
             $nodes[] = new PropertyNode(
+                $object,
                 $value,
                 $propertyMetadata,
                 PropertyPath::append($this->defaultPropertyPath, $propertyName),