array_replace_recursive() return type does not contain NULL

[vzz3]

array_replace_recursive() are documented to return NULL "if an error occurs", but Psalm treats it as it could only return an array and not NULL:

https://psalm.dev/r/66aaa94d25

also see issues #2925

PHP reference: https://www.php.net/manual/de/function.array-replace-recursive.php

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/66aaa94d25

<?php

$a = ['a' => 123];
$b = ['a' => 123];
$res = array_replace_recursive($a, $b);
if(!is_array($res)) {
	throw new \Exception('Can not merge array', 1579732170);
}

Psalm output (using commit 4d99fbc):

ERROR: TypeDoesNotContainType - 6:5 - Type array<array-key, mixed> for $res is always array

[weirdan]

It used to, before PHP 8, for non-array replacements. But Psalm ignores returns indicating parameter type errors because it reports type mismatches itself.

Since PHP 8 array_replace_recusive() throws TypeError instead.

See https://github.com/php/php-src/blob/23b1c4a9820f49534f22512c8eaf4741ee0959dd/ext/standard/basic_functions_arginfo.h#L219-L224.

[orklah]

@weirdan This commit removed null return for array_replace_recursive in base Callmap: b85cbd0#diff-b8eaee1f550652657daf0a771be5e785bdb01e91acd004b4bbb4c41def706713L401

It should have been in Callmap 8 in this case. I think the issue is valid

EDIT: I just got the part "Psalm ignores returns indicating parameter type errors because it reports type mismatches itself." then why not, but the two function should behave the same, no?

[weirdan]

@orklah do you mean array_replace()/array_replace_recusive()?

[orklah]

Yeah, #2925 was about both so I assumed they work similarly

[weirdan]

In PHP, both call the same function with recursive flag set to either 0 or 1, so yeah.

[orklah]

So, we just need to decide whether or not we want the return nullable or not for both of them!

Are we sure null is only returned for invalid input that are flagged in Psalm? because we're basically reverting #2925 (and more) that was made a year ago.

[vzz3]

I think it would be better to return nullable, because it is the much more save assumption. By studying the source code of the PHP function it would be possible to know if the functions only return null if one of the input parameters is not an array. However, even if that is the case, we need to be sure that psalm can not overlook a case where one of the input parameters is not an array. Psalm is real great, but not error free...

[orklah]

Well, we better be sure now, because if we aren't, we let bugs pass on PHP8 where it now throws a TypeError. Meanwhile, not putting null allows user not to code a special case that can't happen.

[weirdan]

Are we sure null is only returned for invalid input that are flagged in Psalm

I am fairly sure of that.