Custom Phpstan rule tests broken after update to 1.20.0 about phpdoc-parser HOT 16 CLOSED

What’s your use-case? Why do you install phpstan/phpdoc-parser separately while you also depend on PHPStan?

from phpdoc-parser.

we don't install phpstan/phpdoc-parser it is only there as a transient dependency, with the latest release suddenly our CI pipelines failed.

To debug it i tried to manually require phpstan/phpdoc-parser to downgrade it to 1.19.1 and everything worked fine then, removing the separate require again (and thus updating to 1.20.0 again) and the error popped up again.

from phpdoc-parser.

Transient dependency of what?

I’ll probably need a small reproduction repo to see what’s going on and whos fault it is.

from phpdoc-parser.

i thought it was a transient dependency from phpstan itself, but turned out that the phpdocumentor/type-resolver package brings along the phpstan/phpdoc-parser.

I'll investigate deeper what's going on, could not reproduce it now.

from phpdoc-parser.

My guess is that the things get mixed up, because phpstan/phpdoc-parser is in phpstan.phar (and that's going to be 1.19.1), and if you install phpstan/phpdoc-parser separately, it's somehow possible the autoloading is gonna mix it up. It's not an easy problem to solve.

from phpdoc-parser.

That's a good insight!

I stumpled upon this line of code in the PharAutoloader of phpstan

if (strpos($class, 'PHPStan\\') !== 0 || strpos($class, 'PHPStan\\PhpDocParser\\') === 0) {
      return;
}

So there seems to be already custom handling for the case when PhpDocParser is installed directly as well.

I'm not sure if there is already an issue there, but looking at the stacktrace again it seems to me like the PhpDocParser is wrongly loaded from the vendor folder instead of from the phar, probably because that class is autoloaded before the .phar file is loaded.

And i'm also not sure on how that could be fixed, but i found that there is some kind of prefixing going on in the phpstan phar file for some other dependencies, eg. Nette or Psr stuff and so on, shouldn't the phpdoc-parser also be prefixed to prevent issues likes this?

from phpdoc-parser.

So it’s the opposite - we actually want to load it from the PHAR.

from phpdoc-parser.

I mean by prefixing the dependency in phpstan we would make sure that from inside phpstan we always use the version of the doc parser shipped with phpstan and we prevent auto load collisions when the doc parser is installed directly as well

from phpdoc-parser.

I mean by prefixing the dependency in phpstan

Can't do that, it's in PHPStan\ namespace and people rely on it in custom PHPStan extensions.

from phpdoc-parser.

Any idea for how to fix this? The only solution i see currently is conflicting the newer doc parser version in our setup, based on what version is included in the packaged phpstan phar 🤔

from phpdoc-parser.

The easiest fix is to release a new PHPStan version with the new phpdoc-parser included, which will make this problem go away. I'll do that in a matter of days.

In the long term, we should probably do something like including the PHAR preload.php script when the PHPStanTestCase is loaded which would ensure that all the phpdoc-parser classes come from the same version.

from phpdoc-parser.

Can you explain the idea with the preload.php in a little more detail?
I'd love to give it a try fixing it.

I already tried some approaches there but in the end it either did not fix the issue or make it worse (fatal errors because of trying to redeclare classes).

from phpdoc-parser.

We could put require_once __DIR__ . '/../../preload.php'; at the top of PHPStanTestCase here: https://github.com/phpstan/phpstan-src/blob/1.10.x/src/Testing/PHPStanTestCase.php

In the compiled phpstan.phar file it preloads unprefixed classes: https://github.com/phpstan/phpstan-src/blob/840a64f113c7ca10d5111719a0338914b0051ceb/compiler/src/Console/PrepareCommand.php#L157-L192

I'd rather do it in a major version because it could break someone's use case.

from phpdoc-parser.

Ok i tried to patch that into our system, but it did not work that easily because it lead to fatal errors that it cannot redeclare classes.

I debugged a little more and found out that in our case the issue was not that phpstan loads the wrong class, but the root issue is that the PhpDocParser class actually is autoloaded before phpstan itself is loaded (but only on first test execution after running composer update).

After debugging the composer class loading i found that symfony when you use the framework-bundle will load the PhpDocParser during container compilation: https://github.com/symfony/symfony/blob/6.3/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php#L1853

So after running composer update the symfony container needs to be recompiled once before the tests actually runs, which leads to the newer version of the PhpDocParser being auto loaded for that test execution run.

In that case i actually am not quite sure at how to fix it, because i'm not sure what is expected in that cases 🤔

from phpdoc-parser.

So for now we are running the test case with @runInSeparateProcess which resolves the symfony container compile issue which seems to work around the issue.

from phpdoc-parser.

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

from phpdoc-parser.