Writing Custom PHPStan Rule to prohibit business logic in controllers

As the official website says - PHPStan scans your whole codebase and looks for both obvious & tricky bugs. Even in those rarely executed if statements that certainly aren't covered by tests.

To solve subject-problem we will use PHPStan static analysis tool. It has a powerful enough engine that allows us 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 PHPStan already installed and configured.

Let me remind you that PHPStan is a static code analyser. It means that it does not run code but only reads it. It performs a number of checks called rules. For example, if the code calls some method, it checks that its call matches the arguments of the method and that these arguments match in type. If any problems are found, the programme will report this in the final report.

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

• Install the package via Composer: composer require --dev phpstan/phpstan
• Create the phpstan.neon file in the root of your project
• Run PHPStan: vendor/bin/phpstan

Example of base phpstan.neon file:

parameters:
    level: 9
    paths:
        - ./src/

Pay attention to the maximum validation level i.e. level: 9 to get all you can out of this tool and to keep the maximum strict validation - it's highly recommended to use this level.

Let's move on to writing rules.

A rule in PHPStan is a class that implements the PHPStan\Rules\Rule interface. You can read about how to write custom rules on the official website page.

<?php declare(strict_types=1);

namespace App\PHPStan\Rules;

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

/**
 * @implements Rule<Class_>
 */
class ProhibitBusinessLogicInController implements Rule
{
    public function getNodeType(): string
    {
        return Stmt\Class_::class;
    }

    public function processNode(Node $node, Scope $scope): array
    {
        $className = (string) $node->namespacedName;
        if (!$this->isController($className)) {
            return [];
        }

        foreach ($node->getMethods() as $method) {
            foreach ($method->getStmts() as $statement) {
                if ($this->isStatementWithBusinessLogic($statement)) {
                    return [
                        RuleErrorBuilder::message(sprintf(
                            'Method "%s::%s" contains business logic. Do something better.',
                            $className,
                            $method->name->toString(),
                        ))
                        ->build(),
                    ];
                }
            }
        }

        return [];
    }

    private function isController(string $class): bool
    {
        return str_ends_with($class, 'Controller');
    }

    private function isStatementWithBusinessLogic($statement): bool
    {
        return in_array($statement::class, [
            Stmt\If_::class,
            Stmt\For_::class,
            Stmt\Foreach_::class,
            Stmt\While_::class,
            Stmt\Switch_::class,
        ]);
    }
}

As you may notice, the rule should have two methods: getNodeType and processNode. It works similarly to an event dispatcher: you register an event type that interests you, and then you receive notifications when the event occurs. For your rule you register a node type that you need and when PHPStan encounters this node, it calls the processNode method. But what exactly is a "Node $node" in it?

Static code analysis means the possibility of passing through each element(node) of your code. E.g. a class is a separate node, and each class method is also a node. Every keyword or expression is also a node. Each PHP file can be represented by one large tree of certain nodes.

Inside PHPStan uses the PHP-Parser library to parse PHP files and create an AST(Abstract Syntax Tree) for each file. It then traverses this tree, asking each rule if it needs to process the current node or not (getNodeType). If needed, it passes the node further for processing (processNode). In return, we receive an array of errors if any were found.

In our example we will be analysing controllers, so we take the Class_ node type.

Next in the processNode method we need to determine whether the passed class is a controller or not. After all, our rule should only work with controllers and no other class.

Let's try to define what class we can consider a controller. Here we face a problem since it can vary from one project to another, and here I'm trying to simplify it and use a simple way to define a controller as the postfix "Controller".

Next we have to get all the statement of each controller method. If statement related to list of our forbidden statements (see isStatementWithBusinessLogic), then this method contains business logic.

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

  services:
    - class: App\PHPStan\Rules\ProhibitBusinessLogicInControllerRule
      tags:
        - phpstan.rules.rule

If you like TDD development PHPStan gives you a good opportunity to write tests for your custom rules. PHPStan rules are tested using PHPUnit.

Custom rule test code:

<?php declare(strict_types=1);

namespace Tests\PHPStan\Rules;

use App\PHPStan\Rules\ProhibitBusinessLogicInControllerRule;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;

class ProhibitBusinessLogicInControllerRuleTest extends RuleTestCase
{
    public function testSkipControllerWithNothingWrong(): void
    {
        $this->analyse(
            [
                __DIR__ . '/fixtures/skip-controller-if-everything-ok.php',
            ],
            []
        );
    }

    public function testControllerContainsForLoop(): void
    {
        $this->analyse(
            [
                __DIR__ . '/fixtures/controller-with-for-loop.php',
            ],
            [
                ['Method "WithForLoopController::indexAction" contains business logic. Do something better.', 4]
            ],
        );
    }

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

As you can see from the example above all the tests accept the fixture and compare the response, whether an error is returned or not. If yes, then which one. I'll give example of fixture file fixtures/controller-with-for-loop.php.

<?php declare(strict_types=1);

class WithForLoopController
{
    public function __construct(
        private readonly Repository $repository
    ) { }

    public function indexAction(): array
    {
        $result = [];
        for ($i = 0; $i < 100; $i++) {
            $result[] = $this->repository->getIndex($i);
        }

        return $result;
    }
}

In this fixture we are testing the case where a for loop is used in the controller. Feel free to cover all "isStatementWithBusinessLogic" cases in your tests.

So, in this article we have written a fairly simple but functional rule for PHPStan. We have discussed the basics of PHPStan and now we can cover our project with more complex tests. I also recommend reading the developer documentation. It won't take you long to learn more rule development features.