[EventDispatcher] Add type-hints to EventDispatcherInterface #31984
Conversation
3b8c6b8 to
d93f1d0
Compare
| @@ -139,7 +139,7 @@ public function hasListeners($eventName = null) | ||
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function addListener($eventName, $listener, $priority = 0) | ||
| public function addListener(string $eventName, $listener, int $priority = 0) |
There was a problem hiding this comment.
Note: I intentionally did not add callable $listener because this would cause a call like…
$dispatcher->addListener('some_event', ['Some\Class', 'someMethod']);… to trigger autoloading for the class name that is passed here. This might have performance implications if we're adding a lot of static listeners.
d93f1d0 to
2bc9472
Compare
Certainly.
I'm not aware of any tool that would not require thorough human review after processing the code. This PR was a manual process. I would volunteer to iterate over the components that I'm familiar with and open similar PRs for them, if you're interested. |
|
Thanks, that'd be great! |
|
Should we open an issue with a checklist of what to look for so we can invite the whole community to check all components? |
I've created #32179 for this purpose. |
|
Thank you @derrabus. |
…face (derrabus) This PR was merged into the 5.0-dev branch. Discussion ---------- [EventDispatcher] Add type-hints to EventDispatcherInterface | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #32179 | License | MIT | Doc PR | N/A This PR adds type-hints to `EventDispatcherInterface` to give it a more php7-like feeling. Adding these type-hints on master should not be a breaking change because we require php 7.2 there, which allows downstream classes/interfaces to remove a type-hint from a method signature. Commits ------- 2bc9472 [EventDispatcher] Add type-hints to EventDispatcherInterface.
This PR adds type-hints to
EventDispatcherInterfaceto give it a more php7-like feeling. Adding these type-hints on master should not be a breaking change because we require php 7.2 there, which allows downstream classes/interfaces to remove a type-hint from a method signature.