8000 [Messenger] #[AsMessageHandler] with union type doesn't compile · Issue #47206 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Messenger] #[AsMessageHandler] with union type doesn't compile #47206

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

Jump to bottom
Closed
alex-dev opened this issue Aug 5, 2022 · 4 comments
Closed

[Messenger] #[AsMessageHandler] with union type doesn't compile #47206

alex-dev opened this issue Aug 5, 2022 · 4 comments
Labels
Bug Messenger Status: Needs Review

Comments

@alex-dev
Copy link
Contributor
alex-dev commented Aug 5, 2022 •
edited
Loading

Symfony version(s) affected

6.0

Description

Having a #[AsMessageHandler] on a method (except __invoke) taking a union will prevent compilation.

How to reproduce

#[AsMessageHandler]
public function method(A|B $message): void {}

Possible Solution

In the MessengerPass::guessHandledClasses() method, every path except the one dealing with union types return if method is known array<class-string, method> else array<int, class-string>. Code path dealing with unions does not.
@alex-dev alex-dev added the Bug label Aug 5, 2022
@xabbuh
Copy link
Member
xabbuh commented Aug 5, 2022

Can you create a small example application that allows to reproduce your issue?

ArchitectNate added a commit to ArchitectNate/symfony-47206 that referenced this issue Aug 8, 2022
@ArchitectNate
Symfony Bug Replication for symfony/symfony#47206
4344890
@ArchitectNate
Copy link
Contributor
ArchitectNate commented Aug 8, 2022 •
edited
Loading

I've replicated the bug here: https://github.com/ArchitectNate/symfony-47206

I'll start digging into a fix

Status: Needs Work

@ArchitectNate ArchitectNate mentioned this issue Aug 9, 2022
Merged
@magikid
Copy link
Contributor
magikid commented Aug 17, 2022

Status: Needs Review

@ArchitectNate
Copy link
Contributor

@xabbuh is the example application and subsequent PR sufficient? Thoughts?

@fabpot fabpot closed this as completed Sep 13, 2022
fabpot added a commit that referenced this issue Sep 13, 2022
@fabpot
bug #47227 [Messenger] Support for custom handler method containing a…
d6bde21 
… Union type tagged with #[AsMessageHandler] (ArchitectNate)

This PR was squashed before being merged into the 6.1 branch.

Discussion
----------

[Messenger] Support for custom handler method containing a Union type tagged with #[AsMessageHandler]

| Q             | A
| ------------- | ---
| Branch?       | 6.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #47206
| License       | MIT
| Doc PR        | No

When Union Types were implemented for the #[AsMessageHandler] attribute, it wasn't designed with non `__invoke` methods in mind.

This fix detects if we are using `__invoke` or not and constructs the return to match the rest of the logic, mainly it allows for a `type => methodName` map if `__invoke` is not being used.

Commits
-------

f38d8f8 [Messenger] Support for custom handler method containing a Union type tagged with #[AsMessageHandler]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Messenger Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @magikid @ArchitectNate @xabbuh @carsonbot @alex-dev
0