Closed kktsvetkov closed 2 years ago
I think the reason I didn't do this is because it would require checks on arrays, strings, and objects too. (eg. __invoke
) Performance performance...
If you want to implement this we can certainly add it as an opt-in plugin, maybe on by default if the performance isn't as bad as I expect it to be. You can reuse MethodValue
to handle most of the boilerplate (Not sure how it would work on plain functions though)
I think the reason I didn't do this is because it would require checks on arrays, strings, and objects too.
I was actually thinking of a simple if (is_callable($var)) {...}
check, and let PHP do the detection. And if it is callable, just push a hint that says "callable".
Are you concerned about the Representation of it when you say "require checks"? E.g. how different callable formats are rendered and if the actual callback is parsed ?
was actually thinking of a simple
if (is_callable($var)) {...}
check, and let PHP do the detection
Yeah I had the same idea, but right now all we need to get callables working is a check for instanceof Closure
. Strings arrays and objects covers a lot of variables, and you'd be running it for every one.
For reference, it's not hard to go through Parser::parse
over 10k times in a single dump in a real system. If half of those are strings/arrays/objects even a little is_callable
can have an effect. (Not to mention the extra plugin dispatch)
Are you concerned about the Representation of it when you say "require checks"?
That too. Currently the render CallablePlugin
has special cases for closures and methods. We'd need to make a common interface for callables to work right.
Strings arrays and objects covers a lot of variables, and you'd be running it for every one. ... For reference, it's not hard to go through
Parser::parse
over 10k times in a single dump in a real system.
It's a valid point. Perhaps it is better to have it as an optional plugin and configurable to what types to scan since you know what types of callbacks your project is using. E.g. there are projects that use the class::method
format in routers, and if that is your project, do configure it to scan strings for callables; another example is WordPress where there are a lot of function names used as callbacks, so if you encounter wp_do_something
there is a benefit to know if this is just a key or is it a name of a function.
Perhaps it is better to have it as an optional plugin and configurable to what types to scan since you know what types of callbacks your project is using.
Yeah that sounds even better. Again, go ahead and make it and we can see what the performance does afterwards. Worst case scenario we have a new optional plugin
Actually I am working on a version of Krumo and I saw that detecting "callable" is something that I could not find in Kint, and that's why I suggested it. I don't feel confident enough in my Kint knowledge (yet) to create a new plugin.
... and we can see what the performance does afterwards.
I've done dumping get_defined_functions()
and there every single element is a callback :) That can be considered one of the worst-case scenarios, right?
I don't feel confident enough in my Kint knowledge (yet) to create a new plugin.
Have you seen the guide?
That can be considered one of the worst-case scenarios, right?
Well I imagine the performance cost of is_callable
doesn't change much either way. Honestly I'm more worried about large amounts of plugin dispatches than the performance of is_callable
itself. It's really something we can only see once it's built
Have you seen the guide?
I have, but only as part of my research and not with the intention to actually build a plugin. I've read all of the site, the latest code, and most of the issues and pull-requests, and some of the code history to see how some of the features have evolved, mainly between 2.x and 3.x
It is an interesting challenge though, I might take a stab at it. I've looked at the pull-requests, and I didn't see any such merged of people creating plugins. Are there any collaborator-created plugins currently in the latest code? Or you do not merge them in the project and advise them to put them in separate libraries as add-ons?
If they're related to native PHP features I include them in core, I only advise to make separate repos for them if they're integration with a third party system or something like that. As far as I know raveren (The last maintainer) is the only person to make a plugin so far. (But I imagine someone's made one for personal use at some point)
As https://wiki.php.net/rfc/deprecate_partially_supported_callables points out, there are several "Callable" types that can't even be identified with is_callable()
(because they're context sensitive) prior to 8.2. We have no way of identifying these, and we never will.
Imagine for instance a callable self::parse
- if we check this with is_callable()
it'll always return true because the plugin by definition has a parse()
method.
And on top of that you also have visibility making certain "Callables" context-sensitive past 8.2 too. X::y()
may only be callable from inside X
if y()
is private, so we'd have to make a judgement call on what scope to assume. Do we assume the scope of the nearest parent object the value is in? Of the calling code? Global?
I think it's best to table this until PHP 9 (or higher?) has an actual well-defined callable type:
If callables were to be limited to public methods in the future [...], then the callable type would become well-defined and could be used as a property type
Until this is implemented and a "strict" version of is_callable
is produced then even regardless of the plugin performance issues we can't do this reliably.
There are several formats that identify callable vars. Most ancient are traditional PHP callbacks such as
[Kint\Kint::class, 'dump']
,[$object, 'method']
and function names, then there's theKint\Kint::dump
format, plus Closures and objects with__invoke()
methods. All of those can be detected by usingis_callable()
and hinted as such.Here's an example with few callable formats: