driftingly / rector-laravel

Rector upgrades rules for Laravel
http://getrector.org
MIT License
469 stars 48 forks source link

Use FullyQualified for class instead of Name #225

Closed megawubs closed 2 weeks ago

megawubs commented 3 weeks ago

This ensures a leading slash \ is added to the namespace of the class that is newed up, ensuring the namespace is loaded from the root and not relative to the current namespace.

Currently this rector breaks my code as it changes the code into something that does not work.

@@ @@
     public function handleTenant(TenantWithDatabase $tenant): void
     {
         $tenant->run(function (): void {
-            CreateTenantBrandingAvatarsJob::dispatch()->onQueue(Queue::CENTRAL);
+            dispatch(new App\Central\Jobs\CreateTenantBrandingAvatarsJob())->onQueue(Queue::CENTRAL);
         });
     }
 }
    ----------- end diff -----------

Applied rules:
 * DispatchToHelperFunctionsRector

Note the missing \ before App. This results in a namespace that can not be resolved. And broken tests

image

But with the leading slash, it's correct.

image

megawubs commented 3 weeks ago

I've tried running composer fix-cs but this changes a lot of code that I did not touch. I don't know if it's required before merge?

TomasVotruba commented 2 weeks ago

Indeed, this is best practice to work with names 👍

GeniJaho commented 2 weeks ago

@megawubs Thanks for the PR and sorry for the late reply. We've just fixed the code style issue and can merge your PR. Can you please run composer fix-cs once more?

megawubs commented 2 weeks ago

I've just updated the code and ran composer fix-cs