[Validator] check for __get method existence if property is uninitialized #35546
Conversation
164ab11 to
560e2c8
Compare
| return null; | ||
| if (\PHP_VERSION_ID >= 70400 && $reflProperty->hasType() && !$reflProperty->isInitialized($object)) { | ||
| // There is no way to check if a property has been unset or it is uninitialized. | ||
| // When trying to access an uninitialized property, __get method is triggered in PHP 7.4.0, but not in 7.4.1+. |
There was a problem hiding this comment.
Does this mean that there was a bug in PHP 7.4.0 and __get() should never have been called? In this case, I am not convinced that we should do anything here as the behaviour is inline with what happens in PHP 7.4.1+.
There was a problem hiding this comment.
or should we call __get() directly?
There was a problem hiding this comment.
There is a discussion PHP Bug # 78904.
This is a PHP related problem, I think. Framework should not implement any hack for this. ProxyManager and others are currently not compatible with PHP 7.4.1+.
There was a problem hiding this comment.
__get must be called when accessing a property if the property has been explicitly unset, but not if it is uninitialized.
On the other side ReflectionProperty::isInitialized is returning false for both situations and there is no safe way to distinguish one state from the other.
There was a problem hiding this comment.
The hasType has been added as if the property is not typed, no additional check should be made.
I think that \Error is too generic. Per typed properties RFC \TypeError is raised in case of access an uninitialized non-nullable typed property.
There was a problem hiding this comment.
The
hasTypehas been added as if the property is not typed, no additional check should be made.
Agreed. hasType is just a zero cost check.
I think that
\Erroris too generic. Per typed properties RFC\TypeErroris raised in case of access an uninitialized non-nullable typed property.
Don't trust docs blindly.
There was a problem hiding this comment.
According that, additional tests should be added.
- typed uninitialized propery with no unset in constructor and with __get method.
This test would validate a correct way of \Error catching. In case if it will be changed to \TypeError in future.
There was a problem hiding this comment.
I've just added a check. Too bad that RFC is not respected about the \TypeError
There was a problem hiding this comment.
Too bad that RFC is not respected about the
\TypeError
Doc Bug #79206
Let's see, it is a doc bug or not.
Here we trying to solve one special case, when typed property is Only difference with 7.4.0 that this magic method was invoked in uninitialized state also. This is normal (invoking |
Yes, i've corrected the comments. |
|
Thank you @alekitto. |
…s uninitialized (alekitto)
This PR was merged into the 3.4 branch.
Discussion
----------
[Validator] check for __get method existence if property is uninitialized
| Q
6D40
| A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix #35544
| License | MIT
Resolve bug #35544.
On PHP 7.4, check if object implements `__get` magic method if property is reported as uninitialized before returning null.
Commits
-------
427bc3a [Validator] try to call __get method if property is uninitialized
Resolve bug #35544.
On PHP 7.4, check if object implements
__getmagic method if property is reported as uninitialized before returning null.