Skip to content

Regression in PHP 8.2 #9121

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

Closed
kylekatarnls opened this issue Jul 24, 2022 · 7 comments
Closed

Regression in PHP 8.2 #9121

kylekatarnls opened this issue Jul 24, 2022 · 7 comments

Comments

@kylekatarnls
Copy link
Contributor

kylekatarnls commented Jul 24, 2022

Description

The following code:

<?php
$d = (new ReflectionClass(DateTime::class))->newInstanceWithoutConstructor();

$d->date = '1990-01-17 10:28:07';
$d->timezone_type = 3;
$d->timezone = 'US/Pacific';

Resulted in this output:

ErrorException: Creation of dynamic property DateTime::$date is deprecated

But before PHP 8.2 we had $d being a usable DateTime object

PHP Version

PHP 8.2.0

Operating System

No response

@KapitanOczywisty
Copy link

Seems like this wasn't working too good.. https://3v4l.org/niJsq and in 8.2 this is only deprecation warning not an actual exception: https://3v4l.org/niJsq/rfc#vgit.master I wouldn't call this regression since it wasn't supported in the first place. https://3v4l.org/8aeug

@cmb69
Copy link
Member

cmb69 commented Jul 25, 2022

https://3v4l.org/R5dN8 shows that the DateTime properties are only dynamically created by the constructor, but are not declared properties. That matches the documentation.

While I agree with @KapitanOczywisty that this is not a regression, but a mere deprecation, it still looks somewhat strange to me to get this error.

@derickr
Copy link
Member

derickr commented Jul 28, 2022

But before PHP 8.2 we had $d being a usable DateTime object

Nope, assigning to the "properties" never did anything, as is illustrated in https://3v4l.org/8aeug

PHP 8.2 now (merely) warns you of this, just like it does for normal user-land dynamic properties.

I don't think there is anything wrong here, and showing this warning is the expected result.

@derickr derickr closed this as completed Jul 28, 2022
@kylekatarnls
Copy link
Contributor Author

It's used for caching data via serialization and it worked before:

https://3v4l.org/DROXa

@kylekatarnls
Copy link
Contributor Author

This the code above breaking in PHP 8.2:

https://3v4l.org/DROXa/rfc#vgit.master

I'm not personally impacted but opis/closure use this construction way for dates, so similar cache system will break.

@KapitanOczywisty
Copy link

It's used for caching data via serialization and it worked before:

Makes more sense now. This was already reported in: #8152 please especially read comment explaining background: #8152 (comment)

Following that issue __serialize/__unserialize methods were added, so starting from 8.2 code could look like this:

<?php
$d = (new ReflectionClass(DateTime::class))->newInstanceWithoutConstructor();

$d->__unserialize([
    'date' => '1990-01-17 10:28:07',
    'timezone_type' => 3,
    'timezone' => 'US/Pacific',
]);

var_dump($d, $d->format('c'));

https://3v4l.org/4poVG/rfc#vgit.master

Change requires checking the php version, but fixes issue once and for all. It could be worth noting this as breaking change just because it is apparently common pattern.

@kylekatarnls
Copy link
Contributor Author

Yep, it would be possible. As in this case DateTime::class is actually a dynamic value that could be anything, it will just need to check method_exists($d, '__unserialize') and keep the previous method as fallback, but globally it would allow to port it for PHP 8.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants