[TypeProvider] Provide argument types during FunctionParamsProviderEvent

[veewee]

Discussed in #7212

When adding following code in both a FunctionParamsProviderInterface and FunctionReturnTypeProviderInterface event handler.

$nodeTypeProvider = $event->getStatementsSource()->getNodeTypeProvider();
$args = $event->getCallArgs();

$type = $nodeTypeProvider->getType($args[0]->value);

• The getFunctionParams hook always returns null on this call.
• The getFunctionReturnType hook always returns the detected type on this call. Something like impure-Closure(string):int

This PR mostly rearranges some code so that the ArgumentsAnalyzer runs before the FunctionParamsProviderInterface hook is called.

[orklah]

Can you check why https://psalm.dev/r/ab6674485b is detected? If you fixed a bug that was making args not being analyzed in existing functions, InvalidArgument should not be here. We must be missing something

EDIT: I'm flagging this as draft to better see PR that are ready

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/ab6674485b

<?php

function test(int $i){}

test(new stdClass);

Psalm output (using commit 9ac77e9):

INFO: UnusedParam - 3:19 - Param $i is never referenced in this method

INFO: MissingReturnType - 3:10 - Method test does not have a return type, expecting void

ERROR: InvalidArgument - 5:6 - Argument 1 of test expects int, stdClass provided

[veewee]

I'll investigate that case deeper somewhere next week.
My best guess is because the ArgumentsAnalyzer is ran again before trying to detect a return type.

[orklah]

If it is, then it's running better the second time because it doesn't have false positive, so if we find the difference, we may find our solution

[veewee]

@orklah : What I found for now is that the given code runs on the 4.X branch:

• FunctionCallAnalyzer::analyze()
• It calls to logic of FunctionCallAnalyzer::getAnalyzeNamedExpression() on L129
  • This is the place where this PR alters code.
  • The ArgumentsAnalyzer on L500 is not being called here, since the function exists
  • It does not try to run a plugin, since the functionId has no registered plugin.
    • Even if it would, it would not contain the analyzed arguments
  • The function_params are injected by the storage on L546
  • This function returns FunctionCallInfo with the function params
• Next, the arguments analyzer is ran back in the regular analyze funciton on line L175
  • The detected function_params are being passed to the ArgumentsAnalyzer on L177

So in this case, the ArgumentsAnalyzer is ran in a different place (L175) and it contains the function_params. The other ArgumentsAnalyzer call on L502 is never really triggered and does not contain any function_params.

Not providing any function_params, to the ArgumentsAnalyzer makes it run very different analyzing code.
See L122. Down in the evaluateArbitraryParam() method, I've added support for the additional ArrowFunction expression.

So to summarize in short: The ArgumentsAnalyzer is currently never being triggered before the FunctionParamsProviderInterface is being triggered.

I'm feeling a bit too noob in this codebase to make the right decisions at this point. Any help is appreciated :)

[orklah]

Ok, so there's two things that troubles me and both can be solved in one go:

• when the function exists, the arguments are analyzed later
• when the function name is an expression(

  psalm/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php

  Line 128 in b9d8dd9

  if ($function_name instanceof PhpParser\Node\Expr) {

  ), we go through getAnalyzeNamedExpression instead of handleNamedFunction and only the latter calls the plugins (so something like $a = 'foo'; $a($arg); won't ever call the plugins

I think both of these issues could be solved by moving this condition:

psalm/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php

Line 525 in b9d8dd9

if ($function_call_info->function_exists) {

to here:

psalm/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php

Line 189 in b9d8dd9

That way, the plugin will be always called and it should have arguments analyzed.

Can you try that?

[orklah]

Thinking about it a little bit more, I think we have a circular issue here...

Analyzing the args require knowing the params, so it's hard to know the args types when retrieving the params.

What this means is:

• either we can make ArgumentsAnalyzer::analyze stop depending on params maybe extract whatever uses params in there and put that after calling the plugins
• or we're stuck with current plugin hook and we would need an entire new set of hooks to be able to choose if we cant to be before or after arg populating

Just by curiosity, what was your goal in having args in order to provide params?

[veewee]

The goal of the plugin is to finetune variadic params.
So for example: you have following function:

$stages = pipe(
    fn (string $x): int => 2,
    fn (string $y): float => 1.2,
    fn (float $z): int => 23
);
$res = $stages('hello');

It passes 'hello' to function 1 and then pipes the results to the other function, resulting in 23.
As you can see, the type of short closure 2 is incorrect : it should be int -> float.

It would be handy to have the analyzed types there, because it makes it easier to follow the arguments of the provided closures and to calculate the parameters of the stages closures and the return type.

I am also planning to write a plugin for a curry function. Also in this case it would be handy, because the provided arguments are depending of the type of the first provided argument:

curry('array_map', $someCallback)=> array -> array

In this case we need to know from the array_map callable what the arguments are so that we can check that argument 2 matches the once from array_map.

If you would want to do something similar right now, you have to parse all possible expressions yourselves - which surely cannot be the intention for plugins to do so.

[veewee]

@orklah Found something that works!

So what changed:

• It now first runs the function analysis as it would without a FunctionParamsProviderInterface plugin.
• Next it uses those resolved params as input for the ArgumentsAnalyzer
• Next it calls the FunctionParamsProviderInterface::getFunctionParams() methods
• This one can access the resolved argument types through the NodeTypeProvider, making it possible to alter them from within the plugin

@klimick :
I think it also solve #7450, since it first runs the regular code - before the plugin is triggered?

This change now results in the pipe plugin to be able to collect type information like this:

/**
 * @param array<array-key, Arg> $args
 * @return StagesOrEmpty
 */
private static function parseStages(StatementsSource $source, array $args): array
{
    $stages = [];
    foreach ($args as $arg) {
        $stage = $arg->value;

        $nodeTypeProvider = $source->getNodeTypeProvider();
        $stageType = $nodeTypeProvider->getType($stage)?->getSingleAtomic();
        if (!$stageType instanceof TClosure) {
            $stages[] = [Type::getMixed(), Type::getMixed(), 'input'];
            continue;
        }

        $params = $stageType->params;
        $firstParam = reset($params);

        $paramName = $firstParam->name ?? 'input';
        $in = self::determineValidatedStageInputParam($source, $stage, $stageType);
        $out = $stageType->return_type ?? Type::getMixed();

        $stages[] = [$in, $out, $paramName];
    }

    return $stages;
}

Context : https://github.com/veewee/pipe-plugin/blob/af08fb95c901e7617f178dfdc2b3782c9865340f/src/Psalm/PipeArgumentsProvider.php#L96-L116

[orklah]

Seems legit! Thanks!

[drupol]

Wooooooooooooooooh !