Refactoring #8: What is dead may never run

It's important to take a good look at your code and clean up when you're done with it. Once in a while, we tend to forget this little detail and just move on, leaving behind trails of unreachable statements.

Here's a very blatant example of a dead piece of code.

{
    $user = auth()->user();

    return $user->posts;

    // This line will never run
    $user->posts->each->publish();
}

The last line above will never execute, and while this type of code looks like a very rare occurrence, it still shows up a lot. There is this automated tool called Rector that is fantastic at finding dead code like this and removing it, or letting you know of its existence.

Rector also unearths other dead code that is not completely obvious at first sight. This one below is very common, actually.

    /**
     * @param GeocoderInterface $geocoder
     */
    public function __construct(GeocoderInterface $geocoder)
    {...}

The PHPDoc is expressing the same information that is clearly stated in the method declaration. A dev can easily see that the method needs a GeocoderInterface object, the extra comment does not tell more than that. Same for an IDE, you will get the autocomplete you need without the comment.

PHPDocs can be liars in disguise

One more thing I don't like about these redundant docs is that they often 'lie'. Take this for example:

    /**
     * @return User
     */
    public function user(): BelongsTo
    {
        return $this->belongsTo(User::class);
    }

This keeps me up at night. This Eloquent relation has a return type of BelongsTo, but if you use it as a method $post->user() somewhere in code, the IDE will wrongly suggest you autocomplete options that belong to an Eloquent Model. It will be fun trying to find these bugs when they happen in runtime, so just remove the comment entirely.

If you find yourself doing this often because you want some more autocomplete options when accessing the actual model $post->user, consider annotating it, like this:

/**
 * @property User $user
 */
class Post extends Model

Laravel's default stubs

Another piece of dead code is the Laravel framework's default stubs. They're there so we can easily input our dependencies. What do we do if we don't have any? Nothing, we just leave it there, because we're lazy.

    /**
     * Create the event listener.
     *
     * @return void
     */
    public function __construct()
    {
    }

    /**
     * Create a new command instance.
     *
     * @return void
     */
    public function __construct()
    {
        parent::__construct();
    }

No harm in keeping them, but if you're not injecting anything, frankly, wipe these suckers from the face of the earth, they're just taking valuable space and pushing the real code down below. You can also change the default stubs by running php artisan stub:publish.

Conditions that are always true

Dead code candidate #4 is tricky. Usually, it's safe to remove the redundant check like in this simple case:

     if (! auth()->check()) {
         return 'not logged in';
     }

    // This second condition is always true
-    if (auth()->check()) {
-        return 'logged in';
-    }

    // You can just execute the code without the condition
+    return 'logged in';

However, there may be cases where a check like that is needed if there's stuff happening behind the scenes, like in this uninspired example:

    if (! $user->canPost()) {
        ...
        return false;
    }

    ...
    $this->updateUserPermissions();

    // You might want to keep this check
    // even if you've checked at the top
    if ($user->canPost()) {
        return true;
    }

A tool like Rector won't magically understand the underlying sorcery that is happening and will simply suggest removing the last condition. To silence it, simply add this comment:

/** @noRector \Rector\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector */
if ($user->canPost()) {
    return true;
}

That's it for the dead code. You killed it. I've also posted about other spells Rector can cast for you, like using updated syntax from PHP 8.1 and above, auto-importing class names in the whole codebase, etc. Check out their site, they add refactorings weekly 🏠🔥.

Refactoring #8: What is dead may never run

Comments

Rector

Refactoring #7: Upping the coding style game in PHP using Rector

More from this blog

Simplifying work with custom stubs in Laravel

Upgrading to Laravel 10, PHPUnit 10, and Pest 2

Speed up GitHub Actions by caching Composer, Rector, & Pint

Doing stricter checks in PHP

Upgrading to Eloquent accessors & mutators from Laravel 9

PHPDocs can be liars in disguise

Laravel's default stubs

Conditions that are always true

Command Palette

Comments

Rector

Refactoring #7: Upping the coding style game in PHP using Rector

More from this blog

PHPDocs can be liars in disguise

Laravel's default stubs

Conditions that are always true