Issues with splat types / varargs functions

shish

I'm not sure if this is multiple issues or one root cause with multiple symptoms...

array<T>|U is silently converted to T|U
- more specifically and worse, array|T (aka array<mixed>|T is converted to mixed|T which effectively makes the function not typed at all...
when a varargs function is typed using phpdoc, only the first argument gets checked

/**
 * with phpdoc generic types, the type is calculated as int|string
 * also: only the first argument is checked
 * @param array<int>|string $a
 */
function x(...$a) {}
x("foo"); // correct
x([1]); // "argument 1 is expected to be int|string" - wrong, argument 1 is expected to be array<int>|string
x(["moo"]); // "argument 1 is expected to be int|string" - wrong, argument 1 _is_ invalid, but not for that reason
x(1, true); // "argument 2 is fine" - wrong - using this syntax appears to totally disable type checking for argument #2 onwards?? neither array<int>|string nor int|string should allow a bool...

/**
 * with native union types, the type is calculated as array<mixed>|string,
 * which is correct but limited by PHP's untyped arrays
 */
function y(array|string ...$a) {}
y("foo"); // correct
y([1]); // correct
y(["moo"]); // I want to ban this, but plain PHP arrays are untyped :(
y([1], true); // 2nd argument is correctly checked-and-rejected here

/**
 * with both, the type is calculated as array<mixed>|string (the phpdoc is ignored?)
 * @param array<int>|string $a
 */
function z(array|string ...$a) {}
z([1]);
z(["moo"]); // this should be banned
z(42); // this should be banned
z([1], true); // 2nd argument is correctly checked-and-rejected here

/**
 * when phpdoc has an untyped array, devsense replaces "array<mixed>" with "mixed"??
 * @param array|string $a
 */
function w(...$a) {}
w([1]);
w(["moo"]);
w(42); // this should be banned

FWIW I haven't found an example of "type hinting a varargs function which accepts a typed array" within the official PHP docs - I'm deciding "correct" behaviour based on "extrapolating what PHP does when a varargs function accepts an untyped array" plus "what phpstan does"

JakubMisek

shish thank you for the details.

Every code and every framework we have optimized for uses a different notation. Sometimes the vararg parameter is annotated as array in PHPDoc, and sometimes it is not.

So here we guess. Usually, the vararg parameter is annotated as array - because the parameter itself inside the function scope will be array. So the argument is typed as an element of that array.

I'll take a look at this test case, and we'll try to be smarter while supporting the other common notations.

shish

Yeah I can imagine it getting different notations because PHP doesn't have an official standard - it is a pain, such is life in PHP-land ^^

For what it's worth, for my specific use case I don't need a general solution for all cases -- I would be happy with adding one more magic edge-case onto the pile of magic edge-cases, and I think it's an edge-case which makes some things better while not having any negative side effects:

if the type of ...$a is a union, then don't unwrap arrays inside that union (or alternatively "only unwrap arrays if the type-hint is exactly array<...>")

eg, @param array<T> $a is equivalent to function foo(T ...$a) {} (unwrapping the array, as you do now - based on the guess of "the user is trying to type-hint $a within the body of the function, instead of type-hinting the parameter")

but @param array<T>|U $a is equivalent to function foo(array|U ...$a) {} (not unwrapping the array, because it doesn't make sense to unwrap something which is inside a union -- the assumption of "the user is probably trying to type-hint what $a looks like inside the function" doesn't apply here, because $a inside the function isn't array<T|U>, it is array<array<T>|U>)

(I hope that made sense ^^)

JakubMisek

shish Thank you for the details! Good points.

I've added your test case to our internal tests, and adjusted the behavior so it works as expected.

Today, we're releasing a pre-release with the changes, so you can try them out and add more test cases if necessary.