From e0c8aba8297513ffa9635f43a6b888b99baacb85 Mon Sep 17 00:00:00 2001 From: Johan DESMYTER Date: Mon, 9 May 2016 19:57:57 +0300 Subject: [PATCH 1/5] [PropertyAccess] dedicate exception if adder/remover exists but value is not an array --- .../Component/PropertyAccess/PropertyAccessor.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index a1ff632231dd2..bcf02f7fcecbe 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -714,6 +714,19 @@ private function getWriteAccessInfo($class, $property, $value) // 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 !== $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(' and ', array_map(function($method){ + return '"'.$method.'()"'; + }, $this->findAdderAndRemover($reflClass, $singulars))), + is_object($value) ? get_class($value) : gettype($value) + ); } else { $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND; $access[self::ACCESS_NAME] = sprintf( From 068bc66d7126f702971d5052702076132cba6539 Mon Sep 17 00:00:00 2001 From: Johan DESMYTER Date: Tue, 10 May 2016 09:27:07 +0300 Subject: [PATCH 2/5] add test to prevent future regression --- .../Tests/PropertyAccessorCollectionTest.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php index be8aba80d3d88..b0f0d4f5c0ef6 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php @@ -194,4 +194,15 @@ public function testIsWritableReturnsFalseIfNoAdderNorRemoverExists() $this->assertFalse($this->propertyAccessor->isWritable($car, 'axes', $axes)); } + + /** + * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException + * expectedExceptionMessageRegExp /The property "axes" in class "Mock_PropertyAccessorCollectionTest_Car[^"]*" can be defined with the methods "addAxis()" and "removeAxis()" but the new value must be an array or an instance of \Traversable, "string" given./ + */ + public function testSetValueFailsIfAdderAndRemoverExistButValueIsNotTraversable() + { + $car = $this->getMock(__CLASS__.'_Car'); + + $this->propertyAccessor->setValue($car, 'axes', 'Not an array or Traversable'); + } } From c98c27ccdc2ce7fa35466649eec836c2177323a1 Mon Sep 17 00:00:00 2001 From: Johan DESMYTER Date: Tue, 10 May 2016 09:36:32 +0300 Subject: [PATCH 3/5] fix coding standard --- .../PropertyAccess/Tests/PropertyAccessorCollectionTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php index b0f0d4f5c0ef6..79e933b1f2374 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php @@ -194,7 +194,7 @@ public function testIsWritableReturnsFalseIfNoAdderNorRemoverExists() $this->assertFalse($this->propertyAccessor->isWritable($car, 'axes', $axes)); } - + /** * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException * expectedExceptionMessageRegExp /The property "axes" in class "Mock_PropertyAccessorCollectionTest_Car[^"]*" can be defined with the methods "addAxis()" and "removeAxis()" but the new value must be an array or an instance of \Traversable, "string" given./ @@ -202,7 +202,7 @@ public function testIsWritableReturnsFalseIfNoAdderNorRemoverExists() public function testSetValueFailsIfAdderAndRemoverExistButValueIsNotTraversable() { $car = $this->getMock(__CLASS__.'_Car'); - + $this->propertyAccessor->setValue($car, 'axes', 'Not an array or Traversable'); } } From e7200ed2922b300faf21c60ac49fa49c02684bed Mon Sep 17 00:00:00 2001 From: Johan DESMYTER Date: Mon, 16 May 2016 08:47:12 +0300 Subject: [PATCH 4/5] fix psr2 closure --- src/Symfony/Component/PropertyAccess/PropertyAccessor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index bcf02f7fcecbe..9195b3f34a1d5 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -722,7 +722,7 @@ private function getWriteAccessInfo($class, $property, $value) '"%s" given.', $property, $reflClass->name, - implode(' and ', array_map(function($method){ + implode(' and ', array_map(function ($method) { return '"'.$method.'()"'; }, $this->findAdderAndRemover($reflClass, $singulars))), is_object($value) ? get_class($value) : gettype($value) From 6b22d1ef14ca88dfafa32459d91f8b31eda9f671 Mon Sep 17 00:00:00 2001 From: Johan DESMYTER Date: Wed, 25 May 2016 20:35:07 +0300 Subject: [PATCH 5/5] simplification of exception generation --- src/Symfony/Component/PropertyAccess/PropertyAccessor.php | 8 +++----- .../Tests/PropertyAccessorCollectionTest.php | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 9195b3f34a1d5..23fc68e33d708 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -714,17 +714,15 @@ private function getWriteAccessInfo($class, $property, $value) // 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 !== $this->findAdderAndRemover($reflClass, $singulars)) { + } 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 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(' and ', array_map(function ($method) { - return '"'.$method.'()"'; - }, $this->findAdderAndRemover($reflClass, $singulars))), + implode('()", "', $methods), is_object($value) ? get_class($value) : gettype($value) ); } else { diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php index 79e933b1f2374..17518468ebad8 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php @@ -197,7 +197,7 @@ public function testIsWritableReturnsFalseIfNoAdderNorRemoverExists() /** * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException - * expectedExceptionMessageRegExp /The property "axes" in class "Mock_PropertyAccessorCollectionTest_Car[^"]*" can be defined with the methods "addAxis()" and "removeAxis()" but the new value must be an array or an instance of \Traversable, "string" given./ + * 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./ */ public function testSetValueFailsIfAdderAndRemoverExistButValueIsNotTraversable() {