diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 5c8c2736a3..3c59f74b84 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -636,14 +636,24 @@ class PropertyAccessor implements PropertyAccessorInterface $access[self::ACCESS_HAS_PROPERTY] = $reflClass->hasProperty($property); $camelized = $this->camelize($property); $singulars = (array) Inflector::singularize($camelized); + $errors = []; if ($useAdderAndRemover) { - $methods = $this->findAdderAndRemover($reflClass, $singulars); + foreach ($this->findAdderAndRemover($reflClass, $singulars) as $methods) { + if (3 === \count($methods)) { + $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_ADDER_AND_REMOVER; + $access[self::ACCESS_ADDER] = $methods[self::ACCESS_ADDER]; + $access[self::ACCESS_REMOVER] = $methods[self::ACCESS_REMOVER]; + break; + } - if (null !== $methods) { - $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_ADDER_AND_REMOVER; - $access[self::ACCESS_ADDER] = $methods[0]; - $access[self::ACCESS_REMOVER] = $methods[1]; + if (isset($methods[self::ACCESS_ADDER])) { + $errors[] = sprintf('The add method "%s" in class "%s" was found, but the corresponding remove method "%s" was not found', $methods['methods'][self::ACCESS_ADDER], $reflClass->name, $methods['methods'][self::ACCESS_REMOVER]); + } + + if (isset($methods[self::ACCESS_REMOVER])) { + $errors[] = sprintf('The remove method "%s" in class "%s" was found, but the corresponding add method "%s" was not found', $methods['methods'][self::ACCESS_REMOVER], $reflClass->name, $methods['methods'][self::ACCESS_ADDER]); + } } } @@ -667,30 +677,69 @@ class PropertyAccessor implements PropertyAccessorInterface // we call the getter and hope the __call do the job $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_MAGIC; $access[self::ACCESS_NAME] = $setter; - } elseif (null !== $methods = $this->findAdderAndRemover($reflClass, $singulars)) { - $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND; - $access[self::ACCESS_NAME] = sprintf( - 'The property "%s" in class "%s" can be defined with the methods "%s()" but '. - 'the new value must be an array or an instance of \Traversable, '. - '"%s" given.', - $property, - $reflClass->name, - implode('()", "', $methods), - \is_object($value) ? \get_class($value) : \gettype($value) - ); } else { - $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND; - $access[self::ACCESS_NAME] = sprintf( - 'Neither the property "%s" nor one of the methods %s"%s()", "%s()", '. - '"__set()" or "__call()" exist and have public access in class "%s".', - $property, - implode('', array_map(function ($singular) { - return '"add'.$singular.'()"/"remove'.$singular.'()", '; - }, $singulars)), - $setter, - $getsetter, - $reflClass->name - ); + foreach ($this->findAdderAndRemover($reflClass, $singulars) as $methods) { + if (3 === \count($methods)) { + $errors[] = sprintf( + 'The property "%s" in class "%s" can be defined with the methods "%s()" but '. + 'the new value must be an array or an instance of \Traversable, '. + '"%s" given.', + $property, + $reflClass->name, + implode('()", "', [$methods[self::ACCESS_ADDER], $methods[self::ACCESS_REMOVER]]), + \is_object($value) ? \get_class($value) : \gettype($value) + ); + } + } + + if (!isset($access[self::ACCESS_NAME])) { + $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND; + + $triedMethods = [ + $setter => 1, + $getsetter => 1, + '__set' => 2, + '__call' => 2, + ]; + + foreach ($singulars as $singular) { + $triedMethods['add'.$singular] = 1; + $triedMethods['remove'.$singular] = 1; + } + + foreach ($triedMethods as $methodName => $parameters) { + if (!$reflClass->hasMethod($methodName)) { + continue; + } + + $method = $reflClass->getMethod($methodName); + + if (!$method->isPublic()) { + $errors[] = sprintf('The method "%s" in class "%s" was found but does not have public access', $methodName, $reflClass->name); + continue; + } + + if ($method->getNumberOfRequiredParameters() > $parameters || $method->getNumberOfParameters() < $parameters) { + $errors[] = sprintf('The method "%s" in class "%s" requires %d arguments, but should accept only %d', $methodName, $reflClass->name, $method->getNumberOfRequiredParameters(), $parameters); + } + } + + if (\count($errors)) { + $access[self::ACCESS_NAME] = implode('. ', $errors).'.'; + } else { + $access[self::ACCESS_NAME] = sprintf( + 'Neither the property "%s" nor one of the methods %s"%s()", "%s()", '. + '"__set()" or "__call()" exist and have public access in class "%s".', + $property, + implode('', array_map(function ($singular) { + return '"add'.$singular.'()"/"remove'.$singular.'()", '; + }, $singulars)), + $setter, + $getsetter, + $reflClass->name + ); + } + } } } @@ -754,13 +803,21 @@ class PropertyAccessor implements PropertyAccessorInterface foreach ($singulars as $singular) { $addMethod = 'add'.$singular; $removeMethod = 'remove'.$singular; + $result = ['methods' => [self::ACCESS_ADDER => $addMethod, self::ACCESS_REMOVER => $removeMethod]]; $addMethodFound = $this->isMethodAccessible($reflClass, $addMethod, 1); + + if ($addMethodFound) { + $result[self::ACCESS_ADDER] = $addMethod; + } + $removeMethodFound = $this->isMethodAccessible($reflClass, $removeMethod, 1); - if ($addMethodFound && $removeMethodFound) { - return [$addMethod, $removeMethod]; + if ($removeMethodFound) { + $result[self::ACCESS_REMOVER] = $removeMethod; } + + yield $result; } } diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestAdderRemoverInvalidArgumentLength.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestAdderRemoverInvalidArgumentLength.php new file mode 100644 index 0000000000..4676bbcafe --- /dev/null +++ b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestAdderRemoverInvalidArgumentLength.php @@ -0,0 +1,27 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\PropertyAccess\Tests\Fixtures; + +class TestAdderRemoverInvalidArgumentLength +{ + public function addFoo() + { + } + + public function removeFoo($var1, $var2) + { + } + + public function setBar($var1, $var2) + { + } +} diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestAdderRemoverInvalidMethods.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestAdderRemoverInvalidMethods.php new file mode 100644 index 0000000000..5c23f8b188 --- /dev/null +++ b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestAdderRemoverInvalidMethods.php @@ -0,0 +1,23 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\PropertyAccess\Tests\Fixtures; + +class TestAdderRemoverInvalidMethods +{ + public function addFoo($foo) + { + } + + public function removeBar($foo) + { + } +} diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassSetValue.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassSetValue.php index f0a7f1f47c..9161f120ff 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassSetValue.php +++ b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassSetValue.php @@ -29,4 +29,8 @@ class TestClassSetValue { $this->value = $value; } + + private function setFoo() + { + } } diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php index b91d1e62eb..2593f61118 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php @@ -189,7 +189,7 @@ abstract class PropertyAccessorCollectionTest extends PropertyAccessorArrayAcces /** * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException - * expectedExceptionMessageRegExp /The property "axes" in class "Mock_PropertyAccessorCollectionTest_Car[^"]*" can be defined with the methods "addAxis()", "removeAxis()" but the new value must be an array or an instance of \Traversable, "string" given./ + * @expectedExceptionMessageRegExp /Could not determine access type for property "axes" in class "Mock_PropertyAccessorCollectionTest_Car[^"]*": The property "axes" in class "Mock_PropertyAccessorCollectionTest_Car[^"]*" can be defined with the methods "addAxis\(\)", "removeAxis\(\)" but the new value must be an array or an instance of \\Traversable, "string" given./ */ public function testSetValueFailsIfAdderAndRemoverExistButValueIsNotTraversable() { diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index d0cbccf1ec..dfd16d9f51 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -17,6 +17,8 @@ use Symfony\Component\PropertyAccess\Exception\NoSuchIndexException; use Symfony\Component\PropertyAccess\PropertyAccess; use Symfony\Component\PropertyAccess\PropertyAccessor; use Symfony\Component\PropertyAccess\Tests\Fixtures\ReturnTyped; +use Symfony\Component\PropertyAccess\Tests\Fixtures\TestAdderRemoverInvalidArgumentLength; +use Symfony\Component\PropertyAccess\Tests\Fixtures\TestAdderRemoverInvalidMethods; use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClass; use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassIsWritable; use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassMagicCall; @@ -762,4 +764,54 @@ class PropertyAccessorTest extends TestCase $this->assertEquals(['aeroplane'], $object->getAircraft()); } + + /** + * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException + * @expectedExceptionMessageRegExp /.*The add method "addFoo" in class "Symfony\\Component\\PropertyAccess\\Tests\\Fixtures\\TestAdderRemoverInvalidMethods" was found, but the corresponding remove method "removeFoo" was not found\./ + */ + public function testAdderWithoutRemover() + { + $object = new TestAdderRemoverInvalidMethods(); + $this->propertyAccessor->setValue($object, 'foos', [1, 2]); + } + + /** + * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException + * @expectedExceptionMessageRegExp /.*The remove method "removeBar" in class "Symfony\\Component\\PropertyAccess\\Tests\\Fixtures\\TestAdderRemoverInvalidMethods" was found, but the corresponding add method "addBar" was not found\./ + */ + public function testRemoverWithoutAdder() + { + $object = new TestAdderRemoverInvalidMethods(); + $this->propertyAccessor->setValue($object, 'bars', [1, 2]); + } + + /** + * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException + * @expectedExceptionMessageRegExp /.*The method "addFoo" in class "Symfony\\Component\\PropertyAccess\\Tests\\Fixtures\\TestAdderRemoverInvalidArgumentLength" requires 0 arguments, but should accept only 1\. The method "removeFoo" in class "Symfony\\Component\\PropertyAccess\\Tests\\Fixtures\\TestAdderRemoverInvalidArgumentLength" requires 2 arguments, but should accept only 1\./ + */ + public function testAdderAndRemoveNeedsTheExactParametersDefined() + { + $object = new TestAdderRemoverInvalidArgumentLength(); + $this->propertyAccessor->setValue($object, 'foo', [1, 2]); + } + + /** + * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException + * @expectedExceptionMessageRegExp /.*The method "setBar" in class "Symfony\\Component\\PropertyAccess\\Tests\\Fixtures\\TestAdderRemoverInvalidArgumentLength" requires 2 arguments, but should accept only 1\./ + */ + public function testSetterNeedsTheExactParametersDefined() + { + $object = new TestAdderRemoverInvalidArgumentLength(); + $this->propertyAccessor->setValue($object, 'bar', [1, 2]); + } + + /** + * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException + * @expectedExceptionMessageRegExp /.*The method "setFoo" in class "Symfony\\Component\\PropertyAccess\\Tests\\Fixtures\\TestClassSetValue" was found but does not have public access./ + */ + public function testSetterNeedsPublicAccess() + { + $object = new TestClassSetValue(0); + $this->propertyAccessor->setValue($object, 'foo', 1); + } }