Refactoring #6: Improve Code Quality in Laravel using Rector

I recently discovered Rector and was completely blown away by its power and effectiveness. The promise is simple: you install and run the package, you get instant automated upgrades and refactorings.

Damn, that's bold, I thought as I dry ran it into one of my projects. While still reading their first page instructions at getrector.org, I decided to dive in and refactor a whole project overnight, to whatever extent possible.

I'm running a Laravel 8 project in a PHP 7.4 environment, and I started out with this simple configuration that handles simple code quality improvements:

return static function (RectorConfig $rectorConfig): void {
    $rectorConfig->paths([
        __DIR__ . '/app'
    ]);

    $rectorConfig->importNames();

    $rectorConfig->sets([
        LevelSetList::UP_TO_PHP_74,
        SetList::CODE_QUALITY,
    ]);
};

Don't worry if you don't understand the code above, I didn't either half an hour ago 😂.

After running vendor/bin/rector --dry-run, I immediately saw these little upgrades that I was never going to do myself anyway.

Auto import class names

     /**
-     *
-     * @return \Illuminate\Broadcasting\Channel|array
-     */

+     *
+     * @return Channel|array
+     */
     public function broadcastOn()

This change is coming from the $rectorConfig->importNames(); configuration, and it's a lifesaver. Most coding standards prefer short class names, so auto-importing the classes in the whole project is a big big win.

Changing the string class name to a class constant

    public function user() {
-      return $this->belongsTo('App\User');

+      return $this->belongsTo(User::class);
     }

This is fantastic for old Laravel projects since most of them still use the string version to this day. It's a great upgrade because IDEs have much better support for the User::class version.

Arrow functions from PHP 7.4

       $schedule->command('dummy-command')
         ->daily()

-        ->when(function () {
-          return \Carbon\Carbon::now()->endOfWeek()->isToday();
-        });

+        ->when(fn() => Carbon::now()->endOfWeek()->isToday());

I didn't even know that arrow functions were introduced in PHP 7.4, and I could have used them without any language upgrade. Now I get to utilize their power with a single rector command.

Inline useless variables

-        $phone = str_replace(' ', '', $phone);
-        return $phone;

+        return str_replace(' ', '', $phone);

This is another low-hanging fruit. The $phone variable here adds nothing to the quality of the code, so it can be safely removed.

Combine the assignment operator

-        $this->order_count = $this->order_count + 1;

+        $this->order_count += 1;

This is an easy one as well. The short version is always better in my opinion.

Simplify the if return statements

-        if ($user->company_id == $this->id) {
-            return true;
-        }
-
-        return false;


+        return $user->company_id == $this->id;

I think you almost always want to do this, it's a beautiful one-liner that's easy to the eye and helps you grasp the logic quicker.
If, however, it ruins your formatting in one of your files, where the existing way makes more sense, you can ignore this rule by adding a comment like this:

/** @noRector \Rector\CodeQuality\Rector\If_\SimplifyIfReturnBoolRector */.

Throw error on JSON operations

     public function setImageUrlsAttribute($value)
     {
-        $this->attributes['image_urls'] = json_encode($value);

+        $this->attributes['image_urls'] = json_encode($value, JSON_THROW_ON_ERROR);
     }

This extra parameter makes it so that when encoding or decoding JSON goes wrong, it won't return null but it will throw an exception instead. I like this one, but I'm not entirely sure I'm ready to have this optimization in my code yet. Since this throws an exception, I'm a bit weary and will store this for later.

We can ignore certain rules by using this configuration.

Convert compact usages into arrays

         $user= auth()->user();
         $credits= $user->credits()->latest()->get();

-        return view('user.credits', compact('user', 'credits'));


+       return view('user.credits', ['user' => $user, 'credits' => $credits]);

Today I learned that there's an opinion about using the compact() function that states it's not ideal, and maybe an antipattern. Not sure how I feel about this, but I'm keeping this refactoring and going with the flow. Nothing wrong with plain old arrays, right?

And many other small upgrades actually. Not to forget that this is coming from just the CODE_QUALITY rules that I imported in a single line above. There are 30+ other categories that I can't wait to use and see what they offer.

At this point I just run vendor/bin/rector, all tests are still passing, commit, push, and deploy. All is well in production now 😁.