That's a lot of tests to be updated.

I have been looking into this more and checking the actual conversion in real code. This might actually suck for users a bit because in this case we basically tell them to add more code for no benefit. For example compare those two:

class User {
    public function __construct(
        public readonly string $id,
        public readonly string $email,
        public readonly DateTime $createdAt
    ) {}
    
    public function __sleep(): array {
        return ['id', 'email', 'createdAt'];
    }
    
    public function __wakeup(): void {
        if (!filter_var($this->email, FILTER_VALIDATE_EMAIL)) {
            throw new InvalidArgumentException('Invalid email format');
        }
        if (empty($this->id)) {
            throw new InvalidArgumentException('ID cannot be empty');
        }
    }
}

The conversion would be like this:

class User {
    public function __construct(
        public readonly string $id,
        public readonly string $email,
        public readonly DateTime $createdAt
    ) {}
    
    public function __serialize(): array {
        return [
            'id' => $this->id,
            'email' => $this->email, 
            'createdAt' => $this->createdAt,
        ];
    }
    
    public function __unserialize(array $data): void {
        if (!filter_var($data['email'], FILTER_VALIDATE_EMAIL)) {
            throw new InvalidArgumentException('Invalid email format');
        }
        if (empty($data['id'])) {
            throw new InvalidArgumentException('ID cannot be empty');
        }
        $this->id = $data['id'];
        $this->email = $data['email'];
        $this->createdAt = $data['createdAt'];
    }
}

Now imagine you have class with 50 properties. Why do we need to force users to add this extra code?

On top of that this is also slower in terms of perf. I did a little bit of testing and just with 3 props, wake up was most of the time faster so with 50+ props, it might be even visible but probably not something major to worry about.

The way how I see it is that __unserialize is not a direct replacement for __wakeup but more a method that can be used if different mapping should be used. But it doesn't seem like the use case that everyone has so for many users it might just mean that

@Girgias Have you actually considered all of this before proposing the deprecation? It passed by closest possible margin (1vote) and I guess some of the voters might have not been aware what's required (honestly I haven't realised it until now and I have actually worked with it in past - I voted no because I thought it does not harm to keep it but in reality it is actually a burden to do the migration and it results in worse code). I think if the RFC showed some examples of needed migration and also mentioned potential perf impact, it wouldn't pass.

Btw the examples are a bit stripped off as above wouldn't need to use __sleep and __serialze (there should be some extra props added but it's just to show a minimal conversion without extra props in the class.

This might actually suck for users a bit because in this case we basically tell them to add more code for no benefit.

I expect the majority of implementations to be just throwing an exception to prevent serialization.

Why do we need to force users to add this extra code?

One good reason against __sleep() is that it's impossible for it to correctly handle inheritance, since private properties cannot be serialized with it. This is also documented:

It is not possible for __sleep() to return names of private properties in parent classes. Doing this will result in an E_NOTICE level error. Use __serialize() instead.

As for __serialize(), the minimal implementation that preserves behavior can just be:

	public function __serialize() {
		return get_mangled_object_vars($this);
	}

Possibly combined with unset() to drop individual properties. __unserialize() indeed is a little more complicated when inheritance gets involved (due to private properties), but that is also something that can be dealt with in a generic fashion, possibly using something like this:

	public function __serialize() {
		return [parent::__serialize(), [
			'my' => 'stuff',
		]];
	}
	public function __unserialize(array $data) {
		[$parentData, $myData] = $data;
		parent::__unserialize($parentData);
		foreach ($myData as $key => $val) {
			$this->$key = $val;
		}
	}

But how exactly it will need to looks greatly depends on whether the parent class has a serialization hook.

But how exactly it will need to looks greatly depends on whether the parent class has a serialization hook.

It could make sense to automatically implement a __serialize() and __unserialize() stub on all classes, which would allow to unconditionally call parent::__serialize() and parent::__unserialize() in all cases, which would make dealing with inheritance easier. I was also thinking about doing something like this for __destruct() (the correct handling of which is a mess currently).

My real problem with this is that the RFC just made it sound like it's a stright forward replacement but it's not. It didn't even give any example of it. I guess those are valid points but none of it was in the RFC because there were tons of other changes and no space for discussion. So people just made decision based on couple of notes that made it sound like a no brainer and even then it passed by the closest possible margin. That says everything. I'm pretty sure if all those points were there, it wouldn't pass. But it's what it is. Feel free to merge it.

That said it's sort of my fault as I didn't raise those points earlier though. It's too late now unfortunately.

For reference, there is a discussion on the mailing list, so please keep it on the list going forward: https://news-web.php.net/php.internals/128456

Given the discussion on internals, the PR should likely be put on hold.
I suggest updating the documentation instead, so that sleep/wakeup aren't displayed first anymore on https://www.php.net/language.oop5.magic and we display notices about this mechanism being soft-deprecated?