Add a custom code sniff with tests to your Laravel project
Published: 20.11.23 Last Updated: 28.11.23
When reading Designing Data-Intensive Applications: The Big Ideas Behind Reliable, Scalable, and Maintainable Systems over summer I came across the following quote which resonated with me.
Humans design and build software systems, and the operators who keep the systems running are also human. Even when they have the best intentions, humans are known to be unreliable. For example, one study of large internet services found that configuration errors by operators were the leading cause of outages, whereas hardware faults (servers or network) played a role in only 10–25% of outages.
Using the above as a bit of inspiration let's look at setting up code sniffs for a Laravel app to mitigate potential developer errors occurring in our code. In this post we will take a look at an example test and how we could be shipping code with passing tests but failing in production due to a simple developer error. After identifying the issue we will add a custom code sniff using a PHP code sniffer to reduce the chances of this type of developer error re-occurring. Finally, we will add tests for the new custom code sniff to our project so edge cases or false positives can be fixed in the future.
The Problematic Test
Let's pretend we have a simple action that makes HTTP requests to a newsletter platform. If the configuration details become invalid we want to store the subscriber's details in a database table until the user is notified about the issue and can re-authorize their newsletter account.
/** @test */
public function invalid_newsletter_configuration_throws_exception_and_store_details_in_database(): void
{
Http::preventStrayRequests();
$user = User::factory()->withoutNewsletterCredentials()->create();
$this->expectException(InvalidNewsletterCredentialsException::class);
StoreNewsletterSubscriberAction::run($user, 'John Doe', 'jd@gmail.com');
$this->assertSame(true, false);
Notification::assertSentTo([$user], InvalidNewsletterCredentialsFound::class);
$this->assertEquals(1, FailedSubscription::count());
tap(FailedSubscription::first(), function($failedSubscription) use ($user) {
$this->assertSame($user->id, $failedSubscription->user_id);
$this->assertSame('John Doe', $failedSubscription->subscriber_name);
$this->assertSame('jd@gmail.com', $failedSubscription->subscriber_email);
});
}
The test might look straightforward, except for the $this->assertSame(true, false);. This has been added here to demonstrate an unintended consequence of using $this->expectException. It silently catches the exception, asserts true, and halts further execution of the test. This means the code we were confident about regarding notifying the user and saving FailedSubscriptions is never actually being tested in the above code.
The Better Test
Let's take a look at a better solution for the above test case using Laravel's $this->assertThrows method.
/** @test */
public function invalid_newsletter_configuration_throws_exception_and_store_details_in_database(): void
{
Http::preventStrayRequests();
$user = User::factory()->withoutNewsletterCredentials()->create();
$this->assertThrows(
fn () => StoreNewsletterSubscriberAction::run($user, 'John Doe', 'jd@gmail.com'),
InvalidNewsletterCredentialsException::class
);
$this->assertSame(true, false);
Notification::assertSentTo([$user], InvalidNewsletterCredentialsFound::class);
$this->assertEquals(1, FailedSubscription::count());
tap(FailedSubscription::first(), function($failedSubscription) use ($user) {
$this->assertSame($user->id, $failedSubscription->user_id);
$this->assertSame('John Doe', $failedSubscription->subscriber_name);
$this->assertSame('jd@gmail.com', $failedSubscription->subscriber_email);
});
}
Our test would now correctly fail on $this->assertSame(true, false); which means we can be confident all assertions in the test are correctly running.
Setting up PHP Code Sniffer for our Custom Sniff
Simple developer errors like this can create outsized effects on end users. Let's say a big merchant with an invalid newsletter configuration is offering 10% off a shopping cart for newsletter subscriptions over Black Friday. They could in fact be losing a large number of newsletter signups, and additional customers.
Additionally attempting to manually enforce rules like this on a large code base during code reviews is laborious and a waste of everyone's time. So let's look at automating the process to improve DX and prevent simple mistakes from persisting in the future.
Install PHP Code Sniffer
composer require squizlabs/php_codesniffer --dev
Add a custom code standards folder and ruleset to your repository
mkdir code-standards
touch code-standards/ruleset.xml
Paste the code below in code-standards/ruleset.xml
<?xml version="1.0"?>
<ruleset name="CodeStandards">
<description>Acme custom coding standard to mitigate bugs.</description>
</ruleset>
Create a php code sniffer config file
touch phpcs.xml
Paste the code below in phpcs.xml
You can read up with more clarity on how best to customize this file with annotations here in our case we are only sniffing php files in app and tests directories and registering the new code sniff that we will put in /code-standards/ directory.
<?xml version="1.0"?>
<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="PHP_CodeSniffer" namespace="App">
<description>The coding standard for PHP_CodeSniffer itself.</description>
<file>app</file>
<file>tests</file>
<arg name="extensions" value="php" />
<rule ref="./code-standards"/>
</ruleset>
Testing our DeprecateExpectExceptionSniff
First, let's start with the tests. We will add two tests. One to test if the offending code is caught and one to ensure our guard clauses to prevent sniffing non-test classes is also working.
The command we will be running for our custom sniff
exec('cat ' . $fixturePath . ' | vendor/bin/phpcs --no-cache --report="json"', $result, $code);
The above command will use the contents of various scenarios stored in fixtures, and pipe the contents of these to vendor/bin/phpcs. After that, we are using the --report="json" option to make assertions on the $result easier. We also use the --no-cache flag as we don't want to cache any of our test runs. $code is the exit code of the command in some cases we will be checking for 1 when we are expecting errors and 0 when we expect no errors.
Create the test file
mkdir -p code-standards/tests/Sniffs/
touch code-standards/tests/Sniffs/DeprecateExpectExceptionSniffTest.php
Add the code to the test file
<?php
namespace CodeStandards\Tests\Sniffs;
use Tests\TestCase;
class DeprecateExpectExceptionSniffTest extends TestCase
{
/** @test */
public function expect_exception_assertion_in_test_class_raises_error(): void
{
$fixturePath = './code-standards/tests/Sniffs/Fixtures/DeprecateExpectExceptionCaught.php.inc';
// Contents of $fixturePath
// <?php
// class ExampleActionTest
// {
// /** @test */
// public function exception_is_thrown(): void
// {
// $this->expectException(SomeException::class);
// ExampleAction::run();
// }
// }
$result = [];
$code = [];
exec('cat ' . $fixturePath . ' | vendor/bin/phpcs --no-cache --report="json"', $result, $code);
$this->assertSame(1, $code);
tap(json_decode($result[0]), function ($report) {
$this->assertSame(1, $report->totals->errors);
$this->assertSame(1, $report->files->STDIN->errors);
$this->assertSame(0, $report->files->STDIN->warnings);
$this->assertCount(1, $report->files->STDIN->messages);
tap($report->files->STDIN->messages[0], function ($error) {
$this->assertFalse($error->fixable);
$this->assertEquals('ERROR', $error->type);
$this->assertEquals('The use of the `expectException` method of PHPUnit should utilize Laravel\'s `assertThrows` instead.', $error->message);
$this->assertEquals('.Sniffs.DeprecateExpectException.CodeStandards\Sniffs\DeprecateExpectExceptionSniff', $error->source);
});
});
}
/** @test */
public function expect_exception_assertion_in_non_test_class_is_ignored(): void
{
$fixturePath = './code-standards/tests/Sniffs/Fixtures/DeprecateExpectExceptionSkipped.php.inc';
// Contents of $fixturePath
// <?php
// class ExampleAction
// {
// public function process(): void
// {
// try {
// Http::get('http://google.com');
// } catch(\Exception $e) {
// $this->expectException($e);
// }
// }
// public function expectException(): void
// {
// // log exception somewhere
// }
// }
$result = [];
$code = [];
exec('cat ' . $fixturePath . ' | vendor/bin/phpcs --no-cache --report="json"', $result, $code);
$this->assertSame(0, $code);
tap(json_decode($result[0]), function ($report) {
$this->assertSame(0, $report->totals->errors);
$this->assertSame(0, $report->files->STDIN->errors);
$this->assertSame(0, $report->files->STDIN->warnings);
$this->assertCount(0, $report->files->STDIN->messages);
});
}
}
If you run the following command in Sail you should get two failing tests.
sail test code-standards/tests/Sniffs/DeprecateExpectExceptionSniffTest.php
Adding our DeprecateExpectExceptionSniff
With these tests in mind let's code it up. PHP Code Sniffer relies on PHP Tokens to work so some familiarity with it is ideal for creating custom sniffs.
Create the test file
mkdir -p code-standards/Sniffs/
touch code-standards/Sniffs/DeprecateExpectExceptionSniff.php
<?php
namespace CodeStandards\Sniffs;
use Illuminate\Support\Str;
use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;
class DeprecateExpectExceptionSniff implements Sniff
{
public function register(): array
{
return [T_VARIABLE]; // checks all variable tokens
}
public function process(File $file, $position): void
{
// Find previous class token position
$classTokenPosition = $file->findPrevious([T_CLASS], $position);
if(!$classTokenPosition) {
return;
}
// Use the classTokenPosition to find the name of the class
$classNamePosition = $file->findNext([T_STRING], $classTokenPosition);
$classNameToken = $file->getTokens()[$classNamePosition];
if (! Str::endsWith($classNameToken['content'], 'Test')) {
// Class is not a test file
return;
}
// Use the original variable position to find the name of the variable
$variableNameToken = $file->getTokens()[$position];
if ($variableNameToken['content'] !== '$this') {
// The variable does not likely reference phpunit
return;
}
$methodNamePosition = $file->findNext([T_STRING], $position);
$methodNameToken = $file->getTokens()[$methodNamePosition];
if ($methodNameToken['content'] !== 'expectException') {
// The variable does not reference our deprecated method
return;
}
$file->addError('The use of the `expectException` method of PHPUnit should utilize Laravel\'s `assertThrows` instead.', $position - 2, self::class);
}
}
If you made it this far you should now have the basics for adding your own Custom Code Sniffers to your Laravel app. All you need to do now is add the command to your CI pipeline and you can ensure custom code standards across your app can be enforced to prevent human errors showing up in your tests and other parts of your code.
Follow me on Twitter to get notified of future blog posts. Next up we will take the information here and apply it to an Automatic Fixer using PHP CS Fixer.
Have questions or want to stay up to date? Find me on Twitter Get notified of new posts