Add DynamicReturnType for call_user_func

[vojtech-dobes]

Honestly, I wonder 2 things :)

1. Is there some deeper reason why this isn't implemented yet :) ?
2. How should I write tests to cover this?

I will appreciate a bit of help, but I would like to polish this to highest standard. I would like to also include call_user_func_array.

[ondrejmirtes]

Also, this extension needs to be registered in config.neon

You can test it by adding a new dataProvider to NodeScopeResolverTest::testFileAsserts().

Why it hasn't been implemented yet - because why would you call call_user_func($arg) when you can call $arg() :) So in case of call_user_func people most usually call it arguments that actually make it impossible to infer the return type... But I'd welcome this extension anyway.

[ondrejmirtes]

This solves only a very narrow usecase when you have callable $cb as a parameter typehint. But callables can also be: arrays, strings, objects. This extension could be implemented as:

$args = $functionCall->args; // TODO check $args isn't empty - to prevent crash on `call_user_func()` $firstArg = array_shift($args); return $scope->getType(new FuncCall($firstArg->value, $args));

And all these cases would be taken care of thanks to this logic: https://github.com/phpstan/phpstan-src/blob/master/src/Analyser/MutatingScope.php#L1810-L1821

[vojtech-dobes]

This is nice, thanks.

[ondrejmirtes]

These functions are not exactly 1:1. You need to provide different treatment to call_user_func_array. I'd prefer two different extensions for that.

[ondrejmirtes]

This is wrong. The arguments should be shifted. Only 1-to-last should be passed as actual arguments.

[ondrejmirtes]

Where it matters - you might have some generic function called like this, and these arguments matter for correctly resolved type variable. There should be a test around that. See https://phpstan.org/blog/generics-in-php-using-phpdocs if you're not familiar with generics.

[vojtech-dobes]

Right, I think I understand. I have one question about this: what would be proper way to prepare params for call_user_func_array from $args[1]? Should I map it somehow myself or is there some nice API for it :) ?

[ondrejmirtes]

It makes sense to do it only if $args[1]->value instanceof Array_, otherwise I'd return the default return type.

The logic could look like this:

$args = []; foreach ($args[1]->value->items as $item) { $args[] = new Arg($item->value, $item->byRef, $item->unpack); }

[ondrejmirtes]

This will not work for some edge cases around functions with multiple variants. Best to call ParametersAcceptorSelector::selectFromArgs().

[vojtech-dobes]

Could you drop just few words on what's the significance/meaning/purpose of that? Practically what should I pass as $parametersAcceptors? What I am trying to do with that call?

[ondrejmirtes]

Some weird PHP functions like strtok have multiple variants, basically function overloading. If there's a function that would have different return types based on which variant gets chosen, then the logic that always selects [0] would be wrong.

$parametersAcceptors is $functionReflection->getVariants()

[vojtech-dobes]

@ondrejmirtes I made some improvements. But I am now getting lot of *ERROR* type from the $scope->getType(new FuncCall( construct. Is it cheap for you to see what might be the problem? Thanks for all the help.

[ondrejmirtes]

Nope, I don't know. You should debug where the ErrorType is coming from. Also, coding standard and PHPStan will definitely fail on the code :) You can run it locally too.

[vojtech-dobes]

@ondrejmirtes One more specific question :). When I see that first argument is String_ and I pass it to $scope->getType(new FuncCall( construct, will it there somewhere understand it's supposed to be name of function, or do I have to say that in my extension upfront?

[ondrejmirtes]

@vojtech-dobes You don't have to care about that. You're delegating the type inference of call_user_func($cb) to how $cb() is inferred. And it's taken care of.

There's the AST world and the type system world. The type system can know that even a Variable from the AST can be a ConstantStringType with a specific function name. It's always better to use the typesystem where possible.

I recommended you to check for the Array_ in case of call_user_func_array (the AST world) because otherwise it would be really hard to construct a FuncCall (the AST world) for the Scope::getType() to infer a type. We could do a little bit better so that even code like this would be analysed, but I think it's overkill to do it:

$args = [1, 2, 3]; call_user_func_array('foo', $args);

[vojtech-dobes]

@ondrejmirtes Well, I am currently struggling with normal named functions passed as string.

I have following code:

$callback = array_shift($args)->value; $params = $args; return $scope->getType(new FuncCall($callback, $params));

I am getting ErrorType and if I read my debugging efforts right, it's because it doesn't conclude anywhere along the line the String_ (in $callback) is actually function. This is happening in the call-user-func.php test, with simple function foo() {}.

I am trying to get inspired by ArrayMapFunctionReturnTypeExtension which clearly has to solve similar issues, but can't get it working for:

function fooReturnsBool(): bool { return TRUE; }

/**  * @return bool  */ $returnsBoolInPhpDoc2 = function () { return TRUE; };

/**  * @template T  * @param T $a  * @return T  */ function returnsGeneric($a) { return $a; } assertType('int', call_user_func('returnsGeneric', 1)); assertType('string', call_user_func('returnsGeneric', 'a'));

Not sure whether I should expect these to be actually supported :).

[ondrejmirtes]

This works so I'm not sure what's wrong: https://phpstan.org/r/cd789039-8f3a-4bb0-9582-6d509d5b3476

[vojtech-dobes]

@ondrejmirtes I've added test case for array_map to demonstrate that there simple function passed as string also doesn't work.

And based on that I wonder, would current stay be enough :). Or should I strive for some more improvements?

[vojtech-dobes]

This would suggest that it doesn't work specifically in tests, is there possible reason for that? https://phpstan.org/r/f92e5ce6-0620-4c98-8ab5-c902e9718e70#

[ondrejmirtes]

I'm not sure, it might be:

1. that function foo already exists somewhere else and PHPStan sees a different definition
2. that the function isn't actually loaded and PHPStan does not see it. Maybe try to define it in a different file and require_once that file in the testcase before calling ->assertTypes().

[ondrejmirtes]

Hi, do you plan to finish this PR? :)

[vojtech-dobes]

@ondrejmirtes I would love to, but I haven't really figured out yet where to properly start looking into why there is some issue with named functions and my time budget for this is tight :).

[ondrejmirtes]

Feel free to open a new PR if you decide to work on this again, thanks!