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
Dynamic function storage provider #7471
Dynamic function storage provider #7471
Conversation
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.
Nice!
This opens up a lot of flexibility for those variadics generic cases!
This pr still contains the preanalysed version of the function params hook. Is it the idea to keep it there since we can now use this new hook?
I would suggest to remove old hook in favor new one. |
Guys, it's amazing to read your comments and the proactivity ! Even if I don't understand anything, I'm pretty sure you're going to make something great that I really want to use ASAP ! |
Would old plugins be easy to migrate? If we do it, it's the right moment, we just removed the old plugins interfaces in Psalm 5, it's possible a lot of plugins didn't migrate already |
It depends which ones are gonna be removed. The one for function parameters I don't think is being used commonly. Because you can't really do that much with it at this moment. But I can only speak for my own plugins :) Current hooks are compatible with the once for methods as well. There we don't have this new hook. |
I suggest to remove On the other side, new hook is very low level. Writing plugin for |
It might make sense to keep them, for making the upgrade traject easier for maintainers as well. |
Ok then let's keep other hooks for now. Before I dive too deep in the code, I have a few questions:
|
I've removed double args analysis just now. This can lead to many problems. If you have not yet understood what the problems are, I can try to describe them.
Of course I can. When PR will ready.
Currently just for test. I planned to use it as example for documentation. |
I removed internal classes usage and tried to make a public api instead.
|
Also, how does it interact with cache? |
I didn't quite understand what you mean. Dynamically created storage saves only in the As I know Psalm treats each closure as unique and create for them individual storage. Does Psalm cache it? $dynamic_storage_id = strtolower($statements_analyzer->getFilePath())
. ':' . $stmt->getLine()
. ':' . (int)$stmt->getAttribute('startFilePos')
. ':dynamic-storage'
. ':-:' . strtolower($function_id); Id from These two calls will be analyzed individually and each call will have their id: custom_array_map(
fn($i) => $i + 1,
[1, 2, 3]
);
custom_array_map(
fn($i) => (string) $i,
fn($i) => $i + 1,
['1', '2', '3']
); There is sense to cache it? And if there is. How? |
src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php
Show resolved
Hide resolved
I meant the file cache Psalm uses to speed up things on subsequent runs. Normally Also, would you mind renaming all occurrences of |
… for more flexibility
Thanks! |
Excellent work, thanks ! |
Great job! |
Will be released in v5? |
Yeah, this has been targeted on master branch, so with V5 release |
Closes #7450
New plugin hook. It allows to create
FunctionStorage
for hard to infer functions by Psalm.I've added test for
custom_array_map
.custom_array_map
is similar to nativearray_map
.Of course plugin hook don't cover all cases that covered by Psalm for
array_map
. But it possible.That means if we improve the hook, we can remove the weird code inside Psalm for array_map or array_filter.
With new hook we can create
FunctionStorage
forpipe
orcurry
orpartial
and many other functions from functional world.I can share plugin hooks for
pipe
andpartialRight
(without hardcoded templates as in #7470) if new hook is legit.@veewee @orklah