-
Notifications
You must be signed in to change notification settings - Fork 439
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
Rewrite multi param isset() to and-chain #2709
Conversation
} | ||
|
||
$types = null; | ||
$tmpVars = [$issetExpr]; |
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 had to rename this var, because the previous $var
variable after this PR is no longer within a foreach and therefore PHPStan would otherwise report a overlap with the foreach-var below
(no logic change contained otherwise in this block)
|
||
$specifiedTypes = new SpecifiedTypes(); | ||
// rewrite multi param isset() to and-chained single param isset() | ||
if (count($expr->vars) > 1) { |
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.
the actual change is here.
Implemented the idea expressed in phpstan/phpstan#8366 (comment)
This pull request has been marked as ready for review. |
ahh, let me add a regression test for phpstan/phpstan#10064 |
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.
Excellent idea, thank you
): void { | ||
|
||
if (isset($untilDate, $count)) { | ||
throw new \InvalidArgumentException('Too much params, choose between until and count.'); |
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.
Additional assertType calls inside this condition would be great
Thank you. |
motivation: ease impl of #2657 by moving the expr-var loop before the context true/false branches which simplifies the branch cases
closes phpstan/phpstan#8366
closes phpstan/phpstan#10064