in_array returns false in strict mode if types are incompatibles #7141
Conversation
mathroc
force-pushed
the
fix/in-array-return-type
branch
8 times, most recently
from
c8eba46
to
2374d24
Compare
| @@ -377,24 +377,7 @@ function assertInArray($x, $y) { | |||
|
|
|||
| return $x; | |||
| }', | |||
| 'error_message' => 'RedundantConditionGivenDocblockType - src' . DIRECTORY_SEPARATOR . 'somefile.php:8:30 - Docblock-defined type int for $x is never string', | |||
| ], | |||
| 'assertNegatedInArrayOfNotIntersectingTypeTriggersMixedReturnStatement' => [ | |||
There was a problem hiding this comment.
would become the same as assertNegatedInArrayOfNotIntersectingTypeTriggersRedundantCondition L317
weirdan
added
the
release:feature
There was a problem hiding this comment.
Not sure about all those changes to expected messages. Shouldn't we mark the return type with from_docblock flag when any of the source types are coming from docblock, like this:
$ret = Type::getBool();
$ret->from_docblock = $needle_type->from_docblock || $haystack_type->from_docblock;
return $ret;?
| if (!$third_arg instanceof ConstFetch | ||
| || strtolower($third_arg->name->parts[0]) !== 'true' |
There was a problem hiding this comment.
This would work with literal true only, but is there any reason for that limitation? Can't we fetch the expression type from the node type provider and use that instead?
There was a problem hiding this comment.
This would work with literal true only, but is there any reason for that limitation? Can't we fetch the expression type from the node type provider and use that instead?
I'm looking at the Psalm internals for the first time, and I forgot to come back to this bit after I discovered the node type provider. I'll change that right away!
Not sure about all those changes to expected messages. Shouldn't we mark the return type with from_docblock flag when any of the source types are coming from docblock, like this:
Didn't know about that either, I'll try and see what I can do 👍
There was a problem hiding this comment.
I think it's ok now, let me know if anything else can be improved
mathroc
force-pushed
the
fix/in-array-return-type
branch
2 times, most recently
from
afce897
to
86bb0e3
Compare
I'm not sure about that. I don't think that's what Psalm does when we use stubs with conditional returns, it seems weird to do that here only |
mathroc
force-pushed
the
fix/in-array-return-type
branch
from
86bb0e3
to
39fb422
Compare
Given the following code: /** @param list<int> $a */
function f(array $a, string $b): void {
if (in_array($b, $a, true) {}
}What's more appropriate here, |
|
The second one seems more logical. We may want to check how conditional returns are applied and make sure the flag goes through too... |
|
yep :) Thanks @mathroc ! |
fixes #5552