laminas / laminas-hydrator

Serialize objects to arrays, and vice versa
https://docs.laminas.dev/laminas-hydrator/
BSD 3-Clause "New" or "Revised" License
123 stars 32 forks source link

Typed property must not be accessed before initialization #4

Open armpogart opened 4 years ago

armpogart commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

armpogart commented 4 years ago

@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 commented 4 years ago

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 commented 2 years ago

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 commented 2 years ago

@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