diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000000..e742c9b351 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,2 @@ +Tests/ export-ignore +phpunit.xml.dist export-ignore diff --git a/Extension/Validator/ViolationMapper/ViolationMapper.php b/Extension/Validator/ViolationMapper/ViolationMapper.php index 49a73221d1..9ac6bb69b3 100644 --- a/Extension/Validator/ViolationMapper/ViolationMapper.php +++ b/Extension/Validator/ViolationMapper/ViolationMapper.php @@ -25,21 +25,6 @@ */ class ViolationMapper implements ViolationMapperInterface { - /** - * @var FormInterface - */ - private $scope; - - /** - * @var array - */ - private $children; - - /** - * @var array - */ - private $rules = array(); - /** * @var Boolean */ @@ -52,6 +37,13 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form { $this->allowNonSynchronized = $allowNonSynchronized; + // The scope is the currently found most specific form that + // an error should be mapped to. After setting the scope, the + // mapper will try to continue to find more specific matches in + // the children of scope. If it cannot, the error will be + // mapped to this scope. + $scope = null; + $violationPath = null; $relativePath = null; $match = false; @@ -65,7 +57,7 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form // This case happens if the violation path is empty and thus // the violation should be mapped to the root form if (null === $violationPath) { - $this->scope = $form; + $scope = $form; } // In general, mapping happens from the root form to the leaf forms @@ -85,11 +77,11 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form // This root will usually be $form. If the path contains // an unmapped form though, the last unmapped form found // will be the root of the path. - $this->setScope($relativePath->getRoot()); + $scope = $relativePath->getRoot(); $it = new PropertyPathIterator($relativePath); - while ($this->isValidScope() && null !== ($child = $this->matchChild($it))) { - $this->setScope($child); + while ($this->acceptsErrors($scope) && null !== ($child = $this->matchChild($scope, $it))) { + $scope = $child; $it->next(); $match = true; } @@ -105,33 +97,32 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form // e.g. "children[foo].children[bar].data.baz" // Here the innermost directly mapped child is "bar" + $scope = $form; $it = new ViolationPathIterator($violationPath); - // The overhead of setScope() is not needed anymore here - $this->scope = $form; - while ($this->isValidScope() && $it->valid() && $it->mapsForm()) { - if (!$this->scope->has($it->current())) { + while ($this->acceptsErrors($scope) && $it->valid() && $it->mapsForm()) { + if (!$scope->has($it->current())) { // Break if we find a reference to a non-existing child break; } - $this->scope = $this->scope->get($it->current()); + $scope = $scope->get($it->current()); $it->next(); } } // Follow dot rules until we have the final target - $mapping = $this->scope->getConfig()->getOption('error_mapping'); + $mapping = $scope->getConfig()->getOption('error_mapping'); - while ($this->isValidScope() && isset($mapping['.'])) { - $dotRule = new MappingRule($this->scope, '.', $mapping['.']); - $this->scope = $dotRule->getTarget(); - $mapping = $this->scope->getConfig()->getOption('error_mapping'); + while ($this->acceptsErrors($scope) && isset($mapping['.'])) { + $dotRule = new MappingRule($scope, '.', $mapping['.']); + $scope = $dotRule->getTarget(); + $mapping = $scope->getConfig()->getOption('error_mapping'); } // Only add the error if the form is synchronized - if ($this->isValidScope()) { - $this->scope->addError(new FormError( + if ($this->acceptsErrors($scope)) { + $scope->addError(new FormError( $violation->getMessageTemplate(), $violation->getMessageParameters(), $violation->getMessagePluralization() @@ -146,11 +137,12 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form * If a matching child is found, it is returned. Otherwise * null is returned. * - * @param PropertyPathIteratorInterface $it The iterator at its current position. + * @param FormInterface $form The form to search. + * @param PropertyPathIteratorInterface $it The iterator at its current position. * * @return null|FormInterface The found match or null. */ - private function matchChild(PropertyPathIteratorInterface $it) + 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 @@ -159,6 +151,21 @@ private function matchChild(PropertyPathIteratorInterface $it) $foundChild = null; $foundAtIndex = 0; + // Construct mapping rules for the given form + $rules = array(); + + foreach ($form->getConfig()->getOption('error_mapping') as $propertyPath => $targetPath) { + // Dot rules are considered at the very end + if ('.' !== $propertyPath) { + $rules[] = new MappingRule($form, $propertyPath, $targetPath); + } + } + + // Ignore virtual forms when iterating the children + $childIterator = new \RecursiveIteratorIterator( + new VirtualFormAwareIterator($form->all()) + ); + // Make the path longer until we find a matching child while (true) { if (!$it->valid()) { @@ -172,7 +179,7 @@ private function matchChild(PropertyPathIteratorInterface $it) } // Test mapping rules as long as we have any - foreach ($this->rules as $key => $rule) { + foreach ($rules as $key => $rule) { /* @var MappingRule $rule */ // Mapping rule matches completely, terminate. @@ -182,17 +189,17 @@ private function matchChild(PropertyPathIteratorInterface $it) // Keep only rules that have $chunk as prefix if (!$rule->isPrefix($chunk)) { - unset($this->rules[$key]); + unset($rules[$key]); } } // Test children unless we already found one if (null === $foundChild) { - foreach ($this->children as $child) { + foreach ($childIterator as $child) { /* @var FormInterface $child */ $childPath = (string) $child->getPropertyPath(); - // Child found, move scope inwards + // Child found, mark as return value if ($chunk === $childPath) { $foundChild = $child; $foundAtIndex = $it->key(); @@ -205,7 +212,7 @@ private function matchChild(PropertyPathIteratorInterface $it) // 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($this->rules) === 0)) { + 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); @@ -277,35 +284,12 @@ private function reconstructPath(ViolationPath $violationPath, FormInterface $or } /** - * Sets the scope of the mapper to the given form. - * - * The scope is the currently found most specific form that - * an error should be mapped to. After setting the scope, the - * mapper will try to continue to find more specific matches in - * the children of scope. If it cannot, the error will be - * mapped to this scope. + * @param FormInterface $form * - * @param FormInterface $form The current scope. - */ - private function setScope(FormInterface $form) - { - $this->scope = $form; - $this->children = new \RecursiveIteratorIterator( - new VirtualFormAwareIterator($form->all()) - ); - foreach ($form->getConfig()->getOption('error_mapping') as $propertyPath => $targetPath) { - // Dot rules are considered at the very end - if ('.' !== $propertyPath) { - $this->rules[] = new MappingRule($form, $propertyPath, $targetPath); - } - } - } - - /** * @return Boolean */ - private function isValidScope() + private function acceptsErrors(FormInterface $form) { - return $this->allowNonSynchronized || $this->scope->isSynchronized(); + return $this->allowNonSynchronized || $form->isSynchronized(); } } diff --git a/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php b/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php index 6f391986d1..b3d9608cc3 100644 --- a/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php +++ b/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php @@ -235,6 +235,7 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', 'address', 'street', 'street', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', 'address', 'street', 'street', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', 'address', 'street', 'street', 'children[address].data'), array(self::LEVEL_2, 'address', 'address', 'street', 'street', 'children[address].data.street'), array(self::LEVEL_2, 'address', 'address', 'street', 'street', 'children[address].data.street.prop'), array(self::LEVEL_1, 'address', 'address', 'street', 'street', 'children[address].data[street]'), @@ -250,6 +251,7 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', 'address', 'street', '[street]', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', 'address', 'street', '[street]', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', 'address', 'street', '[street]', 'children[address].data'), array(self::LEVEL_1, 'address', 'address', 'street', '[street]', 'children[address].data.street'), array(self::LEVEL_1, 'address', 'address', 'street', '[street]', 'children[address].data.street.prop'), array(self::LEVEL_2, 'address', 'address', 'street', '[street]', 'children[address].data[street]'), @@ -265,6 +267,7 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', '[address]', 'street', 'street', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', '[address]', 'street', 'street', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', '[address]', 'street', 'street', 'children[address].data'), array(self::LEVEL_2, 'address', '[address]', 'street', 'street', 'children[address].data.street'), array(self::LEVEL_2, 'address', '[address]', 'street', 'street', 'children[address].data.street.prop'), array(self::LEVEL_1, 'address', '[address]', 'street', 'street', 'children[address].data[street]'), @@ -280,6 +283,7 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', '[address]', 'street', '[street]', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', '[address]', 'street', '[street]', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', '[address]', 'street', '[street]', 'children[address].data'), array(self::LEVEL_1, 'address', '[address]', 'street', '[street]', 'children[address].data.street'), array(self::LEVEL_1, 'address', '[address]', 'street', '[street]', 'children[address].data.street.prop'), array(self::LEVEL_2, 'address', '[address]', 'street', '[street]', 'children[address].data[street]'), @@ -295,6 +299,7 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', 'person.address', 'street', 'street', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', 'person.address', 'street', 'street', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', 'person.address', 'street', 'street', 'children[address].data'), array(self::LEVEL_2, 'address', 'person.address', 'street', 'street', 'children[address].data.street'), array(self::LEVEL_2, 'address', 'person.address', 'street', 'street', 'children[address].data.street.prop'), array(self::LEVEL_1, 'address', 'person.address', 'street', 'street', 'children[address].data[street]'), @@ -318,6 +323,7 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', 'person.address', 'street', '[street]', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', 'person.address', 'street', '[street]', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', 'person.address', 'street', '[street]', 'children[address].data'), array(self::LEVEL_1, 'address', 'person.address', 'street', '[street]', 'children[address].data.street'), array(self::LEVEL_1, 'address', 'person.address', 'street', '[street]', 'children[address].data.street.prop'), array(self::LEVEL_2, 'address', 'person.address', 'street', '[street]', 'children[address].data[street]'), @@ -341,6 +347,7 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', 'person[address]', 'street', 'street', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', 'person[address]', 'street', 'street', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', 'person[address]', 'street', 'street', 'children[address].data'), array(self::LEVEL_2, 'address', 'person[address]', 'street', 'street', 'children[address].data.street'), array(self::LEVEL_2, 'address', 'person[address]', 'street', 'street', 'children[address].data.street.prop'), array(self::LEVEL_1, 'address', 'person[address]', 'street', 'street', 'children[address].data[street]'), @@ -364,6 +371,7 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', 'person[address]', 'street', '[street]', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', 'person[address]', 'street', '[street]', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', 'person[address]', 'street', '[street]', 'children[address].data'), array(self::LEVEL_1, 'address', 'person[address]', 'street', '[street]', 'children[address].data.street'), array(self::LEVEL_1, 'address', 'person[address]', 'street', '[street]', 'children[address].data.street.prop'), array(self::LEVEL_2, 'address', 'person[address]', 'street', '[street]', 'children[address].data[street]'), @@ -387,6 +395,7 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', '[person].address', 'street', 'street', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', '[person].address', 'street', 'street', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', '[person].address', 'street', 'street', 'children[address].data'), array(self::LEVEL_2, 'address', '[person].address', 'street', 'street', 'children[address].data.street'), array(self::LEVEL_2, 'address', '[person].address', 'street', 'street', 'children[address].data.street.prop'), array(self::LEVEL_1, 'address', '[person].address', 'street', 'street', 'children[address].data[street]'), @@ -410,6 +419,7 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', '[person].address', 'street', '[street]', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', '[person].address', 'street', '[street]', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', '[person].address', 'street', '[street]', 'children[address].data'), array(self::LEVEL_1, 'address', '[person].address', 'street', '[street]', 'children[address].data.street'), array(self::LEVEL_1, 'address', '[person].address', 'street', '[street]', 'children[address].data.street.prop'), array(self::LEVEL_2, 'address', '[person].address', 'street', '[street]', 'children[address].data[street]'), @@ -433,6 +443,8 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', '[person][address]', 'street', 'street', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', '[person][address]', 'street', 'street', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', '[person][address]', 'street', 'street', 'children[address]'), + array(self::LEVEL_1, 'address', '[person][address]', 'street', 'street', 'children[address].data'), array(self::LEVEL_2, 'address', '[person][address]', 'street', 'street', 'children[address].data.street'), array(self::LEVEL_2, 'address', '[person][address]', 'street', 'street', 'children[address].data.street.prop'), array(self::LEVEL_1, 'address', '[person][address]', 'street', 'street', 'children[address].data[street]'), @@ -456,6 +468,7 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', '[person][address]', 'street', '[street]', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', '[person][address]', 'street', '[street]', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', '[person][address]', 'street', '[street]', 'children[address].data'), array(self::LEVEL_1, 'address', '[person][address]', 'street', '[street]', 'children[address].data.street'), array(self::LEVEL_1, 'address', '[person][address]', 'street', '[street]', 'children[address].data.street.prop'), array(self::LEVEL_2, 'address', '[person][address]', 'street', '[street]', 'children[address].data[street]'), @@ -479,10 +492,13 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', 'address', 'street', 'office.street', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', 'address', 'street', 'office.street', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', 'address', 'street', 'office.street', 'children[address].data'), + array(self::LEVEL_1, 'address', 'address', 'street', 'office.street', 'children[address].data.office'), array(self::LEVEL_2, 'address', 'address', 'street', 'office.street', 'children[address].data.office.street'), array(self::LEVEL_2, 'address', 'address', 'street', 'office.street', 'children[address].data.office.street.prop'), array(self::LEVEL_1, 'address', 'address', 'street', 'office.street', 'children[address].data.office[street]'), array(self::LEVEL_1, 'address', 'address', 'street', 'office.street', 'children[address].data.office[street].prop'), + array(self::LEVEL_1, 'address', 'address', 'street', 'office.street', 'children[address].data[office]'), array(self::LEVEL_1, 'address', 'address', 'street', 'office.street', 'children[address].data[office].street'), array(self::LEVEL_1, 'address', 'address', 'street', 'office.street', 'children[address].data[office].street.prop'), array(self::LEVEL_1, 'address', 'address', 'street', 'office.street', 'children[address].data[office][street]'), @@ -506,10 +522,13 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', '[address]', 'street', 'office.street', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', '[address]', 'street', 'office.street', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', '[address]', 'street', 'office.street', 'children[address].data'), + array(self::LEVEL_1, 'address', '[address]', 'street', 'office.street', 'children[address].data.office'), array(self::LEVEL_2, 'address', '[address]', 'street', 'office.street', 'children[address].data.office.street'), array(self::LEVEL_2, 'address', '[address]', 'street', 'office.street', 'children[address].data.office.street.prop'), array(self::LEVEL_1, 'address', '[address]', 'street', 'office.street', 'children[address].data.office[street]'), array(self::LEVEL_1, 'address', '[address]', 'street', 'office.street', 'children[address].data.office[street].prop'), + array(self::LEVEL_1, 'address', '[address]', 'street', 'office.street', 'children[address].data[office]'), array(self::LEVEL_1, 'address', '[address]', 'street', 'office.street', 'children[address].data[office].street'), array(self::LEVEL_1, 'address', '[address]', 'street', 'office.street', 'children[address].data[office].street.prop'), array(self::LEVEL_1, 'address', '[address]', 'street', 'office.street', 'children[address].data[office][street]'), @@ -533,10 +552,13 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', 'address', 'street', 'office[street]', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', 'address', 'street', 'office[street]', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', 'address', 'street', 'office[street]', 'children[address].data'), + array(self::LEVEL_1, 'address', 'address', 'street', 'office[street]', 'children[address].data.office'), array(self::LEVEL_1, 'address', 'address', 'street', 'office[street]', 'children[address].data.office.street'), array(self::LEVEL_1, 'address', 'address', 'street', 'office[street]', 'children[address].data.office.street.prop'), array(self::LEVEL_2, 'address', 'address', 'street', 'office[street]', 'children[address].data.office[street]'), array(self::LEVEL_2, 'address', 'address', 'street', 'office[street]', 'children[address].data.office[street].prop'), + array(self::LEVEL_1, 'address', 'address', 'street', 'office[street]', 'children[address].data[office]'), array(self::LEVEL_1, 'address', 'address', 'street', 'office[street]', 'children[address].data[office].street'), array(self::LEVEL_1, 'address', 'address', 'street', 'office[street]', 'children[address].data[office].street.prop'), array(self::LEVEL_1, 'address', 'address', 'street', 'office[street]', 'children[address].data[office][street]'), @@ -564,6 +586,7 @@ public function provideDefaultTests() array(self::LEVEL_1, 'address', '[address]', 'street', 'office[street]', 'children[address].data.office.street.prop'), array(self::LEVEL_2, 'address', '[address]', 'street', 'office[street]', 'children[address].data.office[street]'), array(self::LEVEL_2, 'address', '[address]', 'street', 'office[street]', 'children[address].data.office[street].prop'), + array(self::LEVEL_1, 'address', '[address]', 'street', 'office[street]', 'children[address].data[office]'), array(self::LEVEL_1, 'address', '[address]', 'street', 'office[street]', 'children[address].data[office].street'), array(self::LEVEL_1, 'address', '[address]', 'street', 'office[street]', 'children[address].data[office].street.prop'), array(self::LEVEL_1, 'address', '[address]', 'street', 'office[street]', 'children[address].data[office][street]'), @@ -587,10 +610,13 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', 'address', 'street', '[office].street', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', 'address', 'street', '[office].street', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', 'address', 'street', '[office].street', 'children[address].data'), + array(self::LEVEL_1, 'address', 'address', 'street', '[office].street', 'children[address].data.office'), array(self::LEVEL_1, 'address', 'address', 'street', '[office].street', 'children[address].data.office.street'), array(self::LEVEL_1, 'address', 'address', 'street', '[office].street', 'children[address].data.office.street.prop'), array(self::LEVEL_1, 'address', 'address', 'street', '[office].street', 'children[address].data.office[street]'), array(self::LEVEL_1, 'address', 'address', 'street', '[office].street', 'children[address].data.office[street].prop'), + array(self::LEVEL_1, 'address', 'address', 'street', '[office].street', 'children[address].data[office]'), array(self::LEVEL_2, 'address', 'address', 'street', '[office].street', 'children[address].data[office].street'), array(self::LEVEL_2, 'address', 'address', 'street', '[office].street', 'children[address].data[office].street.prop'), array(self::LEVEL_1, 'address', 'address', 'street', '[office].street', 'children[address].data[office][street]'), @@ -614,10 +640,13 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', '[address]', 'street', '[office].street', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', '[address]', 'street', '[office].street', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', '[address]', 'street', '[office].street', 'children[address].data'), + array(self::LEVEL_1, 'address', '[address]', 'street', '[office].street', 'children[address].data.office'), array(self::LEVEL_1, 'address', '[address]', 'street', '[office].street', 'children[address].data.office.street'), array(self::LEVEL_1, 'address', '[address]', 'street', '[office].street', 'children[address].data.office.street.prop'), array(self::LEVEL_1, 'address', '[address]', 'street', '[office].street', 'children[address].data.office[street]'), array(self::LEVEL_1, 'address', '[address]', 'street', '[office].street', 'children[address].data.office[street].prop'), + array(self::LEVEL_1, 'address', '[address]', 'street', '[office].street', 'children[address].data[office]'), array(self::LEVEL_2, 'address', '[address]', 'street', '[office].street', 'children[address].data[office].street'), array(self::LEVEL_2, 'address', '[address]', 'street', '[office].street', 'children[address].data[office].street.prop'), array(self::LEVEL_1, 'address', '[address]', 'street', '[office].street', 'children[address].data[office][street]'), @@ -641,10 +670,13 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', 'address', 'street', '[office][street]', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', 'address', 'street', '[office][street]', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', 'address', 'street', '[office][street]', 'children[address].data'), + array(self::LEVEL_1, 'address', 'address', 'street', '[office][street]', 'children[address].data.office'), array(self::LEVEL_1, 'address', 'address', 'street', '[office][street]', 'children[address].data.office.street'), array(self::LEVEL_1, 'address', 'address', 'street', '[office][street]', 'children[address].data.office.street.prop'), array(self::LEVEL_1, 'address', 'address', 'street', '[office][street]', 'children[address].data.office[street]'), array(self::LEVEL_1, 'address', 'address', 'street', '[office][street]', 'children[address].data.office[street].prop'), + array(self::LEVEL_1, 'address', 'address', 'street', '[office][street]', 'children[address].data[office]'), array(self::LEVEL_1, 'address', 'address', 'street', '[office][street]', 'children[address].data[office].street'), array(self::LEVEL_1, 'address', 'address', 'street', '[office][street]', 'children[address].data[office].street.prop'), array(self::LEVEL_2, 'address', 'address', 'street', '[office][street]', 'children[address].data[office][street]'), @@ -668,10 +700,13 @@ public function provideDefaultTests() array(self::LEVEL_2, 'address', '[address]', 'street', '[office][street]', 'children[address].children[street].data'), array(self::LEVEL_2, 'address', '[address]', 'street', '[office][street]', 'children[address].children[street].data.prop'), + array(self::LEVEL_1, 'address', '[address]', 'street', '[office][street]', 'children[address].data'), + array(self::LEVEL_1, 'address', '[address]', 'street', '[office][street]', 'children[address].data.office'), array(self::LEVEL_1, 'address', '[address]', 'street', '[office][street]', 'children[address].data.office.street'), array(self::LEVEL_1, 'address', '[address]', 'street', '[office][street]', 'children[address].data.office.street.prop'), array(self::LEVEL_1, 'address', '[address]', 'street', '[office][street]', 'children[address].data.office[street]'), array(self::LEVEL_1, 'address', '[address]', 'street', '[office][street]', 'children[address].data.office[street].prop'), + array(self::LEVEL_1, 'address', '[address]', 'street', '[office][street]', 'children[address].data[office]'), array(self::LEVEL_1, 'address', '[address]', 'street', '[office][street]', 'children[address].data[office].street'), array(self::LEVEL_1, 'address', '[address]', 'street', '[office][street]', 'children[address].data[office].street.prop'), array(self::LEVEL_2, 'address', '[address]', 'street', '[office][street]', 'children[address].data[office][street]'), diff --git a/Tests/Util/PropertyPathBuilderTest.php b/Tests/Util/PropertyPathBuilderTest.php index db0f18b396..6649bdcdc9 100644 --- a/Tests/Util/PropertyPathBuilderTest.php +++ b/Tests/Util/PropertyPathBuilderTest.php @@ -211,6 +211,19 @@ public function testReplaceSubstringWithLengthGreaterOne() $this->assertEquals($path, $this->builder->getPropertyPath()); } + // https://github.com/symfony/symfony/issues/5605 + public function testReplaceWithLongerPath() + { + // error occurs when path contains at least two more elements + // than the builder + $path = new PropertyPath('new1.new2.new3'); + + $builder = new PropertyPathBuilder(new PropertyPath('old1')); + $builder->replace(0, 1, $path); + + $this->assertEquals($path, $builder->getPropertyPath()); + } + public function testRemove() { $this->builder->remove(3); diff --git a/Util/PropertyPathBuilder.php b/Util/PropertyPathBuilder.php index eac49c2e7f..f17c8b2258 100644 --- a/Util/PropertyPathBuilder.php +++ b/Util/PropertyPathBuilder.php @@ -257,13 +257,23 @@ private function resize($offset, $cutLength, $insertionLength) } } else { $diff = $insertionLength - $cutLength; + $newLength = $length + $diff; $indexAfterInsertion = $offset + $insertionLength; + // $diff <= $insertionLength + // $indexAfterInsertion >= $insertionLength + // => $diff <= $indexAfterInsertion + + // In each of the following loops, $i >= $diff must hold, + // otherwise ($i - $diff) becomes negative. + // Shift old elements to the right to make up space for the // inserted elements. This needs to be done left-to-right in // order to preserve an ascending array index order - for ($i = $length; $i < $newLength; ++$i) { + // Since $i = max($length, $indexAfterInsertion) and $indexAfterInsertion >= $diff, + // $i >= $diff is guaranteed. + for ($i = max($length, $indexAfterInsertion); $i < $newLength; ++$i) { $this->elements[$i] = $this->elements[$i - $diff]; $this->isIndex[$i] = $this->isIndex[$i - $diff]; } @@ -273,6 +283,8 @@ private function resize($offset, $cutLength, $insertionLength) // The last written index is the immediate index after the inserted // string, because the indices before that will be overwritten // anyway. + // Since $i >= $indexAfterInsertion and $indexAfterInsertion >= $diff, + // $i >= $diff is guaranteed. for ($i = $length - 1; $i >= $indexAfterInsertion; --$i) { $this->elements[$i] = $this->elements[$i - $diff]; $this->isIndex[$i] = $this->isIndex[$i - $diff];