Tom Newby

Using Psalm to find dead Symfony code

16 Dec 2018

We use vimeo/psalm, an exceptional static analyzer for PHP. It’s amazing and I think every PHP project should be using it. It also has an extra option --find-dead-code that has Psalm report issues such as:

  • UnusedClass
  • UnusedParam
  • UnusedProperty
  • UnusedVariable

As well as “possibly unused” issues:

  • PossiblyUnusedMethod
  • PossiblyUsedParam
  • PossiblyUnusedProperty

You can read more about the specifics of each issue in the Psalm docs but the short explanation is:

  • ‘Unused’ issues refer to privates which are not referenced in your code, whereas
  • ‘PossiblyUnused’ issues refer to publics/protecteds which are not referenced in your code

The significance here is that Psalm is unclear if the code in question is being installed as a library elsewhere - you might not reference a method, but end users might.

Psalm works exceptionally well at tracking these usages but the nature of Symfony’s autowiring and autoconfiguring throws a few spanners in the works.

Luckily, Psalm is quite configurable, but it required a bit of tweaking to produce an accurate config. Below, I outline some of the common issues that were being incorrectly reported by Psalm and the configuration changes I made.

Examples of Symfony specific issues

Controllers & Commands

Symfony controllers are likely never referenced directly by class name in other PHP code. References might exist in a Yaml or XML file, but these are not read or interpreted by Psalm. For Commands, they are automatically configured by using the CommandInterface, and all of the execute() calls are dynamic.

As a result, Psalm reports PossiblyUnused references on:

  • the __construct of Controllers/Commands
  • controller actions
  • the execute method of the Command

As a solution, we then need to suppress all of these issues in /Controller/ and /Command/ namespaces. In my final configuration, I opted to suppress all UnusedClass issues and specifically set only some particular directories to report as errors.

Services

Psalm will erroneously report PossiblyUnusedMethod on the __construct() of each service, as the constructor is automatically handled by the Container.

The solution here is to suppress all PossiblyUnusedMethod issues on __construct() calls to */Service/* classes. Due to how the whitelisting/blacklisting is structured in Psalm, I ended up suppressing all of these, not just limiting to the */Service/* namespaces.

Forms & Serialization

When working with entities and models, you might be taking advantage of the Form and Serialization components in Symfony which both heavily use the PropertyAccess component.

The problem for Psalm is that your getters and setters for these entities and models will be called dynamically at runtime and as such means there is no traceable calls to these methods, resulting in PossiblyUnusedMethod issues reported.

My solution was to ignore PossiblyUnusedMethod on the Entity and Model directories.

Event subscribers

Event subscribers are another example of dynamically executed code:

<?php

...
class TerminateSubscriber implements EventSubscriberInterface
{
    public static function getSubscribedEvents()
    {
        return [
            KernelEvents::TERMINATE => 'onKernelTerminate',
        ];
    }
    
    public function onKernelTerminate(PostResponseEvent $event): void
    {
    	// ...
    }
}    

In this example, the onKernelTerminate method is referenced as a string in getSubscribedEvents(), as we don’t have a function references in the language (though there is an RFC!)

As a result, Psalm cannot find any references that execute onKernelTerminate, nor the getSubscribedEvents, so they are marked as PossiblyUnusedMethod. The solution again is to suppress these issues in these namespaces.

Twig extensions

We have a few Twig extensions similar to below. This code, much like the event subscribers, is dynamically executed and Psalm cannot properly track any usages. In this example, getClass is only referenced as a callable in the constructor of the Twig_Function.

<?php

declare(strict_types=1);

namespace Acme\App\Twig\Extension;

final class GetClassExtension extends \Twig_Extension
{
    public function getFunctions()
    {
        return [
            new \Twig_Function('getClass', [$this, 'getClass']),
        ];
    }

    public function getClass(object $object): string
    {
        return \get_class($object);
    }
}

Just like in the earlier cases, Psalm reports PossiblyUnusedMethod on these Methods, so we need to suppress these issue types for these directories.

Final configuration

This is a simplified version of our configuration we run. We have some other config not included here to make Psalm a bit more lax in our tests/ directory. It also removes a few domain specific rules we have setup.

<!--psalm.xml-->

<?xml version="1.0"?>
<psalm
    totallyTyped="true"
    strictBinaryOperands="true"
    allowPhpStormGenerics="true"
    allowStringToStandInForClass="false"
    autoloader="bin/.phpunit/phpunit-6.5/vendor/autoload.php"
>
    <issueHandlers>
        <!-- Start Dead Code Config -->
        <UnusedMethod>
            <errorLevel type="suppress">
                <directory name="src/*/Controller" />
                <directory name="src/*/Command" />
                <directory name="src/*/EventListener" />
                <referencedMethod name="*::__construct"/>
            </errorLevel>
        </UnusedMethod>

        <UnusedClass errorLevel="suppress">
            <errorLevel type="error">
                <directory name="src/*/Entity" />
                <directory name="src/*/Model" />
                <directory name="src/*/Exception" />
            </errorLevel>
        </UnusedClass>
        <PossiblyUnusedMethod>
            <errorLevel type="suppress">
                <directory name="src/*/Entity" />
                <directory name="src/*/Model" />
                <directory name="src/*/Controller" />
                <directory name="src/*/Command" />
                <directory name="src/*/EventListener" />
                <directory name="src/*/Twig" />
                <!-- Tests have lots of unused publics - all the test methods-->
                <directory name="tests/" />
                <referencedMethod name="*::__construct"/>
            </errorLevel>
        </PossiblyUnusedMethod>
        <!-- End Dead Code Config-->
    </issueHandlers>
</psalm>

Final remarks

In the end, this took me about a day of tweaking, reporting some false positives to Psalm (which were swiftly addressed!) and deleting our resultant unused code. For us, a huge source of unused code was in unused exceptions. As we refactored code, we had lots of custom exceptions which were no longer getting called. Now Psalm will catch this in future cases.

A nice side effect, I noticed that by doing this work, we standarised our directory structures more. As our app is somewhat large, we use subnamespaces to separate up some different concerns in the app, i.e. “Users” and “Storage” and “Email”. Each of these subnamespaces then have their own directories for entities, event subscribers, controllers etc.

Through creating these rules in psaml.xml, I discovered that we had event subscribers stored in /Event, /EventSubscriber and /EventListener through each of the different subnamespaces. There was a few other cases of Models not being stored in the /Model directories etc. as well.

If you implement dead code detection using Psalm, I’d love to hear how it went for you!

Follow me on Twitter: @tomnewbyau