Fix args are mistakenly handled as immediately-invoked #4145
Conversation
|
This pull request has been marked as ready for review. |
staabm
force-pushed
the
bug13288
branch
2 times, most recently
from
a6017ac to
2f3b833
Compare
src/Analyser/NodeScopeResolver.php
Outdated
| if ( | ||
| $parameterType === null | ||
| || $parameterType instanceof MixedType | ||
| || $parameterType->isCallable()->no() |
There was a problem hiding this comment.
I think we need a new TypeUtils method. findCallableType perhaps. It'd return the type for isCallable()->yes(). Additionally it should recursively inspect unions for types like ?callable or ?Closure.
There was a problem hiding this comment.
But I think this logic is wrong. What mainly has to be handled is $parameterCallImmediately->maybe(). That's where you need to ask about this. And also in the else section where it's $callCallbackImmediately = $calleeReflection instanceof FunctionReflection; right now.
There was a problem hiding this comment.
This logic fails for a couple reasons:
- If the parameter is
mixedbut marked with@param-immediately-invoked-callableit'd get silently ignored. - It'd still mark parameter as immediately invoked if the type is for example
string(which is maybe-callable).
There was a problem hiding this comment.
thanks for the in-detail feedback. there are a lot of things to consider, I did not see yet.
additionally phpstan/phpstan#13331 (comment) was useful.
I tried adding more tests and adding comments about expectations, of what I understood from the comments.
thank you
the php engine might invoke the callables whenever it wants. the assumption about a function invoking the callable immediately fits for userland functions good enough though
ondrejmirtes
left a comment
ondrejmirtes
left a comment
There was a problem hiding this comment.
Additionally please add a regression test for 12119
src/Type/TypeUtils.php
Outdated
| return $type; | ||
| } | ||
|
|
||
| if ($type instanceof UnionType || $type instanceof IntersectionType) { |
There was a problem hiding this comment.
I don't think we need to go into intersections. Once one of the intersected types is callable, the whole type is callable.
|
Thank you! |
3 participants
closes phpstan/phpstan#13288
closes phpstan/phpstan#13311
closes phpstan/phpstan#13331
closes phpstan/phpstan#13307
closes phpstan/phpstan#12119