Writing a rule for PHPStan to disallow logic in controllers

In this article we will use PHPStan to find all controllers in our code where some business logic is stored. This way we will force developers to use services or actions. To implement this rule, you must have a PHPStan already installed and configured.

To recap, PHPStan is a static code analyzer. This means that it does not run the code but only reads it. It performs a series of code checks which are called rules. For example, if the code calls some method, it checks that its call corresponds to method arguments and that these arguments correspond to certain types. If any problems are found, the program will tell about it in the final report.

You can find information how to install PHPStan on their official site, but the basic steps are:

1. Install the package via Composer: composer require --dev phpstan/phpstan.

2. Create a phpstan.neon file in the root of your project.

3. Run PHPStan: vendor/bin/phpstan.

An example of a basic configuration:

includes:
    - ./vendor/nunomaduro/larastan/extension.neon

parameters:

    paths:
        - app/
        - src/

    # Level 9 is the highest level
    level: 9
    checkGenericClassInNonGenericObjectType: false

I prefer to use the highest level of check to get everything possible out of this tool. For existing code this can be a problem, you will see so many errors that you don't want to mess with it. In this case you'd better lower the level.

In this article I won't elaborate on the description of the tool, let's go straight to writing rules.

A rule in PHPStan is a class that implements the PHPStan\Rules\Rule interface. The class must be available in the autoloader. I prefer to keep the rules in a separate folder without mixing them with the whole project. To do this, we will need to add the autoload-dev section in composer.json:

"autoload": {
    "psr-4": {
        "Utils\\PHPStan\\": "utils/PHPStan/src",
        "Utils\\PHPStan\\Tests\\": "utils/PHPStan/tests"
    },

After making changes to the composer.json file, remember to run the composer dump-autoload command.

When you're developing a new rule, it's a good idea to analyze not all of the code, but just the individual files with an example of where the error should work. You can do this with the vendor/bin/phpstan analyze [file-path] command.

So, let's write our rule:

<?php
declare(strict_types=1);

namespace Utils\PHPStan;

use PhpParser\Node;
use PhpParser\Node\Stmt\Class_;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\RuleErrorBuilder;

final class NoBusinessLogicInController implements \PHPStan\Rules\Rule
{
    public function __construct(
        private readonly ControllerDetermination $determination
    )
    {
    }

    public function getNodeType(): string
    {
        return Class_::class;
    }

    public function processNode(Node $node, Scope $scope): array
    {
        $className = (string) $node->namespacedName;

        if (
            !$this->determination->isController($className)
        ) {
            return [];
        }

        $methods = $node->getMethods();

        foreach ($methods as $method) {
            $stmts = $method->getStmts();

            foreach ($stmts as $stmt) {
                if ($stmt instanceof Node\Stmt\If_
                    || $stmt instanceof Node\Stmt\While_
                    || $stmt instanceof Node\Stmt\For_
                    || $stmt instanceof Node\Stmt\Foreach_
                    || $stmt instanceof Node\Stmt\Switch_
                ) {
                    return [
                        RuleErrorBuilder::message(sprintf(
                            'Method %s in controller %s contains business logic. Move it to a dedicated service or repository.',
                            $method->name->toString(),
                            $className
                        ))->build()
                    ];
                }
            }
        }

        return [];
    }
}

As you can see, the rule must have two methods: getNodeType() and processNode(). This all works on the same principle as the event dispatcher: you register the type of the event you're interested in and then you get a notification when it happens. For your rule you register the type of node you want and when PHPStan finds this node it calls the processNode() method. But what is a node?

Nodes are elements of our code. For example, a class is a single node. Every method of a class is also a node. Every keyword or expression is also a node. Every PHP file can be defined by one big node tree.

Under the hood, PHPStan uses the PHP-Parser library to parse PHP files and build an AST tree for each file. It then traverses that tree, asking each rule whether it needs the given node for processing or not (getNodeType()). If they do, they pass it on (processNode()). In response we get an array with errors, if any.

In our example we'll analyze controllers, so we take the Class node.

Then in processNode method we need to understand, is the class passed to us a controller or not? After all, in our rule we have to work only with it and no other class.

Let's try to decide which class we can consider a controller. This is where we encounter a problem because it can vary from project to project and if we hardcode a rule, we won't be able to reuse it in other projects. There are many alternatives to search for controllers:

• Namespace must contain the word Controller.
• The file must be located in the controllers folder
• Class must have the @Route annotation
• The class is listed in the routing.php file.

To make our rule flexible, we will apply a pattern called Strategy. And let us use the controller definition rule we want, depending on the project.

First we need to define an interface, let's call it ControllerDetermination.

<?php
declare(strict_types=1);

namespace Utils\PHPStan;

interface ControllerDetermination
{
    public function isController(string $classReflection): bool;
}

All it has to do is have an isController method. We pass it the name of the class as arguments.

Then we attach our interface to the constructor of our rule (Dependency Injection).

Then in the main method processNode we check if our class is a controller. If not, we use early return.

Let's write our implementation of the interface and call it DeterminationBasedOnSuffix

<?php
declare(strict_types=1);

namespace Utils\PHPStan;

final class DeterminationBasedOnSuffix implements ControllerDetermination
{
    public function __construct(
        private readonly string $suffix = 'Controller'
    )
    {
    }

    public function isController(string $classReflection): bool
    {
        return str_ends_with($classReflection, $this->suffix) || strpos($classReflection, $this->suffix . 's');
    }
}

Here we check if the class name ends with the word Controller.

To register our rule, we need to write it in the file phpstan.neon:

  services:

    - class: Utils\PHPStan\NoBusinessLogicInController

      tags:

            - phpstan.rules.rule

    - class: Utils\PHPStan\DeterminationBasedOnSuffix

      arguments:

            suffix: "Controller"

If you like TDD development, PHPStan provides some very nice features. Writing tests here is very easy and enjoyable.

PHPStan rules are tested with PHPUnit. PHPStan comes with a base class called RuleTestCase, which extends the base TestCase class. The test for your rule should be called [RuleClass]Test. Normally, all tests are put in a separate folder and namespace.

Each test will also have a separate folder called Fixtures that contains the sample code you want to analyze. The base RuleTestCase class contains one abstract getRule() method that you must implement. This method should return the instance we need to test.

<?php
declare(strict_types=1);

namespace Utils\PHPStan\Tests\NoBusinessLogicInController;

use PHPStan\Rules\Rule;
use Utils\PHPStan\DeterminationBasedOnSuffix;
use Utils\PHPStan\NoBusinessLogicInController;

final class NoBusinessLogicInControllerTest extends \PHPStan\Testing\RuleTestCase
{
    public function testSkipControllerWithNothingInIt(): void
    {
        $this->analyse(
            [
                __DIR__ . '/Fixtures/skip-controller-if-everything-ok.php',
            ],
            [
                // we expect no errors
            ]
        );
    }

    public function testSkipLogicInNotController(): void
    {
        $this->analyse(
            [
                __DIR__ . '/Fixtures/skip-if-something-in-base-class.php',
            ],
            [
                // we expect no errors
            ]
        );
    }

    public function testFoundErrorInForLoop(): void
    {
        $this->analyse(
            [
                __DIR__ . '/Fixtures/method-has-for-loop.php',
            ],
            [
                ['Method index in controller ErrorsController contains business logic. Move it to a dedicated service or repository.', 4]
            ],
        );
    }

    public function testFoundErrorInIfStatement(): void
    {
        $this->analyse(
            [
                __DIR__ . '/Fixtures/method-has-if-statement.php',
            ],
            [
                ['Method index in controller ErrorsWithIfController contains business logic. Move it to a dedicated service or repository.', 4]
            ],
        );
    }

    /**
     * @inheritDoc
     */
    protected function getRule(): Rule
    {
        return new NoBusinessLogicInController(new DeterminationBasedOnSuffix());
    }
}

As you can see from the example above, all the tests do is accept the fixture and compare the response - whether an error is returned or not. If so, which one. To save space, I'm not going to list all of the fixtures, I'll give you the main one:

<?php
declare(strict_types=1);

class ErrorsController
{
    public function __construct(
        protected \Illuminate\Cache\Repository $repository
    )
    {
    }

    public function index(): array
    {
        for ($i = 1; $i <= 10; $i++) {
            $this->repository->get('some');
        }
        return [];
    }
}

We also need to make the appropriate settings in PHPUnit so that it knows where to find the PHPStan rules.

To do this, we make an entry in the phpunit.xml file:

<testsuites>
    <testsuite name="PHPStan rule tests">
        <directory>./utils/PHPStan/tests</directory>
    </testsuite>
</testsuites>

Now you can run the PHPStan tests with

vendor/bin/phpunit --testsuite "PHPStan rule tests"

So, in this article we have written a rather simple but functional rule for phpstan. We discussed the basics of PHPStan and now we can cover our project with more advanced tests. I also recommend reading the documentation for developers (https://phpstan.org/). It won't take you long to learn more about rule development.