Modernising and refactoring PHP with Rector

Posted: 2025-01-26 12:31:00 by Alasdair Keyes

Direct Link |

---

I've recently heard some good things about the PHP tool Rector (https://getrector.com/). It provides automatic refactoring for your PHP code to increase code quality.

This website (https://www.akeyes.co.uk) is running on PHP 8.x, previously 7.x and with a quick check of my composer.json in Git, I can see that it was even running on PHP 5.6 when the site was built using the Slim Framework (https://www.slimframework.com/).

The earliest version of Laravel that I used was Laravel 5.5 on PHP 7.x, given that it's now on Laravel 11 on PHP 8.2, the code has progressed through many changes to the core language.

During this time, I've tried to keep the code updated. Implementing strict type checking on function parameters and return values. Keeping up with modern standards, but there will always bee a few bits I've missed. I thought this would be an ideal time to try Rector and see how it performs.

First, I checked out the latest version of the codebase on my dev machine and created a new branch.

I've tried to keep my test suite up-to-date which should give me some confidence that any changes made by Rector have not broken anything. We'll see how this confidence holds....

Test current code

$ ./artisan test --compact
.............................................

  Tests:    46 passed (140 assertions)
  Duration: 1.50s

$

Before any changes, everything is passing.

Install Rector

$ composer require rector/rector --dev
./composer.json has been updated
Running composer update rector/rector
...
...
...
Using version ^2.0 for rector/rector
$

Configure Rector

Rector is configured with a file called rector.php. If you don't have this, running Rector for the first time will detect this and offer to put in a basic config.

$ app vendor/bin/rector 

 No "rector.php" config found. Should we generate it for you? [yes]:
 > yes

 [OK] The config is added now. Re-run command to make Rector do the work!

Taking a look at the file, it looks to have scanned for the common directories in my repo.

<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;

return RectorConfig::configure()
    ->withPaths([
        __DIR__ . '/app',
        __DIR__ . '/bootstrap',
        __DIR__ . '/config',
        __DIR__ . '/lang',
        __DIR__ . '/public',
        __DIR__ . '/resources',
        __DIR__ . '/routes',
        __DIR__ . '/tests',
    ])
    // uncomment to reach your current PHP version
    // ->withPhpSets()
    ->withTypeCoverageLevel(0)
    ->withDeadCodeLevel(0)
    ->withCodeQualityLevel(0);

I'm going to keep it at the defaults for the time being, the only change I'll make is to uncomment ->withPhpSets(), this will cause Rector to check the PHP version in my composer.json file and test against that (Currently PHP 8.2).

Running rector again outputs it's suggested changes and has updated the PHP files.

$ vendor/bin/rector 
 136/136 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
12 files with changes
=====================

...
... (Remove for brevity)
...

 [OK] 12 files have been changed by Rector

$

Right off the bat, let's see if anything's broken...

$ ./artisan test --compact

.............................................

  Tests:    46 passed (140 assertions)
  Duration: 1.99s

$

So far, so good. Let's look at some of the changes it's suggested.

1. Removing extra parameters.

Firstly the RemoveExtraParametersRector option has detected that the optional parameters for PHPUnit's assertStatus() can be removed.

-            $response->assertStatus(200, "Assert $uri returns a 200 response");
+            $response->assertStatus(200);

It looks like I added this second parameter to add some debug when I refactored the tests after upgrading to PHPUnit 10.x and hit some problems. It looks like I got confused with Perl's test suite which allows extra parameters. Checking the docs, the assertStatus() function does only accept a single parameter. +1 for Rector

2. Processing files within bootstrap/cache.

As the path suggests, the files bootstrap/cache/services.php and bootstrap/cache/packages.php are cache files and will be deleted and recreated regularly by Laravel. There is no need to fix these. This is user error and can be resolved by adding the following to my rector.php config file.

    ->withSkip([
        __DIR__ . '/bootstrap/cache',
    ])

3. Remove unused variable in try/catch.

This is a good catch and can be removed. I probably left it there during debug and didn't remove it. +2 for Rector.

         try {
             $record = $reader->city($ipAddress);
-        } catch (AddressNotFoundException $e) {
+        } catch (AddressNotFoundException) {
             //
         }

4. Return values for closures.

The AddClosureVoidReturnTypeWhereNoReturnRector option found a few instances in the boilerplate Laravel code within routes/console.php where no return value was specified on closures. +3 for Rector.

-Artisan::command('inspire', function () {
+Artisan::command('inspire', function (): void {
     $this->comment(Inspiring::quote());
 })->purpose('Display an inspiring quote');

5. Updating closure definition format.

The ClosureToArrowFunctionRector wishes me to convert anonymous functions to the newer format arrow => functions.

-Broadcast::channel('App.Models.User.{id}', function ($user, $id) {
-    return (int) $user->id === (int) $id;
-});
+Broadcast::channel('App.Models.User.{id}', fn($user, $id) => (int) $user->id === (int) $id);

I actually prefer the old style anonymous function. Retaining the function () { ... } format helps my brain easily parse what the function is up to. The arrow format feels like a run-on sentence in my mind.

Although not an issue in this instance, arrow functions include variables from the parent scope and don't require the use keyword to pass variables e.g. function() use ($i) { ... }. Requiring use to bring variables into scope is a good thing in my mind, so I think I'll keep these functions as they are.

To stop this I will just add the following to my rector.php config file.

    ->withSkip([
        \Rector\Php74\Rector\Closure\ClosureToArrowFunctionRector::class,
    ]);

Note: The documentation shows that you can use the non-qualified class name ClosureToArrowFunctionRector::class, however this give an error that the Rector rule does not exist, it may work for some rules, but for this one and possible others, you must give the fully qualified class name to the withSkip() function.

6. Constructor property promotion.

I like this. The ClassPropertyAssignToConstructorPromotionRector realises that as I'm now running PHP 8, the old style boiler plate class code can be dispensed with and the cleaner constructor property promotion format can be used. This was the only class in my codebase that was affected, but on a large codebase with some big classes this could really slim down the code. +4 for Rector.

@@ @@

 class Repository
 {
-    /** @var string $folderPath */
-    private $folderPath;
-
-    /** @var string $gitBinary */
-    private $gitBinary;
-
     private const GIT_BINARY = 'git';

     private const DEFAULT_TRUNCATED_COMMIT_ID_LENGTH = 10;
@@ @@
      *
      * @param string $folderPath
      */
-    public function __construct(string $folderPath = '.', string $gitBinary = self::GIT_BINARY)
+    public function __construct(private string $folderPath = '.', private string $gitBinary = self::GIT_BINARY)
     {
-        $this->folderPath = $folderPath;
-        $this->gitBinary = $gitBinary;
     }

     /**

Review

Overall, I'm very impressed with Rector. It did nothing wrong, it broke no code and the only changes I had to make were due to my tastes rather than to fix a problem. If anything I wish my codebase was in a worse condition so that I could get more benefit from it.

It appears there's even a PHPStorm/Jetbrains plugin to automatically run Rector on your code base https://plugins.jetbrains.com/plugin/19718-rector-support.

If I took on an old codebase as part of a new role, I would definitely use Rector to incrementally modernise the code.

If you found this useful, please feel free to donate via bitcoin to 1NT2ErDzLDBPB8CDLk6j1qUdT6FmxkMmNz