-
Notifications
You must be signed in to change notification settings - Fork 7.9k
GH-16317: allow __debugInfo()
overrides in mysqli classes
#16543
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
base: master
Are you sure you want to change the base?
Conversation
For classes that extend `mysqli`, `mysqli_result`, or `mysqli_statement`, and define a `__debugInfo()` magic method, use it.
class subclass_mysqli extends mysqli { | ||
public function __construct() {} | ||
public function __debugInfo(): array { | ||
return ['a' => 'b']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps a test for seeing that you can call parent::__debugInfo()
might be useful to add here as well for the different subclasses
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except the parent classes don't define those methods? There isn't a parent method to call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm interesting if the internals of mysqli fills out an implementation for __debugInfo
, then I would almost argue that is a secondary bug and it should properly expose such /or any other internal class that does similar).
Just tested and confirmed the patch with without my above comment too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to file that bug I might be able to work on it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Internal classes faking methods that are supposed to exists in userland is indeed a long standing issue. E.g. Internal classes don't have a default constructor... i.e. https://3v4l.org/SIMVn#v8.4.2
So I think the best and most consistent solution would be to add the debugInfo()
method instead of patching the handler. As far as I see, the get_debug_info
object handler of mysqli classes are just performance optimizations (the __debugInfo()
method doesn't have to be called). Since performance is not a concern in case of a debug messages, it's not a problem at all if we use the magic method instead of the handler.
In any case, as this idea may be controversial, other maintainers should also express their preference how to best solve the problem before continuing the implementation...
BTW: I suspected that we have similar issues with other object handlers, but I was not able to find any other issues (initially I thought that at least I can find issues with the *_dimension
handlers)
For classes that extend
mysqli
,mysqli_result
, ormysqli_statement
, and define a__debugInfo()
magic method, use it.