From a50cfcb49db0de3bc5bbf2a0d1f26571cc39a23c Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Sun, 3 Jan 2021 12:25:47 +0100 Subject: [PATCH] use bitwise flags to configure when the property accessor should throw --- UPGRADE-5.3.md | 5 ++ UPGRADE-6.0.md | 1 + .../FrameworkExtension.php | 11 +++-- .../Resources/config/property_access.php | 3 +- .../FrameworkExtensionTest.php | 6 +-- .../Bundle/FrameworkBundle/composer.json | 2 +- .../Component/PropertyAccess/CHANGELOG.md | 5 ++ .../PropertyAccess/PropertyAccessor.php | 49 ++++++++++++++++--- .../PropertyAccessorBuilder.php | 12 ++++- .../Tests/PropertyAccessorTest.php | 49 +++++++++++++++---- 10 files changed, 115 insertions(+), 28 deletions(-) diff --git a/UPGRADE-5.3.md b/UPGRADE-5.3.md index 8da8a34e0d472..95f409ca927c1 100644 --- a/UPGRADE-5.3.md +++ b/UPGRADE-5.3.md @@ -67,6 +67,11 @@ PhpunitBridge * Deprecated the `SetUpTearDownTrait` trait, use original methods with "void" return typehint +PropertyAccess +-------------- + +* Deprecate passing a boolean as the second argument of `PropertyAccessor::__construct()`, pass a combination of bitwise flags instead. + PropertyInfo ------------ diff --git a/UPGRADE-6.0.md b/UPGRADE-6.0.md index 5dcfabf494645..e645a500eeed4 100644 --- a/UPGRADE-6.0.md +++ b/UPGRADE-6.0.md @@ -152,6 +152,7 @@ PhpUnitBridge PropertyAccess -------------- + * Drop support for booleans as the second argument of `PropertyAccessor::__construct()`, pass a combination of bitwise flags instead. * Dropped support for booleans as the first argument of `PropertyAccessor::__construct()`. Pass a combination of bitwise flags instead. diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 3ef5e51939e9c..2aacc07ce90d0 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -1573,13 +1573,16 @@ private function registerPropertyAccessConfiguration(array $config, ContainerBui $magicMethods |= $config['magic_get'] ? PropertyAccessor::MAGIC_GET : 0; $magicMethods |= $config['magic_set'] ? PropertyAccessor::MAGIC_SET : 0; + $throw = PropertyAccessor::DO_NOT_THROW; + $throw |= $config['throw_exception_on_invalid_index'] ? PropertyAccessor::THROW_ON_INVALID_INDEX : 0; + $throw |= $config['throw_exception_on_invalid_property_path'] ? PropertyAccessor::THROW_ON_INVALID_PROPERTY_PATH : 0; + $container ->getDefinition('property_accessor') ->replaceArgument(0, $magicMethods) - ->replaceArgument(1, $config['throw_exception_on_invalid_index']) - ->replaceArgument(3, $config['throw_exception_on_invalid_property_path']) - ->replaceArgument(4, new Reference(PropertyReadInfoExtractorInterface::class, ContainerInterface::NULL_ON_INVALID_REFERENCE)) - ->replaceArgument(5, new Reference(PropertyWriteInfoExtractorInterface::class, ContainerInterface::NULL_ON_INVALID_REFERENCE)) + ->replaceArgument(1, $throw) + ->replaceArgument(3, new Reference(PropertyReadInfoExtractorInterface::class, ContainerInterface::NULL_ON_INVALID_REFERENCE)) + ->replaceArgument(4, new Reference(PropertyWriteInfoExtractorInterface::class, ContainerInterface::NULL_ON_INVALID_REFERENCE)) ; } diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/property_access.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/property_access.php index 00d8f66b5afa6..85ab9f18e6e3b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/property_access.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/property_access.php @@ -19,9 +19,8 @@ ->set('property_accessor', PropertyAccessor::class) ->args([ abstract_arg('magic methods allowed, set by the extension'), - abstract_arg('throwExceptionOnInvalidIndex, set by the extension'), + abstract_arg('throw exceptions, set by the extension'), service('cache.property_access')->ignoreOnInvalid(), - abstract_arg('throwExceptionOnInvalidPropertyPath, set by the extension'), abstract_arg('propertyReadInfoExtractor, set by the extension'), abstract_arg('propertyWriteInfoExtractor, set by the extension'), ]) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 83087e5844550..98542f9766394 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -100,8 +100,7 @@ public function testPropertyAccessWithDefaultValue() $def = $container->getDefinition('property_accessor'); $this->assertSame(PropertyAccessor::MAGIC_SET | PropertyAccessor::MAGIC_GET, $def->getArgument(0)); - $this->assertFalse($def->getArgument(1)); - $this->assertTrue($def->getArgument(3)); + $this->assertSame(PropertyAccessor::THROW_ON_INVALID_PROPERTY_PATH, $def->getArgument(1)); } public function testPropertyAccessWithOverriddenValues() @@ -109,8 +108,7 @@ public function testPropertyAccessWithOverriddenValues() $container = $this->createContainerFromFile('property_accessor'); $def = $container->getDefinition('property_accessor'); $this->assertSame(PropertyAccessor::MAGIC_GET | PropertyAccessor::MAGIC_CALL, $def->getArgument(0)); - $this->assertTrue($def->getArgument(1)); - $this->assertFalse($def->getArgument(3)); + $this->assertSame(PropertyAccessor::THROW_ON_INVALID_INDEX, $def->getArgument(1)); } public function testPropertyAccessCache() diff --git a/src/Symfony/Bundle/FrameworkBundle/composer.json b/src/Symfony/Bundle/FrameworkBundle/composer.json index e607e474d6fea..68167b944acf5 100644 --- a/src/Symfony/Bundle/FrameworkBundle/composer.json +++ b/src/Symfony/Bundle/FrameworkBundle/composer.json @@ -84,7 +84,7 @@ "symfony/messenger": "<4.4", "symfony/mime": "<4.4", "symfony/property-info": "<4.4", - "symfony/property-access": "<5.2", + "symfony/property-access": "<5.3", "symfony/serializer": "<5.2", "symfony/security-csrf": "<5.3", "symfony/security-core": "<5.3", diff --git a/src/Symfony/Component/PropertyAccess/CHANGELOG.md b/src/Symfony/Component/PropertyAccess/CHANGELOG.md index ba148d524f610..f90c6f5e7eb36 100644 --- a/src/Symfony/Component/PropertyAccess/CHANGELOG.md +++ b/src/Symfony/Component/PropertyAccess/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +5.3.0 +----- + + * deprecate passing a boolean as the second argument of `PropertyAccessor::__construct()`, expecting a combination of bitwise flags instead + 5.2.0 ----- diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 64b72ab8ce47c..02bcd542f90fd 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -47,6 +47,10 @@ class PropertyAccessor implements PropertyAccessorInterface /** @var int Allow magic __call methods */ public const MAGIC_CALL = ReflectionExtractor::ALLOW_MAGIC_CALL; + public const DO_NOT_THROW = 0; + public const THROW_ON_INVALID_INDEX = 1; + public const THROW_ON_INVALID_PROPERTY_PATH = 2; + private const VALUE = 0; private const REF = 1; private const IS_REF_CHAINED = 2; @@ -82,11 +86,15 @@ class PropertyAccessor implements PropertyAccessorInterface * Should not be used by application code. Use * {@link PropertyAccess::createPropertyAccessor()} instead. * - * @param int $magicMethods A bitwise combination of the MAGIC_* constants - * to specify the allowed magic methods (__get, __set, __call) - * or self::DISALLOW_MAGIC_METHODS for none + * @param int $magicMethods A bitwise combination of the MAGIC_* constants + * to specify the allowed magic methods (__get, __set, __call) + * or self::DISALLOW_MAGIC_METHODS for none + * @param int $throw A bitwise combination of the THROW_* constants + * to specify when exceptions should be thrown + * @param PropertyReadInfoExtractorInterface $readInfoExtractor + * @param PropertyWriteInfoExtractorInterface $writeInfoExtractor */ - 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, /*int */$throw = self::THROW_ON_INVALID_PROPERTY_PATH, CacheItemPoolInterface $cacheItemPool = null, /*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__); @@ -96,10 +104,39 @@ public function __construct(/*int */$magicMethods = self::MAGIC_GET | self::MAGI throw new \TypeError(sprintf('Argument 1 passed to "%s()" must be an integer, "%s" given.', __METHOD__, get_debug_type($readInfoExtractor))); } + if (\is_bool($throw)) { + trigger_deprecation('symfony/property-access', '5.3', 'Passing a boolean as the second argument to "%s()" is deprecated. Pass a combination of bitwise flags instead (i.e an integer).', __METHOD__); + + $throw = $throw ? self::THROW_ON_INVALID_INDEX : self::DO_NOT_THROW; + + if (!\is_bool($readInfoExtractor)) { + $throw |= self::THROW_ON_INVALID_PROPERTY_PATH; + } + } + + if (\is_bool($readInfoExtractor)) { + trigger_deprecation('symfony/property-access', '5.3', 'Passing a boolean as the fourth argument to "%s()" is deprecated. Pass a combination of bitwise flags as the second argument instead (i.e an integer).', __METHOD__); + + if ($readInfoExtractor) { + $throw |= self::THROW_ON_INVALID_PROPERTY_PATH; + } + + $readInfoExtractor = $writeInfoExtractor; + $writeInfoExtractor = 4 < \func_num_args() ? func_get_arg(4) : null; + } + + if (null !== $readInfoExtractor && !$readInfoExtractor instanceof PropertyReadInfoExtractorInterface) { + throw new \TypeError(sprintf('Argument 4 passed to "%s()" must be null or an instance of "%s", "%s" given.', __METHOD__, PropertyReadInfoExtractorInterface::class, get_debug_type($readInfoExtractor))); + } + + if (null !== $writeInfoExtractor && !$writeInfoExtractor instanceof PropertyWriteInfoExtractorInterface) { + throw new \TypeError(sprintf('Argument 5 passed to "%s()" must be null or an instance of "%s", "%s" given.', __METHOD__, PropertyWriteInfoExtractorInterface::class, get_debug_type($writeInfoExtractor))); + } + $this->magicMethodsFlags = $magicMethods; - $this->ignoreInvalidIndices = !$throwExceptionOnInvalidIndex; + $this->ignoreInvalidIndices = 0 === ($throw & self::THROW_ON_INVALID_INDEX); $this->cacheItemPool = $cacheItemPool instanceof NullAdapter ? null : $cacheItemPool; // Replace the NullAdapter by the null value - $this->ignoreInvalidProperty = !$throwExceptionOnInvalidPropertyPath; + $this->ignoreInvalidProperty = 0 === ($throw & self::THROW_ON_INVALID_PROPERTY_PATH); $this->readInfoExtractor = $readInfoExtractor ?? new ReflectionExtractor([], null, null, false); $this->writeInfoExtractor = $writeInfoExtractor ?? new ReflectionExtractor(['set'], null, null, false); } diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessorBuilder.php b/src/Symfony/Component/PropertyAccess/PropertyAccessorBuilder.php index 15beac7cf83e0..0e2acb24f950b 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessorBuilder.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessorBuilder.php @@ -283,6 +283,16 @@ public function getWriteInfoExtractor(): ?PropertyWriteInfoExtractorInterface */ public function getPropertyAccessor() { - return new PropertyAccessor($this->magicMethods, $this->throwExceptionOnInvalidIndex, $this->cacheItemPool, $this->throwExceptionOnInvalidPropertyPath, $this->readInfoExtractor, $this->writeInfoExtractor); + $throw = PropertyAccessor::DO_NOT_THROW; + + if ($this->throwExceptionOnInvalidIndex) { + $throw |= PropertyAccessor::THROW_ON_INVALID_INDEX; + } + + if ($this->throwExceptionOnInvalidPropertyPath) { + $throw |= PropertyAccessor::THROW_ON_INVALID_PROPERTY_PATH; + } + + return new PropertyAccessor($this->magicMethods, $throw, $this->cacheItemPool, $this->readInfoExtractor, $this->writeInfoExtractor); } } diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index 335638553eada..276857c1d727c 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\PropertyAccess\Tests; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Cache\Adapter\ArrayAdapter; use Symfony\Component\PropertyAccess\Exception\AccessException; use Symfony\Component\PropertyAccess\Exception\InvalidArgumentException; @@ -38,6 +39,8 @@ class PropertyAccessorTest extends TestCase { + use ExpectDeprecationTrait; + /** * @var PropertyAccessor */ @@ -115,7 +118,20 @@ public function testGetValueThrowsExceptionIfPropertyNotFound($objectOrArray, $p */ public function testGetValueReturnsNullIfPropertyNotFoundAndExceptionIsDisabled($objectOrArray, $path) { - $this->propertyAccessor = PropertyAccess::createPropertyAccessorBuilder()->disableExceptionOnInvalidPropertyPath()->getPropertyAccessor(); + $this->propertyAccessor = new PropertyAccessor(PropertyAccessor::MAGIC_GET | PropertyAccessor::MAGIC_SET, PropertyAccessor::DO_NOT_THROW); + + $this->assertNull($this->propertyAccessor->getValue($objectOrArray, $path), $path); + } + + /** + * @group legacy + * @dataProvider getPathsWithMissingProperty + */ + public function testGetValueReturnsNullIfPropertyNotFoundAndExceptionIsDisabledUsingBooleanArgument($objectOrArray, $path) + { + $this->expectDeprecation('Since symfony/property-access 5.3: Passing a boolean as the fourth argument to "Symfony\Component\PropertyAccess\PropertyAccessor::__construct()" is deprecated. Pass a combination of bitwise flags as the second argument instead (i.e an integer).'); + + $this->propertyAccessor = new PropertyAccessor(PropertyAccessor::MAGIC_GET | PropertyAccessor::MAGIC_SET, PropertyAccessor::DO_NOT_THROW, null, false); $this->assertNull($this->propertyAccessor->getValue($objectOrArray, $path), $path); } @@ -134,6 +150,19 @@ public function testGetValueThrowsNoExceptionIfIndexNotFound($objectOrArray, $pa public function testGetValueThrowsExceptionIfIndexNotFoundAndIndexExceptionsEnabled($objectOrArray, $path) { $this->expectException(NoSuchIndexException::class); + $this->propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, PropertyAccessor::THROW_ON_INVALID_INDEX | PropertyAccessor::THROW_ON_INVALID_PROPERTY_PATH); + $this->propertyAccessor->getValue($objectOrArray, $path); + } + + /** + * @group legacy + * @dataProvider getPathsWithMissingIndex + */ + public function testGetValueThrowsExceptionIfIndexNotFoundAndIndexExceptionsEnabledUsingBooleanArgument($objectOrArray, $path) + { + $this->expectException(NoSuchIndexException::class); + $this->expectDeprecation('Since symfony/property-access 5.3: Passing a boolean as the second argument to "Symfony\Component\PropertyAccess\PropertyAccessor::__construct()" is deprecated. Pass a combination of bitwise flags instead (i.e an integer).'); + $this->propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, true); $this->propertyAccessor->getValue($objectOrArray, $path); } @@ -256,7 +285,7 @@ public function testGetValueNotModifyObject() public function testGetValueNotModifyObjectException() { - $propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, true); + $propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, PropertyAccessor::THROW_ON_INVALID_INDEX | PropertyAccessor::THROW_ON_INVALID_PROPERTY_PATH); $object = new \stdClass(); $object->firstName = ['Bernhard']; @@ -344,7 +373,7 @@ public function testSetValueThrowsNoExceptionIfIndexNotFound($objectOrArray, $pa */ public function testSetValueThrowsNoExceptionIfIndexNotFoundAndIndexExceptionsEnabled($objectOrArray, $path) { - $this->propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, true); + $this->propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, PropertyAccessor::THROW_ON_INVALID_INDEX | PropertyAccessor::THROW_ON_INVALID_PROPERTY_PATH); $this->propertyAccessor->setValue($objectOrArray, $path, 'Updated'); $this->assertSame('Updated', $this->propertyAccessor->getValue($objectOrArray, $path)); @@ -431,7 +460,7 @@ public function testSetValueThrowsExceptionIfNotObjectOrArray($objectOrArray, $p public function testGetValueWhenArrayValueIsNull() { - $this->propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, true); + $this->propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, PropertyAccessor::THROW_ON_INVALID_INDEX | PropertyAccessor::THROW_ON_INVALID_PROPERTY_PATH); $this->assertNull($this->propertyAccessor->getValue(['index' => ['nullable' => null]], '[index][nullable]')); } @@ -465,7 +494,7 @@ public function testIsReadableReturnsTrueIfIndexNotFound($objectOrArray, $path) */ public function testIsReadableReturnsFalseIfIndexNotFoundAndIndexExceptionsEnabled($objectOrArray, $path) { - $this->propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, true); + $this->propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, PropertyAccessor::THROW_ON_INVALID_INDEX | PropertyAccessor::THROW_ON_INVALID_PROPERTY_PATH); // When exceptions are enabled, non-existing indices cannot be read $this->assertFalse($this->propertyAccessor->isReadable($objectOrArray, $path)); @@ -537,7 +566,7 @@ public function testIsWritableReturnsTrueIfIndexNotFound($objectOrArray, $path) */ public function testIsWritableReturnsTrueIfIndexNotFoundAndIndexExceptionsEnabled($objectOrArray, $path) { - $this->propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, true); + $this->propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, PropertyAccessor::THROW_ON_INVALID_INDEX | PropertyAccessor::THROW_ON_INVALID_PROPERTY_PATH); // Non-existing indices can be written even if exceptions are enabled $this->assertTrue($this->propertyAccessor->isWritable($objectOrArray, $path)); @@ -721,7 +750,7 @@ public function testCacheReadAccess() { $obj = new TestClass('foo'); - $propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, false, new ArrayAdapter()); + $propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, PropertyAccessor::THROW_ON_INVALID_PROPERTY_PATH, new ArrayAdapter()); $this->assertEquals('foo', $propertyAccessor->getValue($obj, 'publicGetSetter')); $propertyAccessor->setValue($obj, 'publicGetSetter', 'bar'); $propertyAccessor->setValue($obj, 'publicGetSetter', 'baz'); @@ -735,7 +764,7 @@ public function testAttributeWithSpecialChars() $obj->{'a/b'} = '1'; $obj->{'a%2Fb'} = '2'; - $propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, false, new ArrayAdapter()); + $propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, PropertyAccessor::THROW_ON_INVALID_PROPERTY_PATH, new ArrayAdapter()); $this->assertSame('bar', $propertyAccessor->getValue($obj, '@foo')); $this->assertSame('1', $propertyAccessor->getValue($obj, 'a/b')); $this->assertSame('2', $propertyAccessor->getValue($obj, 'a%2Fb')); @@ -756,7 +785,7 @@ public function testAnonymousClassRead() $obj = $this->generateAnonymousClass($value); - $propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, false, new ArrayAdapter()); + $propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, PropertyAccessor::THROW_ON_INVALID_PROPERTY_PATH, new ArrayAdapter()); $this->assertEquals($value, $propertyAccessor->getValue($obj, 'foo')); } @@ -784,7 +813,7 @@ public function testAnonymousClassWrite() $obj = $this->generateAnonymousClass(''); - $propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, false, new ArrayAdapter()); + $propertyAccessor = new PropertyAccessor(PropertyAccessor::DISALLOW_MAGIC_METHODS, PropertyAccessor::THROW_ON_INVALID_PROPERTY_PATH, new ArrayAdapter()); $propertyAccessor->setValue($obj, 'foo', $value); $this->assertEquals($value, $propertyAccessor->getValue($obj, 'foo'));