Scanning WordPress with PHPStan

 Scans with PHPStan need a highly customized ruleset to get remotely usable results and even then, they are riddles with false positives, making the output neigh unusable.

Note: this is not necessarily criticism of the PHPStan tooling, but is largely due to the fact that WordPress barely uses type declarations, while PHPStan is primarily geared towards projects using modern code.

An initial scan with the most basic of configurations, yields well over 20.000 issues.

A scan with the above mentioned, highly customized ruleset, aimed at PHP 8 related issues specifically, still yields 580 issues at level 5, another 2.150 potential issues at level 7, though these will likely contain a lot of false positives and yet 380 more issues at level 8 with the same caveat.

A Trac ticket was opened a while back to address a list of issues based on an unknown configuration, but specifically aimed at passed parameter type mismatches (level 5). There is a draft PR available to fix these issues.

An initial assessment of this PR, however, shows that the majority of fixes proposed would hide issues by typecasting variables to the expected type, not actually fix them by doing proper type checking. This can lead to unexpected behaviour in the application if these changes are not accompanied by strict unit tests (and they are not). This will also likely result in much harder to debug errors further down the line.

At this time, it has not been verified whether the fixes proposed are even warranted or that the issues identified should be considered false positives.

Scans run with PHPStan 0.12.52.

Testing

Due to the nature of the problematic changes in PHP 8.0, static analysis can only go so far. Manually reviewing and testing software is painstaking work and humans are very prone to overlook things when there is a lot to be looking out for.

Manually testing performed by end-users tends to be relatively useless, as this will generally only result in “happy paths” being tested. Comprehensive exploratory and regression testing is needed to achieve more reliable results. And even if problems are found, it requires extensive debugging to figure out the cause of the problem – WordPress ? Plugin Theme ? – and whether it is related to PHP compatibility.

More than anything it is important to have automated tests of good quality and to run these on PHP 8, as this will give the best indication of PHP 8.0 problems to expect.

Running automated tests on PHP 8

Getting an automated test suite to run on PHP 8 takes us down the next rabbit hole as the de facto tool for unit testing in the PHP world, PHPUnit, generally does a major release every year and with each major drops support for older PHP versions and introduces breaking changes. The first PHPUnit version which is officially compatible with PHP 8.0 is PHPUnit 9.3, released August 2020.

As WordPress still supports PHP 5.6 as a minimum, to run tests on PHP 8.0, any WordPress related test suite will have to be compatible with PHPUnit 5 up to PHPUnit 9.

While tooling is being built to help with that (look out for a blogpost about this over the next week!), it still takes time and effort to implement these tools and make a test suite compatible. Time which is taken away from the time available to actually fix PHP 8 related problems.

Getting the tests to run on PHP 8 for WP Core

The tests for WP Core are run against PHP 8 and are currently passing. The tests are being run on a composer installed version of PHPUnit 7.5, even though PHPUnit 9.3 is the earliest PHPUnit version officially compatible with PHP 8.0.

This last point has been overcome by copying a select number of files/classes from PHPUnit 9.3 to the WP test suite, excluding the PHPUnit native classes from the Composer autoload generation, in favour of using the copies from PHPUnit 9.3 in the WP test suite. While this works for now, this is a hacky solution and may not be sustainable in the future, aside from the maintenance it may currently require.

Note: while all other WordPress Core CI builds use the Phar version of PHPUnit, this is not possible when running PHPUnit 7.5 on PHP 8.0 as PHPUnit 7 is no longer maintained and the Phar contains incompatible dependencies of PHPUnit. Using a Composer based install of PHPUnit overcomes this as it will pull in the latest compatible versions of the dependencies, though –ignore-platform-reqs is needed.

As for the quality of the tests, this was relatively low to begin with, with loose type checking being used in the majority of cases.

A Trac ticket to address this was already opened in 2016. With an eye on the stricter type adherence in PHP 8.0, this ticket has been revived and a lot of work has been done to mitigate this.

At the time of writing, there are nearly 800 instances (676 assertEquals() + 96 assertNotEquals()) still using loose type checking – down from over 8000 instances.

In part the remaining loose type assertions are legitimate when objects are being compared, in part these still need to be addressed, but would currently result in test failures. These last ones highlight shortcomings either in the tests, but more often in the code being tested.