Rule idea: validate that method names returned in implementations of EventSubscriberInterface are valid

[stof]

When implementing the EventSubscriberInterface of Symfony, the structure of the array being returned from getSubscribedEvents is already validated by phpstan thanks to the phpdoc return type, (see https://github.com/symfony/symfony/blob/7.3/src/Symfony/Component/EventDispatcher/EventSubscriberInterface.php) except for one requirement: the fact that the subscribed listener is the name of a public method of the subscriber class.

It would be great to provide a custom rule comparing those method names to the list of public methods of the class.

[ondrejmirtes]

For more clarity it'd be good to show examples of code that should and should not be flagged by this rule.

[stof]

The phpdoc of the interface shows the 3 kind of structure we can have in the returned array (different keys of the array might use different structures if they have different needs regarding the priority or the number of listeners).

When the method name is a literal string (probably >99.9% of usages, as the return value cannot be dynamic anyway), the rule would report errors in 2 cases:

• when the class does not have a method named like that string literal
• when the method is not public

namespace App;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;

class BrokenListener implements EventSubscriberInterface
{
    public static function getSubscribedEvents(): array
    {
        return [
            'first_event' => 'nonExistentMethod', // broken because "nonExistentMethod" is not a method of that class
            'second_event' => ['handleEvent', 12], // broken because "handleEvent" is not public
        ];
    }

    protected function handleEvent() {}
}

[stof]

For completeness, phpstan does not currently detect all mistakes in the returned structure when enabling phpstan-symfony because the stubs are overriding the upstream return type. https://github.com/phpstan/phpstan-symfony/pull/435 is about fixing it.

[ondrejmirtes]

What's the significance of ['handleEvent', 12]?

[stof]

Register handleEvent is the event listener, with priority 12

I'm quoting the method documentation here (from the link in the opening issue):

The array keys are event names and the value can be:

• The method name to call (priority defaults to 0)
• An array composed of the method name to call and the priority
• An array of arrays composed of the method names to call and respective priorities, or 0 if unset

For instance:

• ['eventName' => 'methodName']
• ['eventName' => ['methodName', $priority]]
• ['eventName' => [['methodName1', $priority], ['methodName2']]]

[ondrejmirtes]

All of these are important for the theoretical rule here.

[stof]

Another alternative would be for phpstan to implement a pseudo-type public-method-of<T of object> (name to be bikeshed), which would allow to describe the contract fully in the return type (by replacing string with public-method-of<static>) and have phpstan enforce that. public-method-of<T of object> would be the union of strings for the names of public methods in the class T.

such pseudo type might be useful for other kinds of APIs where a method name is referenced as a string.