declare assert-if-true for filesystem functions#1993
declare assert-if-true for filesystem functions#1993staabm wants to merge 3 commits intophpstan:1.11.xfrom
Conversation
There was a problem hiding this comment.
Two problems here:
chdir($dir) ? 'foo' : 'bar'- in the else branch$dirwill be considered''- If you pass
non-empty-stringtochdir(...), you'll get "always true" error.
Both can be solved with =non-empty-string. See: phpstan/phpstan#8348 + phpstan/phpstan#8351
49564b7 to
766c8d9
Compare
ondrejmirtes
left a comment
There was a problem hiding this comment.
To make sure that = actually fixes these problems, you should have tested the else condition types and also put a test about this in ImpossibleCheckTypeFunctionCallRule (before adding the = everywhere). Please verify this was originally a problem and that it's fixed after the change. I'm sure you can find the previous version in your reflog :) It's e117fce or 49564b7.
|
good point. |
|
the last commit is fixing cases like <?php
namespace Bug4816;
use function PHPStan\Testing\assertType;
function (string $x): void {
if (is_dir($x)) {
assertType('true', is_dir($x));
}
};(so we don't regress 1eaef04) my understanding is, that because this PR adds stubs with phpstan-src/src/Analyser/TypeSpecifier.php Lines 455 to 472 in 7cd03f0 and that the reason why my previous code sample did regress. After adding @rvanvelzen could you give me a hand? why do we not invoke btw: I noticed we also don't invoke |
|
I had no idea that the assertion would actually assert the opposite if the condition is not met. This is very counter-intuitive for me.. I think the behavior with "=" should be the default and there should rather be another modifier to request the double behavior. |
closes phpstan/phpstan#6788