-
Notifications
You must be signed in to change notification settings - Fork 542
More precise types in immediately invoked callables #3592
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
Conversation
Thanks so much @staabm. I just tested this on TwigStan and it solves the problem. 🎉 This was a major blocker, as proper analysis of templates after such immediately involved callable became impossible. It would be really great if this can make it in a last 1.12 release 🙏 |
$this->inFuncCall | ||
&& $node instanceof Node\Expr\Closure | ||
) { | ||
$node->setAttribute(self::ATTRIBUTE_NAME, true); |
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.
This would lead to a lot of false positives. You only want to tag this if in $node->name
, but right now the visitor is tagging args too.
Just check $node->name
inside $node instanceof Node\Expr\FuncCall
.
retargeted for 1.12 in #3593 |
assertType('array{date: DateTime, id: 1}', $c); | ||
|
||
$x = (function() use (&$c) { | ||
assertType("array{date: DateTime, id: 1, name?: 'ruud'}", $c); |
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.
I believe this is wrong, see phpstan/phpstan#11945
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.
My reasoning is, that since this function is invoked immediately, it should be treated as the same code as normally, without the function.
return 'x'; | ||
})(); | ||
|
||
assertType("array{date: DateTime, id: 1, name?: 'ruud'}", $c); |
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.
I believe this is wrong, see phpstan/phpstan#11945
closes phpstan/phpstan#11561