driftingly / rector-laravel

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

Add rule to convert `MyJob::dispatch()` to `dispatch(new MyJob())` #193

Closed bram-pkg closed 4 months ago

bram-pkg commented 4 months ago

It would be nice if we could automatically convert MyJob::dispatch() or MyEvent::dispatch() to the more static-analysis-friendly version of using the dispatch(new MyJob()) and event(new MyEvent())` syntax.

-ExampleEvent::dispatch($email);
+event(new ExampleEvent($email));

-ExampleJob::dispatch($email);
+dispatch(new ExampleJob($email));
bram-pkg commented 4 months ago

I've extracted the test classes to a Source directory, re-requesting review. Thanks for the rapid response :)

bram-pkg commented 4 months ago

That's odd, the tests fail if I remove them, even if I add importNames to the config. Any idea why that could be happening?

bram-pkg commented 4 months ago
Details

``` rector-laravel git:add-dispatch-rule* ❯ vendor/bin/phpunit PHPUnit 10.5.11 by Sebastian Bergmann and contributors. Runtime: PHP 8.3.3 Configuration: /Users/bram/src/github/rector-laravel/phpunit.xml ............................................................... 63 / 218 ( 28%) ............................................................... 126 / 218 ( 57%) ............................................................... 189 / 218 ( 86%) FF........................... 218 / 218 (100%) Time: 00:02.885, Memory: 104.00 MB There were 2 failures: 1) RectorLaravel\Tests\Rector\StaticCall\DispatchToHelperFunctionsRector\DispatchToHelperFunctionsRectorTest::test with data set #0 ('/Users/bram/src/github/rector...hp.inc') Failed on fixture file "bus_dispatchable.php.inc" Failed asserting that string matches format description. --- Expected +++ Actual @@ @@ use RectorLaravel\Tests\Rector\StaticCall\DispatchToHelperFunctionsRector\Source\TestJob; -dispatch(new TestJob('param1', 'param2')); +dispatch(new RectorLaravel\Tests\Rector\StaticCall\DispatchToHelperFunctionsRector\Source\TestJob('param1', 'param2')); /Users/bram/src/github/rector-laravel/vendor/rector/rector/src/Testing/PHPUnit/AbstractRectorTestCase.php:193 /Users/bram/src/github/rector-laravel/vendor/rector/rector/src/Testing/PHPUnit/AbstractRectorTestCase.php:137 /Users/bram/src/github/rector-laravel/tests/Rector/StaticCall/DispatchToHelperFunctionsRector/DispatchToHelperFunctionsRectorTest.php:22 2) RectorLaravel\Tests\Rector\StaticCall\DispatchToHelperFunctionsRector\DispatchToHelperFunctionsRectorTest::test with data set #1 ('/Users/bram/src/github/rector...hp.inc') Failed on fixture file "event_dispatchable.php.inc" Failed asserting that string matches format description. --- Expected +++ Actual @@ @@ use RectorLaravel\Tests\Rector\StaticCall\DispatchToHelperFunctionsRector\Source\TestEvent; -event(new TestEvent('param1', 'param2')); +event(new RectorLaravel\Tests\Rector\StaticCall\DispatchToHelperFunctionsRector\Source\TestEvent('param1', 'param2')); /Users/bram/src/github/rector-laravel/vendor/rector/rector/src/Testing/PHPUnit/AbstractRectorTestCase.php:193 /Users/bram/src/github/rector-laravel/vendor/rector/rector/src/Testing/PHPUnit/AbstractRectorTestCase.php:137 /Users/bram/src/github/rector-laravel/tests/Rector/StaticCall/DispatchToHelperFunctionsRector/DispatchToHelperFunctionsRectorTest.php:22 FAILURES! Tests: 218, Assertions: 280, Failures: 2. ```

peterfox commented 4 months ago

@bram-pkg you've only updated the original and not the transformed in those tests. Sorry I should provided a suggestion covering both.

Ignore me 😅 I misspoke with the original comment. Rector will always add the fully qualified name as it typically doesn't know if the class has been imported to the namespace or not. Just ignore my original comments. It's good to merge.

bram-pkg commented 4 months ago

Okay, thanks for clearing that up 😅 I was quite confused why it wouldn't remove the FQDN even with importNames().

Glad it's all good now :) Cheers

bram-pkg commented 4 months ago

Did you forget to merge this? @peterfox :D