[Form] Fixed violation mapping if multiple forms are using the same (or part of the same) property path

Author: alekitto

Extension/Validator/ViolationMapper/ViolationMapper.php

@@ -147,12 +147,9 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form
      */
     private function matchChild(FormInterface $form, PropertyPathIteratorInterface $it)
     {
-        // Remember at what property path underneath "data"
-        // we are looking. Check if there is a child with that
-        // path, otherwise increase path by one more piece
+        $target = null;
         $chunk = '';
-        $foundChild = null;
-        $foundAtIndex = 0;
+        $foundAtIndex = null;
 
         // Construct mapping rules for the given form
         $rules = array();
@@ -164,17 +161,11 @@ private function matchChild(FormInterface $form, PropertyPathIteratorInterface $
             }
         }
 
-        // Skip forms inheriting their parent data when iterating the children
-        $childIterator = new \RecursiveIteratorIterator(
+        $children = iterator_to_array(new \RecursiveIteratorIterator(
             new InheritDataAwareIterator($form)
-        );
-
-        // Make the path longer until we find a matching child
-        while (true) {
-            if (!$it->valid()) {
-                return;
-            }
+        ));
 
+        while ($it->valid()) {
             if ($it->isIndex()) {
                 $chunk .= '['.$it->current().']';
             } else {
@@ -196,33 +187,27 @@ private function matchChild(FormInterface $form, PropertyPathIteratorInterface $
                 }
             }
 
-            // Test children unless we already found one
-            if (null === $foundChild) {
-                foreach ($childIterator as $child) {
-                    /* @var FormInterface $child */
-                    $childPath = (string) $child->getPropertyPath();
-
-                    // Child found, mark as return value
-                    if ($chunk === $childPath) {
-                        $foundChild = $child;
-                        $foundAtIndex = $it->key();
-                    }
+            /** @var FormInterface $child */
+            foreach ($children as $key => $child) {
+                $childPath = (string) $child->getPropertyPath();
+                if ($childPath === $chunk) {
+                    $target = $child;
+                    $foundAtIndex = $it->key();
+                } elseif (0 === strpos($childPath, $chunk)) {
+                    continue;
                 }
+
+                unset($children[$key]);
             }
 
-            // Add element to the chunk
             $it->next();
+        }
 
-            // If we reached the end of the path or if there are no
-            // more matching mapping rules, return the found child
-            if (null !== $foundChild && (!$it->valid() || count($rules) === 0)) {
-                // Reset index in case we tried to find mapping
-                // rules further down the path
-                $it->seek($foundAtIndex);
-
-                return $foundChild;
-            }
+        if (null !== $foundAtIndex) {
+            $it->seek($foundAtIndex);
         }
+
+        return $target;
     }
 
     /**

Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php

@@ -1474,4 +1474,25 @@ public function testErrorMappingForFormInheritingParentData($target, $childName,
             $this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChildName.' should have an error, but has none');
         }
     }
+
+    public function testBacktrackIfSeveralSubFormsWithSamePropertyPath()
+    {
+        $violation = $this->getConstraintViolation('data.address[street]');
+        $parent = $this->getForm('parent');
+        $child1 = $this->getForm('subform1', 'address');
+        $child2 = $this->getForm('subform2', 'address');
+        $grandChild = $this->getForm('street');
+
+        $parent->add($child1);
+        $parent->add($child2);
+        $child2->add($grandChild);
+
+        $this->mapper->mapViolation($violation, $parent);
+
+        // The error occurred on the child of the second form with the same path
+        $this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
+        $this->assertCount(0, $child1->getErrors(), $child1->getName().' should not have an error, but has one');
+        $this->assertCount(0, $child2->getErrors(), $child2->getName().' should not have an error, but has one');
+        $this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChild->getName().' should have an error, but has none');
+    }
 }