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.
