Avoid false positive about array which are non-callable

[VincentLanglet]

From discussion #10839 (comment)

This should be valid, cf https://3v4l.org/JZgNh

[kkmuffme]

I get the point of this PR and it's good, the question is since the array{class-string, string} type generally indicates a callable.
Are we ok with it not catching obvious errors anymore in that case?
e.g. these wouldn't report an error anymore too, however practically these would be bugs.

foo([\DateTime::class, "formt"]);
foo([\DateTime::class, "doesntExist"]);

[VincentLanglet]

I get the point of this PR and it's good, the question is since the array{class-string, string} type generally indicates a callable.

generally but not always. If someone wants to indicate a callable, he HAVE TO use the right type callable.

Are we ok with it not catching obvious errors anymore in that case?

The opposite question would be "Are we ok to return false positive for people using correctly the phpdoc in order to keep catching errors for people which used the wrong phpdoc". I would say NEVER.

And if we're doing nothing we'll end with a lot of issue like #10936 which makes psalm unusable with Symfony/Twig.

I agree catching a wrong method could be great, but first the false-positive need to be fixed.

[kkmuffme]

Agree, 2 things that come to my mind at this point:

• if we do that for arrays, this should also be changed for strings (e.g. callable|string) since it's the same issue (in the PR/same file just a bit further down in the elseif). Can you update this in the PR?
• can you confirm that this will still report an error with this PR:

/**
 * @param int|callable $arg
 */
function foo($arg): void {}
foo([\DateTime::class, "errorHere"]);',

[VincentLanglet]

Agree, 2 things that come to my mind at this point:

• if we do that for arrays, this should also be changed for strings (e.g. callable|string) since it's the same issue (in the PR/same file just a bit further down in the elseif). Can you update this in the PR?
• can you confirm that this will still report an error with this PR:

/**
 * @param int|callable $arg
 */
function foo($arg): void {}
foo([\DateTime::class, "errorHere"]);',

Both are added in the last commit.

[weirdan]

Thanks!

[VincentLanglet]

Would it be ok to make a new patch release @weirdan ? This PR fix a lot of false positive for symfony/twig users.

[michnovka]

This PR did not fix the issue with setting protected function as callable.

https://psalm.dev/r/3c405968f1

This code reports no error on psalm.dev, as it uses outdated version there. But with 5.24 and 5.25 it complains that

setCode expects a public callable, but a non-public callable provided (see https://psalm.dev/004)

However setting a protected function this way is totally fine. The issue would be setting a private function

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/3c405968f1

<?php

class Command{
    /**
     * @param callable $code
     */
	public function setCode(callable $code): static
    {
        return $this;
    }
}

class Code extends Command{
    public function __construct(){
        parent::setCode([$this, 'runCode']);
    }
    
    protected function runCode(): void
    {}
}

Psalm output (using commit 16b24bd):

No issues!