Preloading failure when running tests with phpunit

[Androoha1]

The bundled dependencies of the tool (vendor dir) are namespace prefixed to avoid conflicts. But nicik/php-parser package is not prefixed.

Example file:
vendor/nikic/php-parser/lib/PhpParser/NodeVisitor.php

In my project I am developing a custom rector rule, I run rector tests as a separate test suit, and then I get this conflict when trying to run my tests:

~\Desktop\BTA_projects\Mobile-App\rest-api\web git:[ONE-11888-clean-up-obsolete-code-parts-with-rector]
vendor/bin/phpunit --testdox --testsuite Rector
PHPUnit 12.3.0 by Sebastian Bergmann and contributors.

Runtime: PHP 8.4.7
Configuration: C:\Users\andrii.posternak\Desktop\BTA_projects\Mobile-App\rest-api\web\phpunit.xml

PHP Fatal error: Cannot redeclare interface PhpParser\NodeVisitor (previously declared in C:\Users\andrii.posternak\Desktop\BTA_projects\Mobile-App\rest-api\web\vendor\nikic\php-parser\lib\PhpParser\NodeVisitor.php:5) in C:\Users\andrii.posternak\Desktop\BTA_projects\Mobile-App\rest-api\web\vendor\rector\rector\vendor\nikic\php-parser\lib\PhpParser\NodeVisitor.php on line 6

Fatal error: Cannot redeclare interface PhpParser\NodeVisitor (previously declared in C:\Users\andrii.posternak\Desktop\BTA_projects\Mobile-App\rest-api\web\vendor\nikic\php-parser\lib\PhpParser\NodeVisitor.php:5) in C:\Users\andrii.posternak\Desktop\BTA_projects\Mobile-App\rest-api\web\vendor\rector\rector\vendor\nikic\php-parser\lib\PhpParser\NodeVisitor.php on line 6

[samsonasik]

That is expected to allow custom rule. To handle preload issue, read documentation

https://getrector.com/documentation/troubleshooting-preload

[calebdw]

Forcing every user to have to add a vendor file to the autoload-dev.files seems wrong...surely there should be a better way to handle this

[samsonasik]

@calebdw this only happen on phpunit 12+, that autoload is too early loaded, which not part of rector task to cover that, I think it probably better to report to phpunit with reproducible steps in case of it.

[Androoha1]

@samsonasik, thanks! Adding that preload file solved the conflict.

But could you please explain why the conflict occurred in the first place ? I can't understand why rector tried to load the same class, with the same FQCN, when it was already loaded by my project's autoloader ?

[samsonasik]

The projects may or may not have 2 dependencies to build custom rules:

• nikic/php-parser
• phpstan/phpdoc-parser

or it has, but different versions. Without preload, that installation will harder for user to use the latest rector as possible, while have some old dependencies.

To make custom rule work, the rector vendor preload needs to be autoload early, that's already covered by its bootstrap.

rector/bootstrap.php

Line 18 in 479e63a

$composerAutoloader = require __DIR__ . '/vendor/autoload.php';

but on PHPUnit 12+, they include "vendor/autoload.php" too early, the phpunit.xml even with define "custom autoload", the root "vendor/autoload.php" is loaded too early, so the issue happen.

This is the tweak mentioned in the documentation above https://getrector.com/documentation/troubleshooting-preload

since the magic happen on PHPUnit CLI command that not part of Rector job.

[samsonasik]

For additional note, we patch nikic/php-parser node so it must be preloaded https://github.com/rectorphp/vendor-patches/tree/main/patches

[calebdw]

Since you are more familiar with the issue would you be willing to open an issue at the phpunit repo?

[samsonasik]

I was trying to do that, but can't get the exact part which phpunit lines cause the magic yet. Sebastian is strict about issue reproducing, so until we can reproduce which phpunit line cause the issue, we can use this troubleshooting hack :)

[Androoha1]

@samsonasik I understood that the tweak helps to ensure that the rector's bundled implementation is loaded, and not some other. But I can't understand why that conflict occurs ? Can you explain this ? I can't understand why some autoloader tries to load that interface when it's already loaded in the memory ? Autoloaders should only load something if its not loaded already :) But since the interface was already loaded why rector tried to load it second time ?

[samsonasik]

afaik, It is not rector that try to load double time, but phpunit, the errors is in there :), it like create stubs and use autoload:

require_once __DIR__ . '/../Stub/SomeClass.php';
require __DIR__ . '/../../vendor/autoload.php';

which must be works to override SomeClass.php under vendor, but on phpunit case, the autoload is too early loaded.

[Androoha1]

@samsonasik But rector goes second, right ? It's rector who tries to load the interface, which is already loaded in memory. I may be mistaken, but it seems to be clear from the error message that I sent. The error says that the class from the rector's vendor directly was tried to be loaded. Do you mean that the rector tool somewhere directly "requires" the interface ? Like 'require .../NodeVisitor.php' ?

[samsonasik]

The preload does that, and must be loaded early before project autoload, and rector bootstrap cover it.

On phpunit < 12, it works, the issue only happen on run phpunit 12+ :)

I can only found the solution with the tweak per documentation, for now :)

[Androoha1]

@samsonasik Thank you for explaining, got it.

I spent some time and found the problematic place in the phpunit project:
https://github.com/sebastianbergmann/phpunit/blob/afdb826537513d0485c73207a28fbed41c8a8675/src/TextUI/Application.php#L112

This preload method was added since v12.2.0.

It preloads all first-party dependencies of the framework.

And then the smoking gun is the phpunit/php-code-coverage package. I has some node-visitors that extent the PhpParser\NodeVisitor, which is loaded from the project's package at that point of execution, not from the rector bundled dependencies.

And then your preloading script tries to load the same interface second time and hence the issue.

I don't know, to be honest, whose fault this is more. Since you use the phpunit framework you have to respect its mechanisms and adopt to them. But maybe they can provide some configuration for more flexibility and it can be used in the abstract rector test class.

I think you have more expertise here to make a decision. And now you also have the needed info to communicate this with phpunit.

Will you write an issue or do something about this ?