PHPStan Drupal 0.12.7 was released this morning, with only one fix. But it's an important one. This post acts as a retrospective for diving into an obscure issue that took a few months to determine. It turns out that the Rector project had been facing this bug for weeks, as well.
It all started in October 2020 when PHPStan Drupal was crashing when trying to autoload \Drupal\Tests\PhpunitCompatibilityTrait
. I couldn't figure out why. I spent a lot of time debugging it and stepping through versions of PHPStan to find when it once worked and when it broke: https://github.com/mglaman/phpstan-drupal/pull/145. The file was discovered, autoloaded, but wouldn't be recognized by PHPStan.
So, what's the deal with this code, right? How can one trait bring everything down? Well, it does some tricky things. It's a trait that provides compatibility between PHPUnit 6 and PHPUnit 7 dynamically by leveraging a class alias.
// In order to manage different method signatures between PHPUnit versions, we
// dynamically load a compatibility trait dependent on the PHPUnit runner
// version.
if (!trait_exists(PhpunitVersionDependentTestCompatibilityTrait::class, FALSE)) {
class_alias("Drupal\TestTools\PhpUnitCompatibility\PhpUnit" . RunnerVersion::getMajor() . "\TestCompatibilityTrait", PhpunitVersionDependentTestCompatibilityTrait::class);
}
/**
* Makes Drupal's test API forward compatible with multiple versions of PHPUnit.
*/
trait PhpunitCompatibilityTrait {
use PhpunitVersionDependentTestCompatibilityTrait;
The code is legit and works. But it broke during static analysis. It didn't matter if the file was included manually or not. If I removed the condition and only had a hardcoded class alias, things were fine. But this condition caused everything to fall apart.
Eventually, I tracked down a working version of PHPStan – 0.12.42
. So what happened in 0.12.43
? The library nikic/php-parser
was updated to 4.10.0
. Back then, when I looked at the released changes, I had no idea why. I added a Composer version conflict to prevent the crash, and the DrupalCI PHPStan job would stop dying.
I tried debugging it a few times here and there. But I could not figure it out. And since it was such a huge amount of time and effort required, it was not on the top of my priorities since it's free time development.
Earlier this month, December 2020, I decided to tackle this beast finally. I did so via my live stream. I thought I found the bug in PHPStan. In the autoload source locator, there is a comparison of the start line. For PhpunitCompatibilityTrait
this was mismatched and failing. Eureka!
All was fixed by changing a line of code from checking the class node's start line to its identifier's start line.
// Before
if ($startLine !== $classNode->getNode()->getStartLine()) {
continue;
}
// After
if ($startLine !== $classNode->getNode()->name->getStartLine()) {
continue;
}
Unfortunately, this uncovered a deeper and harder problem. When testing PHP's built-in reflection, there were no problems (see https://3v4l.org/IfhZY and https://3v4l.org/5HOOW.) That meant it wasn't a PHP bug. It meant it was a bug in the parser. In nikic/php-parser
specifically. When analyzing a trait node, the start line was being determined by the if
statement and not the trait
identifier. If you look in the screenshot below, both statements have the same start line of 10, when the trait is defined on line 17.
So, I opened an issue: https://github.com/nikic/PHP-Parser/issues/738. Thankfully Nikita Popov replied that it was probably related to parsing of traits and optional attributes (for PHP 8 support.)
I managed to create a failing test and opened a PR to prove that something was wrong: https://github.com/nikic/PHP-Parser/pull/739. I also used this test to run and debug the failing test locally to try and solve this. I was able to determine that the attribute stack position was off by one. Changing it manually fixed the problem. However, the parser is generated from a grammar file, so the fix was elsewhere.
A few nights ago, I decided to grab some whiskey and sit down with this. I started messing around with the php7.y
grammar file. I got nowhere. I couldn't figure out the right changes that needed to be made. I then made the mistake of trying to dive into ircmaxell/php-yacc
and understand how the grammar file was compiled.
Luckily, to my rescue, Nikita created a fix – https://github.com/nikic/PHP-Parser/commit/d3d1ee470a67998b14b5c04b0713…. I definitely would not have gotten this on my own.
If you use the Upgrade Status module, upgrade to 8.x-3.1 🙂
I'll be working on a new release of Drupal Check this week, as well.
Support the development of PHPStan Drupal
Want to support the development of PHPStan Drupal? I hope this blog post has helped explain the intricacies that go into maintaining this project. The project must stay up to date with changes from PHPStan and Drupal. It also must support Drupal 8 and Drupal 9 (and beyond.)
You can fund development by becoming a sponsor on GitHub Sponsors: https://github.com/sponsors/mglaman
Want more? Sign up for my weekly newsletter