8000 [EventDispatcher] Declare return type of `getSubscribedEvents()` by Warxcell · Pull Request #41995 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[EventDispatcher] Declare return type of getSubscribedEvents() #41995

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 14, 2021
Merged

[EventDispatcher] Declare return type of getSubscribedEvents() #41995

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Symfony/Component/EventDispatcher/EventSubscriberInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ interface EventSubscriberInterface
* The code must not depend on runtime state as it will only be called at compile time.
* All logic depending on runtime state must be put into the individual methods handling the events.
*
* @return array The event names to listen to
* @return array<string, mixed> The event names to listen to
Copy link
Contributor
@sstok sstok Jul 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use something more explicit than mixed? Personally I try to use union's as much as possible.
Like array<string, string|array{0: string}|array{0: string, 1: int}|array<int, array{0: string}|array{0: string, 1: int}>>

And yes it's a bit verbose 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's actually not the right signature. the right one is this:

array<string, string|array{0: string, 1?: int}|list<array{0: string, 1?: int}>>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, you edited in the meantime. Your new proposal is too verbose due to not using the optional key for the priority
8000

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This notation is not understood by all tools yet. I think the current type is precise enough.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL the list pseudo-type 😄 and optional keys.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://psalm.dev/r/0e971e1c02 but for some reason it doesn't like it :D

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't use @template for this (as that's a generic and implies the return type), you need to define a custom type instead but Pslam doesn't support this it seems.

I only use PHPStan atm because Psalm doesn't support PHP 8 yet 😞

*/
public static function getSubscribedEvents();
}
0