-
Notifications
You must be signed in to change notification settings - Fork 27
Fix 130 #134
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
Fix 130 #134
Conversation
Can you show the false positive here? |
It's just that https://github.com/phpstan/phpstan-webmozart-assert/blob/1.1.2/tests/Type/WebMozartAssert/data/impossible-check.php#L26 is not reported anymore, but maybe I need to adapt something else here |
@ondrejmirtes I took another look to find out why that assertion is not reported anymore. The step debugging revealed the following for the second assertion: What baffles me is the scope state. |
It's coming from https://github.com/phpstan/phpstan-src/blob/1.6.4/src/Analyser/MutatingScope.php#L2345 but that makes sense. Since phpstan only knows that both left and right are |
8eaed3f
to
7c8b85c
Compare
Came up with something new for the Also added 2 other regression tests, but the bugs behind them were most likely fixed by the recent typespecifier adaptions in phpstan itself 🎉 |
I think this one's good enough, did not came up with anything better yet and it's not too weird either IMO. |
Thank you. |
Closes #118
Closes #119
Closes #130
Looks like the rootExpr approach is potentially making problems if the same thing is asserted twice in a row, where previously it would detect the second one as always evaluating to true/false. But this could be related to the all* implementation here too.