Skip to content

Commit f90a9fd

Browse files
pierredupfabpot
pierredup
authored and
fabpot
committed
Improve errors when trying to find a writable property
1 parent 95e8a65 commit f90a9fd

File tree

6 files changed

+194
-31
lines changed

6 files changed

+194
-31
lines changed

‎src/Symfony/Component/PropertyAccess/PropertyAccessor.php

Lines changed: 87 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -636,14 +636,24 @@ private function getWriteAccessInfo(string $class, string $property, $value): ar
636636
$access[self::ACCESS_HAS_PROPERTY] = $reflClass->hasProperty($property);
637637
$camelized = $this->camelize($property);
638638
$singulars = (array) Inflector::singularize($camelized);
639+
$errors = [];
639640

640641
if ($useAdderAndRemover) {
641-
$methods = $this->findAdderAndRemover($reflClass, $singulars);
642+
foreach ($this->findAdderAndRemover($reflClass, $singulars) as $methods) {
643+
if (3 === \count($methods)) {
644+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_ADDER_AND_REMOVER;
645+
$access[self::ACCESS_ADDER] = $methods[self::ACCESS_ADDER];
646+
$access[self::ACCESS_REMOVER] = $methods[self::ACCESS_REMOVER];
647+
break;
648+
}
649+
650+
if (isset($methods[self::ACCESS_ADDER])) {
651+
$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]);
652+
}
642653

643-
if (null !== $methods) {
644-
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_ADDER_AND_REMOVER;
645-
$access[self::ACCESS_ADDER] = $methods[0];
646-
$access[self::ACCESS_REMOVER] = $methods[1];
654+
if (isset($methods[self::ACCESS_REMOVER])) {
655+
$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]);
656+
}
647657
}
648658
}
649659

@@ -667,30 +677,69 @@ private function getWriteAccessInfo(string $class, string $property, $value): ar
667677
// we call the getter and hope the __call do the job
668678
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_MAGIC;
669679
$access[self::ACCESS_NAME] = $setter;
670-
} elseif (null !== $methods = $this->findAdderAndRemover($reflClass, $singulars)) {
671-
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND;
672-
$access[self::ACCESS_NAME] = sprintf(
673-
'The property "%s" in class "%s" can be defined with the methods "%s()" but '.
674-
'the new value must be an array or an instance of \Traversable, '.
675-
'"%s" given.',
676-
$property,
677-
$reflClass->name,
678-
implode('()", "', $methods),
679-
\is_object($value) ? \get_class($value) : \gettype($value)
680-
);
681680
} else {
682-
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND;
683-
$access[self::ACCESS_NAME] = sprintf(
684-
'Neither the property "%s" nor one of the methods %s"%s()", "%s()", '.
685-
'"__set()" or "__call()" exist and have public access in class "%s".',
686-
$property,
687-
implode('', array_map(function ($singular) {
688-
return '"add'.$singular.'()"/"remove'.$singular.'()", ';
689-
}, $singulars)),
690-
$setter,
691-
$getsetter,
692-
$reflClass->name
693-
);
681+
foreach ($this->findAdderAndRemover($reflClass, $singulars) as $methods) {
682+
if (3 === \count($methods)) {
683+
$errors[] = sprintf(
684+
'The property "%s" in class "%s" can be defined with the methods "%s()" but '.
685+
'the new value must be an array or an instance of \Traversable, '.
686+
'"%s" given.',
687+
$property,
688+
$reflClass->name,
689+
implode('()", "', [$methods[self::ACCESS_ADDER], $methods[self::ACCESS_REMOVER]]),
690+
\is_object($value) ? \get_class($value) : \gettype($value)
691+
);
692+
}
693+
}
694+
695+
if (!isset($access[self::ACCESS_NAME])) {
696+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND;
697+
698+
$triedMethods = [
699+
$setter => 1,
700+
$getsetter => 1,
701+
'__set' => 2,
702+
'__call' => 2,
703+
];
704+
705+
foreach ($singulars as $singular) {
706+
$triedMethods['add'.$singular] = 1;
707+
$triedMethods['remove'.$singular] = 1;
708+
}
709+
710+
foreach ($triedMethods as $methodName => $parameters) {
711+
if (!$reflClass->hasMethod($methodName)) {
712+
continue;
713+
}
714+
715+
$method = $reflClass->getMethod($methodName);
716+
717+
if (!$method->isPublic()) {
718+
$errors[] = sprintf('The method "%s" in class "%s" was found but does not have public access', $methodName, $reflClass->name);
719+
continue;
720+
}
721+
722+
if ($method->getNumberOfRequiredParameters() > $parameters || $method->getNumberOfParameters() < $parameters) {
723+
$errors[] = sprintf('The method "%s" in class "%s" requires %d arguments, but should accept only %d', $methodName, $reflClass->name, $method->getNumberOfRequiredParameters(), $parameters);
724+
}
725+
}
726+
727+
if (\count($errors)) {
728+
$access[self::ACCESS_NAME] = implode('. ', $errors).'.';
729+
} else {
730+
$access[self::ACCESS_NAME] = sprintf(
731+
'Neither the property "%s" nor one of the methods %s"%s()", "%s()", '.
732+
'"__set()" or "__call()" exist and have public access in class "%s".',
733+
$property,
734+
implode('', array_map(function ($singular) {
735+
return '"add'.$singular.'()"/"remove'.$singular.'()", ';
736+
}, $singulars)),
737+
$setter,
738+
$getsetter,
739+
$reflClass->name
740+
);
741+
}
742+
}
694743
}
695744
}
696745

@@ -754,13 +803,21 @@ private function findAdderAndRemover(\ReflectionClass $reflClass, array $singula
754803
foreach ($singulars as $singular) {
755804
$addMethod = 'add'.$singular;
756805
$removeMethod = 'remove'.$singular;
806+
$result = ['methods' => [self::ACCESS_ADDER => $addMethod, self::ACCESS_REMOVER => $removeMethod]];
757807

758808
$addMethodFound = $this->isMethodAccessible($reflClass, $addMethod, 1);
809+
810+
if ($addMethodFound) {
811+
$result[self::ACCESS_ADDER] = $addMethod;
812+
}
813+
759814
$removeMethodFound = $this->isMethodAccessible($reflClass, $removeMethod, 1);
760815

761-
if ($addMethodFound && $removeMethodFound) {
762-
return [$addMethod, $removeMethod];
816+
if ($removeMethodFound) {
817+
$result[self::ACCESS_REMOVER] = $removeMethod;
763818
}
819+
820+
yield $result;
764821
}
765822
}
766823

‎src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestAdderRemoverInvalidArgumentLength.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\PropertyAccess\Tests\Fixtures;
13+
14+
class TestAdderRemoverInvalidArgumentLength
15+
{
16+
public function addFoo()
17+
{
18+
}
19+
20+
public function removeFoo($var1, $var2)
21+
{
22+
}
23+
24+
public function setBar($var1, $var2)
25+
{
26+
}
27+
}

‎src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestAdderRemoverInvalidMethods.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\PropertyAccess\Tests\Fixtures;
13+
14+
class TestAdderRemoverInvalidMethods
15+
{
16+
public function addFoo($foo)
17+
{
18+
}
19+
20+
public function removeBar($foo)
21+
{
22+
}
23+
}

‎src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassSetValue.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,8 @@ public function __construct($value)
2929
{
3030
$this->value = $value;
3131
}
32+
33+
private function setFoo()
34+
{
35+
}
3236
}

‎src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ public function testIsWritableReturnsFalseIfNoAdderNorRemoverExists()
189189

190190
/**
191191
* @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException
192-
* 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./
192+
* @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./
193193
*/
194194
public function testSetValueFailsIfAdderAndRemoverExistButValueIsNotTraversable()
195195
{

‎src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
use Symfony\Component\PropertyAccess\PropertyAccess;
1818
use Symfony\Component\PropertyAccess\PropertyAccessor;
1919
use Symfony\Component\PropertyAccess\Tests\Fixtures\ReturnTyped;
20+
use Symfony\Component\PropertyAccess\Tests\Fixtures\TestAdderRemoverInvalidArgumentLength;
21+
use Symfony\Component\PropertyAccess\Tests\Fixtures\TestAdderRemoverInvalidMethods;
2022
use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClass;
2123
use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassIsWritable;
2224
use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassMagicCall;
@@ -762,4 +764,54 @@ public function testAdderAndRemoverArePreferredOverSetterForSameSingularAndPlura
762764

763765
$this->assertEquals(['aeroplane'], $object->getAircraft());
764766
}
767+
768+
/**
769+
* @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException
770+
* @expectedExceptionMessageRegExp /.*The add method "addFoo" in class "Symfony\\Component\\PropertyAccess\\Tests\\Fixtures\\TestAdderRemoverInvalidMethods" was found, but the corresponding remove method "removeFoo" was not found\./
771+
*/
772+
public function testAdderWithoutRemover()
773+
{
774+
$object = new TestAdderRemoverInvalidMethods();
775+
$this->propertyAccessor->setValue($object, 'foos', [1, 2]);
776+
}
777+
778+
/**
779+
* @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException
780+
* @expectedExceptionMessageRegExp /.*The remove method "removeBar" in class "Symfony\\Component\\PropertyAccess\\Tests\\Fixtures\\TestAdderRemoverInvalidMethods" was found, but the corresponding add method "addBar" was not found\./
781+
*/
782+
public function testRemoverWithoutAdder()
783+
{
784+
$object = new TestAdderRemoverInvalidMethods();
785+
$this->propertyAccessor->setValue($object, 'bars', [1, 2]);
786+
}
787+
788+
/**
789+
* @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException
790+
* @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\./
791+
*/
792+
public function testAdderAndRemoveNeedsTheExactParametersDefined()
793+
{
794+
$object = new TestAdderRemoverInvalidArgumentLength();
795+
$this->propertyAccessor->setValue($object, 'foo', [1, 2]);
796+
}
797+
798+
/**
799+
* @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException
800+
* @expectedExceptionMessageRegExp /.*The method "setBar" in class "Symfony\\Component\\PropertyAccess\\Tests\\Fixtures\\TestAdderRemoverInvalidArgumentLength" requires 2 arguments, but should accept only 1\./
801+
*/
802+
public function testSetterNeedsTheExactParametersDefined()
803+
{
804+
$object = new TestAdderRemoverInvalidArgumentLength();
805+
$this->propertyAccessor->setValue($object, 'bar', [1, 2]);
806+
}
807+
808+
/**
809+
* @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException
810+
* @expectedExceptionMessageRegExp /.*The method "setFoo" in class "Symfony\\Component\\PropertyAccess\\Tests\\Fixtures\\TestClassSetValue" was found but does not have public access./
811+
*/
812+
public function testSetterNeedsPublicAccess()
813+
{
814+
$object = new TestClassSetValue(0);
815+
$this->propertyAccessor->setValue($object, 'foo', 1);
816+
}
765817
}

0 commit comments

Comments
 (0)