From 1fa22d91aee65a62872662bc90aee955d38b40ee Mon Sep 17 00:00:00 2001 From: Bart van den Burg Date: Sat, 7 Jul 2012 14:39:16 +0200 Subject: [PATCH] [Form] Output a more usable error when PropertyPath has tried to find adders and getters, but failed to find them --- .../Tests/Util/PropertyPathCollectionTest.php | 49 ++++++++++++++++++ .../Component/Form/Util/PropertyPath.php | 50 +++++++------------ 2 files changed, 67 insertions(+), 32 deletions(-) diff --git a/src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php b/src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php index ff8b88f43c..3798d25245 100644 --- a/src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php +++ b/src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Form\Tests\Util; use Symfony\Component\Form\Util\PropertyPath; +use Symfony\Component\Form\Util\FormUtil; class PropertyPathCollectionTest_Car { @@ -72,6 +73,11 @@ class PropertyPathCollectionTest_CarOnlyRemover public function getAxes() {} } +class PropertyPathCollectionTest_CarNoAdderAndRemover +{ + public function getAxes() {} +} + class PropertyPathCollectionTest_CompositeCar { public function getStructure() {} @@ -195,4 +201,47 @@ abstract class PropertyPathCollectionTest extends \PHPUnit_Framework_TestCase $path->setValue($car, $axesAfter); } + + /** + * @dataProvider noAdderRemoverData + */ + public function testNoAdderAndRemoverThrowsSensibleError($path, $message) + { + $car = $this->getMock(__CLASS__ . '_CarNoAdderAndRemover'); + $axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth')); + $axesAfter = $this->getCollection(array(0 => 'first', 1 => 'second', 2 => 'third')); + + try { + $path->setValue($car, $axesAfter); + $this->fail('An expected exception was not thrown!'); + } catch (\Symfony\Component\Form\Exception\InvalidPropertyException $e) { + $this->assertEquals(str_replace("{class}", get_class($car), $message), $e->getMessage()); + } + } + + public function noAdderRemoverData() + { + $data = array(); + + $propertyPath = new PropertyPath('axes'); + $expectedMessage = sprintf( + 'Neither element "axes" nor method "setAxes()" exists in class ' + .'"{class}", nor could adders and removers be found based on the ' + .'guessed singulars: %s (provide a singular by suffixing the ' + .'property path with "|{singular}" to override the guesser)', + implode(', ', (array) $singulars = FormUtil::singularify('Axes')) + ); + $data[] = array($propertyPath, $expectedMessage); + + $propertyPath = new PropertyPath('axes|boo'); + $expectedMessage = sprintf( + 'Neither element "axes" nor method "setAxes()" exists in class ' + .'"{class}", nor could adders and removers be found based on the ' + .'passed singular: %s', + 'boo' + ); + $data[] = array($propertyPath, $expectedMessage); + + return $data; + } } diff --git a/src/Symfony/Component/Form/Util/PropertyPath.php b/src/Symfony/Component/Form/Util/PropertyPath.php index 873d541426..f97a5e343e 100644 --- a/src/Symfony/Component/Form/Util/PropertyPath.php +++ b/src/Symfony/Component/Form/Util/PropertyPath.php @@ -437,6 +437,8 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface */ protected function writeProperty(&$objectOrArray, $property, $singular, $isIndex, $value) { + $adderRemoverError = null; + if ($isIndex) { if (!$objectOrArray instanceof \ArrayAccess && !is_array($objectOrArray)) { throw new InvalidPropertyException(sprintf('Index "%s" cannot be modified in object of type "%s" because it doesn\'t implement \ArrayAccess', $property, get_class($objectOrArray))); @@ -446,8 +448,14 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface } elseif (is_object($objectOrArray)) { $reflClass = new ReflectionClass($objectOrArray); + // The plural form is the last element of the property path + $plural = $this->camelize($this->elements[$this->length - 1]); + + // Any of the two methods is required, but not yet known + $singulars = null !== $singular ? array($singular) : (array) FormUtil::singularify($plural); + if (is_array($value) || $value instanceof Traversable) { - $methods = $this->findAdderAndRemover($reflClass, $singular); + $methods = $this->findAdderAndRemover($reflClass, $singulars); if (null !== $methods) { // At this point the add and remove methods have been found $itemsToAdd = is_object($value) ? clone $value : $value; @@ -480,6 +488,13 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface } return; + } else { + $adderRemoverError = ', nor could adders and removers be found based on the '; + if (null === $singular) { + $adderRemoverError .= 'guessed singulars: '.implode(', ', $singulars).' (provide a singular by suffixing the property path with "|{singular}" to override the guesser)'; + } else { + $adderRemoverError .= 'passed singular: '.$singular; + } } } @@ -503,7 +518,7 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface // needed to support \stdClass instances $objectOrArray->$property = $value; } else { - throw new InvalidPropertyException(sprintf('Neither element "%s" nor method "%s()" exists in class "%s"', $property, $setter, $reflClass->name)); + throw new InvalidPropertyException(sprintf('Neither element "%s" nor method "%s()" exists in class "%s"%s', $property, $setter, $reflClass->name, $adderRemoverError)); } } else { throw new InvalidPropertyException(sprintf('Cannot write property "%s" in an array. Maybe you should write the property path as "[%s]" instead?', $property, $property)); @@ -532,37 +547,8 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface * * @throws InvalidPropertyException If the property does not exist. */ - private function findAdderAndRemover(\ReflectionClass $reflClass, $singular) + private function findAdderAndRemover(\ReflectionClass $reflClass, array $singulars) { - if (null !== $singular) { - $addMethod = 'add' . $this->camelize($singular); - $removeMethod = 'remove' . $this->camelize($singular); - - if (!$this->isAccessible($reflClass, $addMethod, 1)) { - throw new InvalidPropertyException(sprintf( - 'The public method "%s" with exactly one required parameter was not found on class %s', - $addMethod, - $reflClass->name - )); - } - - if (!$this->isAccessible($reflClass, $removeMethod, 1)) { - throw new InvalidPropertyException(sprintf( - 'The public method "%s" with exactly one required parameter was not found on class %s', - $removeMethod, - $reflClass->name - )); - } - - return array($addMethod, $removeMethod); - } - - // The plural form is the last element of the property path - $plural = $this->camelize($this->elements[$this->length - 1]); - - // Any of the two methods is required, but not yet known - $singulars = (array) FormUtil::singularify($plural); - foreach ($singulars as $singular) { $addMethod = 'add' . $singular; $removeMethod = 'remove' . $singular;