Skip to content
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.

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

Add missing renames for dispatchNow method on Jobs #219

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

GeniJaho
Copy link
Collaborator

@GeniJaho GeniJaho commented May 7, 2024

Fixes #215.

@peterfox
Copy link
Collaborator

peterfox commented May 8, 2024

Have you tested this? It might not work as Dispatchables is a trait if I remember correctly and won't be picked up in the same way PHPStan does with Classes and Interfaces because it's not a type.

@GeniJaho
Copy link
Collaborator Author

GeniJaho commented May 8, 2024

@peterfox Yes, both in a real project and with tests in the rector-laravel code. The RenameMethodRector takes trait uses into account when we provide traits instead of regular classes to it.

It would be great if we could add tests for the sets instead of just the rules. Not sure if this has been mentioned or considered before in the core Rector repo.

@peterfox
Copy link
Collaborator

peterfox commented May 8, 2024

@GeniJaho okay good to know, I know when I've used PHPStan directly I found it didn't find traits so wanted to double check.

I created a folder for testing Sets if you're asking if it's possible? It's just creating a config that points to the sets. Tests are otherwise the same as rule tests.

@GeniJaho
Copy link
Collaborator Author

@peterfox that's exactly what I was looking for, thank you. I didn't know we already had them.

I added tests for the Laravel 10 Set and fixed a 'bad' configuration for renaming $routeMiddleware in the Http Kernel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dispatchNow is not converted to dispatchSync
2 participants