Add generic annotations to `EventInterface`

[villermen]

Q	A
Documentation	yes
Bugfix	no
BC Break	no
New Feature	no
RFC	no
QA	no

Description

It would be cool if you could somehow hint at the types of an event's target and params that IDEs and static code checking tools can work with. I've added a rough outline of what I mean in the PR using PHPStan-targeted annotations.

This way you can define the target and parameter types of an event in one go instead of doing it on each line the event is accessed:

public function onSomeEvent(EventInterface $event): void
{
    /** @var Element|null $element */
    $element = $event->getTarget();
    /** @var array{foo?: string} */
    $params = $event->getParams();

    $this->doSomethingWithFoo($params['foo']); // IDE can provide autocompletion and type recognition for "foo".
}

becomes

/**
 * @param EventInterface<Element|null, array{foo?: string}>
 */
public function onSomeEvent(EventInterface $event): void
{    
    $this->doSomethingWithFoo($event->getParams()['foo']); // IDE can provide autocompletion and type recognition for "foo".
}

This leaves the type annotation code in the main docblock of the method.

Of course you could also create an actual native-class for each event. That's more type-safe, but creates a lot of additional overhead. I can totally understand if that's the desired approach too, so take this is a suggestion only =)

[villermen]

Took care of the checks =)

Looks like Event correctly picks up on the variables passed in __construct(), setTarget() and setParams().

I was sadly unable to extend the template when calling setParam(). That ultimately leads to getParams() and getParam() only being able to check on the type specified or inferred from setParams(). So I'm not entirely happy with the result but it's the best I could muster.

[Ocramius]

BTW, vimeo/psalm:^5 now installed (if you rebase).

It will either help or hurt :D

[villermen]

I'd prefer to see the psalm tests in a subdirectory of /test/ [..]

@gsteel Actually agreed with you there, but then I noticed "benchmarks/". That's also a testing directory separate from "test/" so I stuck to that pattern. Ended up being happier with this situation because these classes are not necessarily tests that have to be run by PHPUnit. Will correct when you still prefer adding below "test/".

[gsteel]

I'd prefer to see the psalm tests in a subdirectory of /test/ [..]

Will correct when you still prefer adding below "test/".

I don't have strong opinions on this @villermen - it's all good as long as it's all export ignored and subject to the same CS rules etc 👍

[Ocramius]

I see that you opted for &static: think that will suffice? 🤔

[villermen]

think that will suffice? 🤔

@Ocramius I think it does! After CheckEvent::setParams() or CheckEvent::setTarget() are called the type will become Event<NewTTarget, NewTParams&CheckEvent when using either &$this or &static.

I've also looked into getting it to work without this-out, but it become very cumbersome to deal with then. Ctor inference sets a type that's too narrow, setters can't use template types. The best we can do in that case is defining the templates and using them on the getters.

[Ocramius]

Thanks @villermen!