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

Avoid “Deprecate dynamic properties” warning in PHP 8.2 #159

Merged
merged 1 commit into from
Oct 13, 2022

Conversation

chregu
Copy link
Contributor

@chregu chregu commented Oct 12, 2022

PHP 8.2 deprecates access to dynamic properties, if not explicitly allowed through __set/__get or an annotation.
Therefor PHPStan fails for some reason in \Jcupitt\Vips\ImageAutodoc, even though it is defined in the Image class, see for example here https://github.com/rokka-io/imagine-vips/actions/runs/3236307918/jobs/5301940309

Adding __set and __get to ImageAutodoc prevents this "fail".

More details about this here https://wiki.php.net/rfc/deprecate_dynamic_properties

(PHP 8.2 is not released yet and "normal" code runs normally, just PHPStan fails currently).

This should also prevent a fatal error, if that is really removed in PHP 9.0 (__set/__get won't be removed, just implicit dynamic properties)

@chregu chregu force-pushed the php-8.2-deprecate-dynamic-properties branch from 06642a5 to f3e6dd7 Compare October 12, 2022 17:36
@chregu chregu force-pushed the php-8.2-deprecate-dynamic-properties branch from f3e6dd7 to 7f64fa4 Compare October 13, 2022 06:34
@jcupitt jcupitt merged commit 247ab4f into libvips:master Oct 13, 2022
@kleisauke
Copy link
Member

Perhaps an alternative solution is to mark the Image class or abstract ImageAutodoc class with the #[AllowDynamicProperties] attribute? This should be fully backwards-compatible with earlier PHP versions according to the RFC mentioned above.

@jcupitt
Copy link
Member

jcupitt commented Oct 13, 2022

Ooop, sorry Kleis, I missed your comment. Shall I revert?

@kleisauke
Copy link
Member

I wasn't sure whether PHPStan supports that attribute and if that would fix it, so perhaps a revert is not necessary.

@chregu
Copy link
Contributor Author

chregu commented Oct 13, 2022

That's the other alternative, but then that maybe will break with PHP 9 again (Don't think that's totally decided yet). The abstract functions for __set and __get seem to be the future proof way.

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

Successfully merging this pull request may close these issues.

3 participants