[PropertyAccess] Allow to disable magic __get & __set #32133
Conversation
|
Rebased to target 5.1 |
|
@fabpot No problem, done. |
| /** @var int Allow none of the magic methods */ | ||
| public const DISALLOW_MAGIC_METHODS = ReflectionExtractor::DISALLOW_MAGIC_METHODS; | ||
| /** @var int Allow magic __get methods */ | ||
| public const MAGIC_GET = ReflectionExtractor::ALLOW_MAGIC_GET; |
There was a problem hiding this comment.
These are now aliases to the ReflectionExtractor constants, since it would be unexpected/not-discoverable for the user to use ReflectionExtractor consts to configure the accessor.
src/Symfony/Component/PropertyInfo/Extractor/ReflectionExtractor.php
Outdated
Show resolved
Hide resolved
| $allowMagicSet = (bool) ($magicMethods & self::ALLOW_MAGIC_SET); | ||
|
|
||
| if (isset($context['enable_magic_call_extraction'])) { | ||
| trigger_deprecation('symfony/property-info', '5.2', 'Using the "enable_magic_call_extraction" context option in "%s()" is deprecated. Use "enable_magic_methods_extraction" instead.', __METHOD__); |
There was a problem hiding this comment.
Other options are keeping this key and have enable_magic_get_extraction / enable_magic_set_extraction context keys as well and either:
- have 3 new constructor arguments for the default
- or keep a single constructor arguments with flags (but still have dedicated context keys).
| @@ -73,14 +83,15 @@ class ReflectionExtractor implements PropertyListExtractorInterface, PropertyTyp | |||
| * @param string[]|null $accessorPrefixes | |||
| * @param string[]|null $arrayMutatorPrefixes | |||
| */ | |||
| public function __construct(array $mutatorPrefixes = null, array $accessorPrefixes = null, array $arrayMutatorPrefixes = null, bool $enableConstructorExtraction = true, int $accessFlags = self::ALLOW_PUBLIC, InflectorInterface $inflector = null) | |||
| public function __construct(array $mutatorPrefixes = null, array $accessorPrefixes = null, array $arrayMutatorPrefixes = null, bool $enableConstructorExtraction = true, int $accessFlags = self::ALLOW_PUBLIC, InflectorInterface $inflector = null, int $magicMethodsFlags = self::ALLOW_MAGIC_GET | self::ALLOW_MAGIC_SET) | |||
There was a problem hiding this comment.
The magicMethodsFlags defaults are configured here for BC with 5.1, but before 5.1, magic get and set weren't supported, right?
So should it have been considered a BC break to enable them by default on 5.1?
There was a problem hiding this comment.
@ogizanagi Yes, this looks like a BC break to me. I think we need to set the default value to ReflectionExtractor::DISALLOW_MAGIC_METHODS. We can consider deprecating this default value, but I am not convinced that's really necessary.
…zanagi) This PR was merged into the 5.1 branch. Discussion ---------- [PropertyInfo] Fix ReflectionExtractor + minor tweaks | Q | A | ------------- | --- | Branch? | 5.1 <!-- see below --> | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix N/A <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT | Doc PR | N/A Spotted while rebasing #32133 Commits ------- 7ccb5a1 [PropertyInfo] Fix ReflectionExtractor + minor tweaks
|
#37857 has been merged up to master, you can rebase now. |
|
fabbot failure is unrelated to this PR |
|
Thank you @ogizanagi. |
…abbuh) This PR was merged into the 5.2 branch. Discussion ---------- [PropertyAccess] use bitwise instead of boolean flags see symfony/symfony#32133 Commits ------- c5101c7 [PropertyAccess] use bitwise instead of boolean flags
Working with some legacy code having annoying
__get&__setmethods, this would have been useful the same way__callcan be enabled/disabled.