-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix initializing lazy objects and get rid of "Typed property must not be accessed before initialization" errors #10385
Fix initializing lazy objects and get rid of "Typed property must not be accessed before initialization" errors #10385
Conversation
… be accessed before initialization" errors
b91484a
to
a56c278
Compare
a56c278
to
3b8692f
Compare
Where is the point in taking my fix and implement it in just a different coding style? |
No specific point, I just missed your PR sorry, thanks for point at it. |
@simonberger closed #10362, so let's get this one merged :-) |
LGTM, fixes my similar issue. thanks NG. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good because there are more tests, but don't ask me to explain this change 😅 If somebody can help me figure it out, I might feel confident enough to merge.
if (isset($this->_hints[Query::HINT_REFRESH_ENTITY])) { | ||
$this->registerManaged($this->class, $this->_hints[Query::HINT_REFRESH_ENTITY], $data); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These lines were removed in #10187 as pointed out in #10336 (comment)
|
||
$skippedProperties[$prefix . $name] = true; | ||
$filter = ReflectionProperty::IS_PRIVATE; | ||
$reflector = $reflector->getParentClass(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to do the same as before, but for does it also for ancestors in the class hierarchy, retrieving only private properties for those (I guess public and protected are already retrieved at the first iteration because they are inherited).
$this->_em->clear(); | ||
|
||
$this->assertSame('foo', $entity->relation->value); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this test 100% covers this change, the while
loop only ever does one iteration. Yet when I use git restore --source=origin/2.14.x lib/Doctrine/ORM/Proxy/ProxyFactory.php
, a preexisting test fails Doctrine\Tests\ORM\Functional\MappedSuperclassTest::testCRUD
, which means the scenario where there is more than 1 iteration is already covered 👍
It gets broken by the changes in SimpleObjectHydrator
, and fixed by the changes in ProxyFactory
Fix #10336