How we're using static analysis to improve our codebase

The initial Flare codebase was written almost two years ago. Though that's not so long ago, but PHP landscape changed drastically in that timeframe. PHP version 7.4 and 8.0 were significant releases, and static analyzers became a popular topic in the PHP world.

A static analyzer helps you to find bugs in your code without even running it. Popular static analyzers for PHP are Psalm and PHPStan. In this post, we're going to look at what such a static analyzer can find in a two-year-old codebase.

Since Flare is a Laravel project, we decided to use Larastan, a Laravel flavoured extension of PHPStan.

Installing Larastan

Larastan is installed just like any other PHP dependency with Composer:

composer require --dev nunomaduro/larastan

When Larastan is installed you create a phpstan.neon file in which you can configure PHPStan:

`includes:

• ./vendor/nunomaduro/larastan/extension.neonparameters:
  paths:
• applevel: 1ignoreErrors:
• '#Unsafe usage of new static#'checkMissingIterableValueType: false
  `

PHPStan has eight levels: one is the lowest level of error checking, and eight is the highest, most strict one. We're planning to raise the level in the coming months, so we know for sure that the Flare codebase is wholly checked.

Now you can run PHPStan like this:

./vendor/bin/phpstan analyse

PHPStan will output a list of errors within your codebase like this one:

 ------ ----------------------------------------------------- 
  Line   Domain/Subscription/Listeners/CreateInvoice.php      
 ------ ----------------------------------------------------- 
  17     Variable $invoice in PHPDoc tag @var does not match  
         assigned variable $cashierInvoice.                   
  30     Access to an undefined property                      
         Laravel\Cashier\Invoice::$id.                        
 ------ ----------------------------------------------------- 

 ------ ------------------------------------------------------ 
  Line   Domain/Subscription/Models/UsagePeriodDay.php         
 ------ ------------------------------------------------------ 
  19     Call to an undefined method                           
         App\Domain\Team\Models\Team::getApiUsageResetDate().  
 ------ ------------------------------------------------------ 

 ------ ------------------------------------------------------ 
  Line   Domain/Subscription/Notifications/ConfirmPayment.php  
 ------ ------------------------------------------------------ 
  37     Access to an undefined property                       
         Laravel\Cashier\Payment::$id.                         
 ------ ------------------------------------------------------ 

A look at some errors PHPStan found

     protected function couponEligibleForBillable(
         string $coupon,
---      Team | Billable $billable,
+++      Team $billable,
         string $plan,
         array $options
     ): bool {

This piece of code checks if a coupon can be used for a Team, Team|Billable is a valid union type since we're using PHP 8. But Billable is a trait that cannot be type hinted as a method's parameter.

     public function getTotalWithoutTax(): float
     {
+++      if ($this instanceof Refund) {
+++          return $this->total - $this->tax;
+++      }
+++
         return ($this->amount_due) - $this->tax;
     }

We have an Invoice and Refund models, both using a trait with this code to get the total amount without tax. On the Invoice model, we use the amount_due property for this calculation. The Refund model has no such property, so we should use the total property.

--- public function didRecentlyUnsubscribe(): bool
--- {
---     if ($this->subscribed()) {
---         return false;
---     }
---  
---     if ($this->hasEverSubscribedTo()) {
---         return carbon($this->subscription()->ends_at)->isBetween(now(), now()->subDays(2));
---     }
---  
---     return carbon($this->trial_ends_at)->isBetween(now(), now()->subDays(2));
--- }

Recently we've updated the Laravel Spark version within Flare. The method above was being used with the previous version of Spark. The new version of Spark doesn't require it anymore, which means it could be deleted entirely.

---      return $this->last_logged_in->addMonths(6)->isPast();
+++      return $this->last_logged_in_at->addMonths(6)->isPast();

This code in the User model checks if the user had logged in within the past six months. If not, it will mark the user to be deleted. The name last_logged_in was changed a few weeks ago to last_logged_in_at to follow the naming convention of other properties with a DateTime. We oversaw this one in our code. Luckily PHPStan found it!

---  public function invitations()
+++  public function invitations(): HasMany
     {
         return $this->hasMany(Invitation::class);
     }

Some of the relations within our Teams model were missing some typing. Luckily this was a real quick fix.

Closing thoughts

The errors above were only a tiny subset of fixes we did on the Flare codebase using PHPStan. In the coming months, we will pump up the PHPStan checking level and make our codebase as bug-free as possible.

You can find more information about Larastan and PHPStan in its GitHub repository. It's worth running it on your codebase, and you'll probably find bugs you looked over. Flare can catch bugs that PHPStan cannot find, so you can fix them later, isn't that nice?