From 756b9bc38e658a297bd45505f867faa6fca5c2e0 Mon Sep 17 00:00:00 2001 From: Nyholm Date: Sat, 31 Oct 2020 11:39:23 +0100 Subject: [PATCH 1/3] PropertyInfo dont try to set null when argument is not nullable --- src/Symfony/Component/PropertyAccess/PropertyAccessor.php | 1 + .../PropertyInfo/Extractor/ReflectionExtractor.php | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 64b72ab8ce47c..71df5449c3880 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -612,6 +612,7 @@ private function getWriteInfo(string $class, string $property, $value): Property 'enable_magic_methods_extraction' => $this->magicMethodsFlags, 'enable_constructor_extraction' => false, 'enable_adder_remover_extraction' => $useAdderAndRemover, + 'value' => $value, ]); if (isset($item)) { diff --git a/src/Symfony/Component/PropertyInfo/Extractor/ReflectionExtractor.php b/src/Symfony/Component/PropertyInfo/Extractor/ReflectionExtractor.php index eb688469edaba..31b0cab1aafbd 100644 --- a/src/Symfony/Component/PropertyInfo/Extractor/ReflectionExtractor.php +++ b/src/Symfony/Component/PropertyInfo/Extractor/ReflectionExtractor.php @@ -388,6 +388,12 @@ public function getWriteInfo(string $class, string $property, array $context = [ } $method = $reflClass->getMethod($methodName); + /** @var \ReflectionParameter $parameter */ + $parameter = $method->getParameters()[0]; + if (\array_key_exists('value', $context) && null === $context['value'] && !$parameter->allowsNull()) { + $errors[] = sprintf('The method "%s" in class "%s" was found but does not allow null.', $methodName, $class); + continue; + } if (!\in_array($mutatorPrefix, $this->arrayMutatorPrefixes, true)) { return new PropertyWriteInfo(PropertyWriteInfo::TYPE_METHOD, $methodName, $this->getWriteVisiblityForMethod($method), $method->isStatic()); From 29020ba768713e3f07c3f08a101be7324e121809 Mon Sep 17 00:00:00 2001 From: Nyholm Date: Mon, 15 Feb 2021 17:38:58 +0100 Subject: [PATCH 2/3] Added tests --- .../Extractor/ReflectionExtractorTest.php | 53 +++++++++++++++++++ .../PropertyInfo/Tests/Fixtures/Dummy.php | 8 +++ 2 files changed, 61 insertions(+) diff --git a/src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php b/src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php index 0e5193a33deb6..db1b06558964f 100644 --- a/src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php +++ b/src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php @@ -20,6 +20,7 @@ use Symfony\Component\PropertyInfo\Tests\Fixtures\DefaultValue; use Symfony\Component\PropertyInfo\Tests\Fixtures\Dummy; use Symfony\Component\PropertyInfo\Tests\Fixtures\NotInstantiable; +use Symfony\Component\PropertyInfo\Tests\Fixtures\ParentDummy; use Symfony\Component\PropertyInfo\Tests\Fixtures\Php71Dummy; use Symfony\Component\PropertyInfo\Tests\Fixtures\Php71DummyExtended; use Symfony\Component\PropertyInfo\Tests\Fixtures\Php71DummyExtended2; @@ -581,4 +582,56 @@ public function extractConstructorTypesProvider(): array ['ddd', null], ]; } + + public function testGetWriteInfoForSettersWithNullableParameter() + { + $extractor = new ReflectionExtractor(['set']); + $writeInfo = $extractor->getWriteInfo(Dummy::class, 'optionalDate', [ + 'value' => new \DateTime(), + ]); + $this->assertSame('method', $writeInfo->getType()); + $this->assertEmpty($writeInfo->getErrors()); + + // If the key "value" exists and is null, we should still get the method + $writeInfo = $extractor->getWriteInfo(Dummy::class, 'optionalDate', [ + 'value' => null, + ]); + $this->assertSame('method', $writeInfo->getType()); + $this->assertEmpty($writeInfo->getErrors()); + + // If we dont pass a key "value", we should behave as if the value exists. + $writeInfo = $extractor->getWriteInfo(Dummy::class, 'optionalDate', []); + $this->assertSame('method', $writeInfo->getType()); + $this->assertEmpty($writeInfo->getErrors()); + } + + public function testGetWriteInfoForSettersWithNonNullableParameter() + { + $extractor = new ReflectionExtractor(['set']); + $writeInfo = $extractor->getWriteInfo(Dummy::class, 'date', [ + 'value' => new \DateTime(), + ]); + $this->assertSame('method', $writeInfo->getType()); + $this->assertEmpty($writeInfo->getErrors()); + + // If the key "value" exists and is null, then make sure there is an error when parameter is required + $writeInfo = $extractor->getWriteInfo(Dummy::class, 'date', [ + 'value' => null, + ]); + $this->assertSame('none', $writeInfo->getType()); + $this->assertTrue(in_array('The method "setDate" in class "Symfony\Component\PropertyInfo\Tests\Fixtures\Dummy" was found but does not allow null.', $writeInfo->getErrors())); + + // If we dont pass a key "value", we should behave as if the value exists. + $writeInfo = $extractor->getWriteInfo(Dummy::class, 'date', []); + $this->assertSame('method', $writeInfo->getType()); + $this->assertEmpty($writeInfo->getErrors()); + } + + public function testGetWriteInfoForSettersWithNoParameter() + { + $extractor = new ReflectionExtractor(['add']); + $writeInfo = $extractor->getWriteInfo(Dummy::class, 'nothing'); + $this->assertSame('none', $writeInfo->getType()); + $this->assertNotEmpty($writeInfo->getErrors()); + } } diff --git a/src/Symfony/Component/PropertyInfo/Tests/Fixtures/Dummy.php b/src/Symfony/Component/PropertyInfo/Tests/Fixtures/Dummy.php index 0ec61c081c733..b90b19ae17bf7 100644 --- a/src/Symfony/Component/PropertyInfo/Tests/Fixtures/Dummy.php +++ b/src/Symfony/Component/PropertyInfo/Tests/Fixtures/Dummy.php @@ -237,4 +237,12 @@ public function addDate(\DateTime $date) public function hasElement(string $element): bool { } + + public function setOptionalDate(\DateTime $date = null) + { + } + + public function addNothing() + { + } } From 6677aa79a114499487f4ee2c2937048d325b1797 Mon Sep 17 00:00:00 2001 From: Nyholm Date: Mon, 15 Feb 2021 17:41:25 +0100 Subject: [PATCH 3/3] cs --- src/Symfony/Component/PropertyAccess/PropertyAccessor.php | 2 +- .../PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 71df5449c3880..0ae709765c21d 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -86,7 +86,7 @@ class PropertyAccessor implements PropertyAccessorInterface * to specify the allowed magic methods (__get, __set, __call) * or self::DISALLOW_MAGIC_METHODS for none */ - public function __construct(/*int */$magicMethods = self::MAGIC_GET | self::MAGIC_SET, bool $throwExceptionOnInvalidIndex = false, CacheItemPoolInterface $cacheItemPool = null, bool $throwExceptionOnInvalidPropertyPath = true, PropertyReadInfoExtractorInterface $readInfoExtractor = null, PropertyWriteInfoExtractorInterface $writeInfoExtractor = null) + public function __construct(/*int */ $magicMethods = self::MAGIC_GET | self::MAGIC_SET, bool $throwExceptionOnInvalidIndex = false, CacheItemPoolInterface $cacheItemPool = null, bool $throwExceptionOnInvalidPropertyPath = true, PropertyReadInfoExtractorInterface $readInfoExtractor = null, PropertyWriteInfoExtractorInterface $writeInfoExtractor = null) { if (\is_bool($magicMethods)) { trigger_deprecation('symfony/property-access', '5.2', 'Passing a boolean as the first argument to "%s()" is deprecated. Pass a combination of bitwise flags instead (i.e an integer).', __METHOD__); diff --git a/src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php b/src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php index db1b06558964f..49fe6867bc0f2 100644 --- a/src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php +++ b/src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php @@ -20,7 +20,6 @@ use Symfony\Component\PropertyInfo\Tests\Fixtures\DefaultValue; use Symfony\Component\PropertyInfo\Tests\Fixtures\Dummy; use Symfony\Component\PropertyInfo\Tests\Fixtures\NotInstantiable; -use Symfony\Component\PropertyInfo\Tests\Fixtures\ParentDummy; use Symfony\Component\PropertyInfo\Tests\Fixtures\Php71Dummy; use Symfony\Component\PropertyInfo\Tests\Fixtures\Php71DummyExtended; use Symfony\Component\PropertyInfo\Tests\Fixtures\Php71DummyExtended2; @@ -619,7 +618,7 @@ public function testGetWriteInfoForSettersWithNonNullableParameter() 'value' => null, ]); $this->assertSame('none', $writeInfo->getType()); - $this->assertTrue(in_array('The method "setDate" in class "Symfony\Component\PropertyInfo\Tests\Fixtures\Dummy" was found but does not allow null.', $writeInfo->getErrors())); + $this->assertTrue(\in_array('The method "setDate" in class "Symfony\Component\PropertyInfo\Tests\Fixtures\Dummy" was found but does not allow null.', $writeInfo->getErrors())); // If we dont pass a key "value", we should behave as if the value exists. $writeInfo = $extractor->getWriteInfo(Dummy::class, 'date', []);