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?