How to provide a templated param through the plugin system?

[veewee]

Hello,

Working on a little plugin over here that introspects the parameters of callable chains in a pipe combinator function:

https://github.com/azjezz/psl/blob/2.0.x/src/Psl/Fun/pipe.php

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

It inspects that the output type of the first function matchtes the input of the second function etc.
It works pretty well (for as far as I know):

https://gist.github.com/veewee/db5b2ad4868d9739732acafbbf7ef2f5

There is a supported case in which no stages are provided to the function:

$x = pipe()('hello');
> $x = string('hello')

$y = pipe()(123);
> $y = int(123)

In that case, the function just returns its input: callable (T): T
However - I am not able to return this templated version from the plugin.
See https://gist.github.com/veewee/db5b2ad4868d9739732acafbbf7ef2f5#file-pipeargumentsprovider-php-L76-L94

This is the input:

use function Psl\Fun\pipe;

$stages = pipe();
$res = $stages('hello');

/** @psalm-trace $stages */
/** @psalm-trace $res */

Tried various options like:

$templated = new Type\Union([
    new Type\Atomic\TTemplateParam($name, new Type\Union([Type\Atomic::create('mixed')]), 'fn-'.$event->getFunctionId())
]);

return new Type\Union([
    new Type\Atomic\TClosure(
        value: Closure::class,
        params: [
            new FunctionLikeParameter(
                'input',
                false,
                $templated,
                is_optional: false,
                is_nullable: false,
                is_variadic: false,
            )
        ],
        return_type: $templated,
    )
]);

Output:

ERROR: UnusedVariable - ../tests/static-analysis/Fun/pipe.php:6:1 - $res is never referenced or the value is not used (see https://psalm.dev/077)
$res = $stages('hello');


ERROR: InvalidArgument - ../tests/static-analysis/Fun/pipe.php:6:16 - Argument 1 expects T:fn-psl\fun\pipe as mixed, "hello" provided (see https://psalm.dev/004)
$res = $stages('hello');


ERROR: Trace - ../tests/static-analysis/Fun/pipe.php:9:0 - $res: T:fn-psl\fun\pipe as mixed (see https://psalm.dev/224)
/** @psalm-trace $res */

I would expect psalm to know it is a string in this case.

An clue on how I can configure the plugin to make it work with templated parameters so that it knows the end result?

Context:

• PHP 81
• Psalm v4.15.0@a1b5e489e6fcebe40cb804793d964e99fc347820

[veewee]

Played around with this a bit more and haven't really found a solution.
As far as I can tell, the returned template type mentioned above is correct.

It looks like an issue in which the FuncCall to $stages isn't replacing the TTemplateParam with a TemplateResult if the template type is applied through a plugin.

Not sure if I need to / can set something in e.g. the parent context, codebase, ... object.

[orklah]

Thanks, I'll check that!

First class callable issue will possibly get fixed by #7232. It could be helpful to check if it is if you have time to spare

[orklah]

Psalm seems to be missing support for templates resolution in closures it seems. Here's an example without using your plugin that does exactly the same thing: https://psalm.dev/r/0df3da05ef

[veewee]

Ah ok, tried callable's as well which has the same issues.

Is that something to add to the road map and shall I create an issue for that?
I just wen through the list and saw some similar / related issues.

Past week, I also tries something along these lines for this specific case: - but haven't found something that makes it work.

public static function afterExpressionAnalysis(AfterExpressionAnalysisEvent $event): ?bool
    {
        $source = $event->getStatementsSource();
        $templateMap = $source->getTemplateTypeMap();
        $typeProvider = $event->getStatementsSource()->getNodeTypeProvider();
        $expr = $event->getExpr();

        if (!$templateMap || !$expr instanceof FuncCall) {
            return null;
        }

        $type = $typeProvider->getType($expr);
        if (!$type || $type->hasCallableType()) {
            return null;
        }

        $emptyPipeTemplate = null;
        foreach ($type->getTemplateTypes() as $template) {
            if (str_ends_with($template->getKey(), ':fn-psl\\fun\\pipe')) {
                $emptyPipeTemplate = $template;
                break;
            }
        }
        if (!$emptyPipeTemplate) {
            return null;
        }

        $args = $expr->getArgs();
        $returnType = count($args) ? self::createSimpleType($args[0]?->value->getType() ?? 'mixed') : self::createSimpleType('empty');

        $name = $emptyPipeTemplate->param_name;
        $firstClass = $emptyPipeTemplate->defining_class;

        TemplateInferredTypeReplacer::replace(
            $type,
            $res = new TemplateResult(
                [$name => [$firstClass => $type]],
                [$name => [$firstClass => $returnType]]
            ),
            $event->getCodebase()
        );

        return null;
    }

[orklah]

yeah, please create an issue so we can remember this issue exists.

[veewee]

done -> #7244

The first class callable syntax from #7232 also seems to not crash psalm now.
Just new stuff to take into consideration in the plugin :)

[orklah]

I never tried something like that. However, I noticed that $event->getFunctionId() does not return a lowercase-string and this code transform the function id in lowercase in the third arg:

psalm/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php

Line 445 in c0adde8

$template_function_id = 'fn-' . strtolower($cased_function_id);

Could it be related somehow?

[orklah]

nevermind, your function is already in lowercase so this wouldn't solve anything anyway

[veewee]

@orklah Small side question related to above plugin.
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

Would it make sense to somehow enable the detection of the resulting union type from the getFunctionParams hook before it is triggered as well or is this something that won't be possible by design?

[orklah]

I'd say it's most probably a bug, ParamsTypeProviders are not frequently used.

Theoretically, args should have been analyzed here:

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

Line 500 in b9d8dd9

if (ArgumentsAnalyzer::analyze(

. A few lines before the call to getFunctionParams which will make the call to the plugin

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

Line 527 in b9d8dd9

$function_call_info->function_params = $codebase->functions->params_provider->getFunctionParams(

Maybe there's something preventing getting into the ArgumentsAnalyzer::analyze in your case?

[veewee]

Ah yes, spotted some bugs in there. Will investigate it further and create a PR. Thanks!

[veewee]

@orklah Found some solutions, but it results in quite some issues.
Not sure if this is the right way:

Code changes : https://github.com/vimeo/psalm/compare/4.x...veewee:function-params-provider-improvements?expand=1
issues : https://github.com/veewee/psalm/actions/runs/1698237121

Previously, the analyzer only ran if the function did not exist.
Now it runs when the function does not exist as well.
During the tests, I can see it frequently results in TheType|mixed to be detected instead of TheType only.
Not sure how I can fix this.

Shall I just open the PR so that we can discuss it in there or are you willing to take a deeper look yourself? :)

[orklah]

I'm not sure I understand where the invalid arg is coming from in this example https://psalm.dev/r/ab6674485b if we only analyze functions that does not exists.

Please create the PR, we'll look at that

[veewee]

Allright -> #7394