Skip to content
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

Typed property must not be accessed before initialization #4

Open
armpogart opened this issue Feb 5, 2020 · 9 comments
Open

Typed property must not be accessed before initialization #4

armpogart opened this issue Feb 5, 2020 · 9 comments
Assignees
Labels
Bug Something isn't working Won't Fix This will not be worked on

Comments

@armpogart
Copy link

armpogart commented Feb 5, 2020

Bug Report

Q A
Version(s) 3.0.2 (zendframework/zend-hydrator)

Summary

Cycle ORM uses the hydrator in it's mapper while persisting entities to the database and I have encountered problem while using PHP 7.4 typed properties. Following line raises the issue as it is trying to access typed property which doesn't have any default value (so it is not initialized by that time).

PHP 7.4 added ReflectionProperty class methods to work in such situations such as hasType and isInitialized. I guess the fix would be rather simple with those methods. But this will raise minimum PHP requirement to 7.4 or maybe you can apply the fix only after doing method_exists to check the PHP version.

Se also original issue for reference.

How to reproduce

As I'm not sure exactly what hydrator does, can propose just one way to reproduce with Cycle ORM documentation example and add typing to any property of entity just like this (see the int typing near $id) and run it:

<?php declare(strict_types=1);

namespace Example;

use Cycle\Annotated\Annotation\Entity;
use Cycle\Annotated\Annotation\Column;

/**
 * @Entity
 */
class User
{
    /**
     * @Column(type="primary")
     * @var int
     */
    protected int $id;

    /**
     * @Column(type="string")
     * @var string
     */
    protected $name;

    public function getId(): int
    {
        return $this->id;
    }

    public function getName(): string
    {
        return $this->name;
    }

    public function setName(string $name): void
    {
        $this->name = $name;
    }
}
@kynx
Copy link
Contributor

kynx commented Feb 6, 2020

I've been bitten by the declare(strict_types=1) addition to ClassMethodsHydrator as well. Turns out I've been relying on the implicit coercion from the type declarations in my setter methods for database entities. Everything that hits them is a string.

I haven't had the time to work out what to do and have locked on 2.4.2 until I do. Remove the type declarations and cast everything on assignment? Do something clever with filters? Whatever, it's going to be a lot of work.

In general I think strict_types in a library that is designed to call end-user code is a bit too opinionated. And this is a perfect example of why. I'd be interested to hear if the maintainers would support removing them.

@armpogart
Copy link
Author

@kynx I get what you are saying, but this specific error has nothing to do with declare(strict_types=1). No matter you have that declaration or not (on both sides), if you try to access uninitialized typed property in PHP 7.4 you will get this error.

@Xerkus
Copy link
Member

Xerkus commented Feb 6, 2020

@armpogart this looks kinda like implementation bug. If it is required but not initialized by the time attempt to extract data happens means inconsistent state.

@Xerkus
Copy link
Member

Xerkus commented Feb 6, 2020

Hydrator is expected to return array of specific keys. It can't just skip key but null is not a valid value.

In your specific example id is not available until object is persisted. That makes null a valid initial state for it:

protected int $id  = null;

@kynx
Copy link
Contributor

kynx commented Feb 6, 2020

@armpogart Ack, wasn't reading properly. Sorry to hijack!

@armpogart
Copy link
Author

armpogart commented Feb 6, 2020

@Xerkus I will agree only partially, while this is correct for the $id case, what about any other field (property) such as $name. The null can be a valid value for the db column, if I initialize it null by default, from the orm point of view it won't be able to distinguish whether I was aiming for the null value or it is simply null by default.

If we were talking about first class strictly typed languages such as Java, or C#, I will certainly say that not initializing property is implementation problem, but I'm not sure for the PHP case.

Anyway as you said Hydrator is expected to return array of keys and I don't see the reason for the Hydrator not to do it in case if user implementation is incorrect as the language allows it. It's just not Hydrator's problem I think.

@Xerkus Xerkus added Bug Something isn't working Won't Fix This will not be worked on labels Feb 7, 2020
@Xerkus
Copy link
Member

Xerkus commented Feb 7, 2020

This topic is deeper that I first assumed. It goes further than just suppressing this error.

This will need full feature proposal RFC exploring use cases with the goal to establish well defined behavior expectations followed by consistent implementation across the component.

Marking this as a won't fix for now.

@ghost
Copy link

ghost commented Aug 8, 2022

So is this topic open or closed?
It's 2 years later and I'm having the same issue with associations, which cannot be null because then you're trying to access a property on a null.

Example:

        /**
         * @var Employees|null
         *
         * @ORM\ManyToOne(targetEntity="\App\Entity\Employees")
         * @ORM\JoinColumns({
         *   @ORM\JoinColumn(name="reportsTo", referencedColumnName="employeeNumber")
         * })
         */
        private ?Employees $reportsTo;

        public function __construct()
        {
            $this->reportsTo = null;
        }

Then:

$reportsTo = $entity->getReportsTo()->getJobTitle();

... will always throw the fatal error because $entity->getReportsTo() is null

@Xerkus
Copy link
Member

Xerkus commented Aug 9, 2022

@webtop Your problem is not related to this issue or to hydrator component.

"Typed property must not be accessed before initialization" error with $reportsTo = $entity->getReportsTo(); means ORM does not populate property with value, null or otherwise. You need to set default value in property declaration as ORM usually skips calling constructor:

 private ?Employees $reportsTo = null;

If getReportsTo() must not return null it is your responsibility to ensure non-null value was set or otherwise check for null before using it.
PHP 8 provides convenience null-safe operator https://www.php.net/manual/en/language.oop5.basic.php#language.oop5.basic.nullsafe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Won't Fix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants