Enforce optional parameters in callable types

[rvanvelzen]

Fixes phpstan/phpstan#7320

[ondrejmirtes]

The test assertions from the failures probably need updating, otherwise 👍 :)

[ondrejmirtes]

The test assertions from the failures probably need updating, otherwise 👍 :)

[ondrejmirtes]

Thank you!

[ondrejmirtes]

It's so annoying but I have to revert this. In a test project I'm running each new PHPStan commit on, this change caused 81 new errors. And it's because of these annotations in doctrine/collections (the same mistake is repeated above multiple methods):

    /**
     * Tests for the existence of an element that satisfies the given predicate.
     *
     * @param Closure $p The predicate.
     * @psalm-param Closure(TKey=, T=):bool $p
     *
     * @return bool TRUE if the predicate is TRUE for at least one element, FALSE otherwise.
     */
    public function exists(Closure $p);

This means that passing Closure(int|string, TValue): bool into this method is not valid. Although in reality the code always calls the closure with two arguments...

[ondrejmirtes]

So some research needs to be done:

1. Does Closure(TKey=, T=):bool mean the same thing in Psalm and PHPStan?
2. Should the Doctrine Collections annotations be fixed?

[ondrejmirtes]

Also there's number of issues phpstan-bot commented on because playground samples changed. All of this tells me impact on the ecosystem would be too huge even if we're right. Might be better to save a fix like this for a major version.

[rvanvelzen]

I think most of the issues the bot found were due to the describe() change.

I'll research this for a bit soon.

[rvanvelzen]

Psalm does not consider this to be an issue, but I think it's pretty clear that it is. I believe the annotations in Doctrine Collections are wrong.

Specifically: callable(T=) can be called with either 0 or 1 parameter, but passing in callable(T) will require 1 parameter. This is what this PR would fix.

However, I agree that this change could be overly disruptive. An option might be to put it in bleedingEdge?

[ondrejmirtes]

Can you please open a Psalm issue to see what the maintainers think about this? Thanks :)

Bleeding edge - I considered it, but this is deep in the typesystem so there would have to be some kind of global static singleton toggle to query... (There was once in the past for similar purpose).

[greg0ire]

FYI doctrine/collections 1.7.2 no longer has this issue.

[stof]

FYI, I opened the psalm issue at vimeo/psalm#8438

[ondrejmirtes]

Readded this again: 45404d9