Skip to content

Comments

[PHP 8.3] Fix get_class() deprecations#152

Closed
Ayesh wants to merge 1 commit intowintercms:developfrom
Ayesh:php83/get_class-deprecation
Closed

[PHP 8.3] Fix get_class() deprecations#152
Ayesh wants to merge 1 commit intowintercms:developfrom
Ayesh:php83/get_class-deprecation

Conversation

@Ayesh
Copy link

@Ayesh Ayesh commented Aug 14, 2023

In PHP 8.3, [calling `get_class()` and `get_parent_class()` functions without
arguments is deprecated](https://php.watch/versions/8.3/get_class-get_parent_class-parameterless-deprecated).

This fixes them with identical alternatives that do not cause the deprecation notice.

References:
 - [PHP RFC: Deprecate functions with overloaded signatures](https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures)
 - [PHP 8.3: get_class() and get_parent_class() function calls without arguments deprecated](https://php.watch/versions/8.3/get_class-get_parent_class-parameterless-deprecated)
@Ayesh Ayesh closed this by deleting the head repository Aug 14, 2023
@bennothommo
Copy link
Member

@Ayesh unfortunately that won't work - we have tried that before.

We'll have to refactor the ExtendableTrait to use the Reflection library in order to properly handle this case, I believe.

@LukeTowers
Copy link
Member

@bennothommo why does it not work?

@bennothommo
Copy link
Member

@LukeTowers I think it's a quirk of traits - the parent keyword is (AFAIK) compiled on initialization, and inside the scope of a trait, it fails because a trait has no parent.

@LukeTowers
Copy link
Member

Should we log that as an issue with PHP 8.3 then?

@bennothommo
Copy link
Member

Evidentally, it's an issue with PHP 8.0-8.2 as well, going by our tests. I'm just about to roll out a PR that does what we need with Reflection classes. It should be way more reliable.

bennothommo added a commit that referenced this pull request Aug 14, 2023
This uses Reflection to instead determine the parent class and available magic methods to pass through to. In order to prevent infinite looping (which could've potentially been a problem before), it will also ignore any "extendable" classes when determining the parent.

Also fixed some tests that were using undefined class properties, and were also throwing deprecation errors.

Replaces #152
@Ayesh
Copy link
Author

Ayesh commented Aug 14, 2023

I think the approach in #153 is more robust. However, I wanted to note that get_parent_class($this) should work even when they are in traits (eval).

@LukeTowers
Copy link
Member

@Ayesh if that's the case are you able to figure out why the tests failed in our case with this PR? I would prefer the simpler approach of just passing $this if we can make that work, although I haven't dug into @bennothommo's PR too deeply yet.

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