Psalm: incorrectly-ordered parameters to array_map misreported

[SignpostMarv]

When passing array_map() to array_combine() where "correctly"-typed parameters are passed in the wrong order to array_map(), psalm misreports an info-level MixedTypeCoercion, correctly reports an error-level InvalidArgument to argument 2 of array_map(), but doesn't report any InvalidArgument regarding argument 1 of array_map() being expected to be callable.

Ideally:

• if argument 1 to array_map() is not callable, throw up an InvalidArgument
• if calling array_map() with two arguments, the first of which is an array, the second of which is callable, it'd be nice if the MixedTypeCoercion and InvalidArgument errors were suppressed in favour of an "incorrect argument order" error- on the basis that people who work a lot with both array_filter() and array_map() occasionally forget that PHP's type ordering is incredibly inconsistent.

repro:

• https://getpsalm.org/r/47a4288a2e
• using 9f1dc42

ERROR: InvalidArgument - 10:43 - Argument 2 of array_map expects array, callable(string):string provided
INFO: MixedTypeCoercion - 10:15 - Argument 1 of array_combine expects array, parent type array provided

$foo = ['one' => 1, 'two' => 2, 'three' => 3];

/**
* @var callable(string):string
*/
$bar = 'strrev';

array_combine(array_map(array_keys($foo), $bar), $foo);

[weirdan]

• if argument 1 to array_map() is not callable, throw up an InvalidArgument

:+1:

• if calling array_map() with two arguments, the first of which is an array, the second of which is callable, it'd be nice if the MixedTypeCoercion and InvalidArgument errors were suppressed in favour of an "incorrect argument order" error- on the basis that people who work a lot with both array_filter() and array_map() occasionally forget that PHP's type ordering is incredibly inconsistent.

Looks too specific to me. If you're getting two InvalidArgument's it's pretty easy to see you're passed them in the wrong order. Array could also be callable btw, so 'second of which is callable' criteria would yield false positives.

There's also a crash when array_map receives non-callable array as a first parameter: https://getpsalm.org/r/24c056ad43

[SignpostMarv]

re: crash, can the callable be stubbed out?

i.e. callable as Closure|callable-method-string|function-string|array{0:T|class-string<T>, 1:method-string<T>|static-method-string<T>}

where

• callable-method-string follows the format of {class-string<T>}::{method-string<T>}
• method-string is something I've mentioned before elsewhere to be defined as "must be well-formatted for a method name", and method-string<T> is to be defined as "must be a method name on T"
• static-method-string as above, but the method must be statically invokeable from the executed scope
• function-string is "anything that resolves to true if you call function_exists()" ?

see partial implementation without using proposed method/function types: https://getpsalm.org/r/0a7f54f962

[weirdan]

can the callable be stubbed out?

I don't see why would you need that when typed callables are available (docs). Are you maybe looking to define array_map like this: https://getpsalm.org/r/12acfc5375 ?

[weirdan]

@muglug the aforementioned generic version of array_map (https://getpsalm.org/r/12acfc5375) looks pretty complete to me, maybe it makes sense to replace custom array_map handling with a templated definition like that?

Edit: nope, array_map is trickier than that.

[SignpostMarv]

@weirdan strictly defining array_map() would work for array_map(), I'd be suggesting to more strictly define what callable can be so every method accepting callable as a parameter will throw up an issue if it gets given the wrong input.

[weirdan]

@SignpostMarv but callable checks are fine (as you can see with user-defined functions). Psalm just doesn't use them for array_map as it handles this function (and a number of others) in a custom way. See https://github.com/vimeo/psalm/blob/master/src/Psalm/Internal/Analyzer/FunctionAnalyzer.php

[muglug]

There's also a crash when array_map receives non-callable array as a first parameter:

Fixed in d70e295

[muglug]

Fixed in 3d4710c9d303ad390016a3904bf3f03068732937