[compat] Resolve preloading of stmts interface crashing in PHPUnit 12 and in PHPStan + Rector test suite

[TomasVotruba]

This is a meta issue to focus on solving tests crashing on PHPUnit 12 and related php-parser preload issues.

At the moment, we patch php-parser in our /vendor to allow single interface to ~10 nodes that use $stmts. We can use signle type to iterate on all stmts, instead of explicitly using these 10.

I opened PR to allow this in a php-parser core, but it didn't get far: https://github.com/nikic/PHP-Parser/pull/836 So I patched vendor. Not the best solution, but with preload it worked for couple of years: https://github.com/rectorphp/vendor-patches

Now PHPUnit 12.2 introduced preload if its own vendor php-parser first: https://github.com/rectorphp/rector/issues/9401 And this triggered a new round of issues. https://github.com/rectorphp/rector/issues?q=phpunit+12

There is no will to fix this in PHPUnit (https://github.com/sebastianbergmann/phpunit/issues/6381), so we can either let tests crash, or fix it by going back to listing of all stmts nodes explicitly. It will be bit more verbose on our side, but community tests will work.

The imminent step is: removing vendor patches of php-parser.

Then as following, I see 2 options:

• add a new PR to php-parser with stmts interface without ClassMethod and hope it passes, that would resolve all our problems as feature we need would be in php-parser core - might take some time and its seems as not welcomed at the moment
• go back to full listing of nodes that contain $stmts - quick, just works and we can handle it ourselves

Feedback and practical solutions are welcomed

[TomasVotruba]

I made a PR for option 1: https://github.com/nikic/PHP-Parser/pull/1113

[samsonasik]

iirc, the preload is ensure the version rector php-parser use is the one from rector prefixed vendor, so even without patch, the order of rector preload still needed, except, in composer.json, we explicitly require nikic/php-parser, like phpstan does.

https://github.com/rectorphp/rector/blob/3176cfd0b40cadc6d81307d66adb00d25a962253/composer.json#L10-L13

That's to avoid when project require php-parser v4, run cause error.

[staabm]

I wonder, why you need StmtsAwareInterface.

couldn't you instead e.g. introduce a VirtualNode Type (like there are dozens in PHPStan) and do something like

final class StatementsAwareNode extends NodeAbstract implements VirtualNode {
  public function __construct(
     private Closure|Case_|Catch_|... $node  // put all types here, which you currently use `StmtsAwareInterface` on
  ) {}

  public function getStatements() {
    return $this->node->stmts;
  }
}

which you can instanceof on and which you can type in parameters/return types and which does not require hacks in php-parser.

[samsonasik]

@TomasVotruba I am testing that add this check on bootstrap.php early:

+if (defined('PHPUNIT_COMPOSER_INSTALL')) {
+    require_once __DIR__ . '/preload.php';
+}

// inspired by https://github.com/phpstan/phpstan/blob/master/bootstrap.php
spl_autoload_register(function (string $class): void {

make it works, tested on repo https://github.com/ostrolucky/rector-rules,

I will create PR for it, and verify more on projects.

[samsonasik]

@TomasVotruba @staabm I created PR to try handle on bootstrap.php

• https://github.com/rectorphp/rector-src/pull/7451

The logic can be more precise if needed.

[TomasVotruba]

@staabm Where would we place it? I think we're using PHPStan parser class and therefore unable to do this.

[TomasVotruba]

I made a simple repository to quickly test this type of autoload racecondition conflicts: https://github.com/rectorphp/rector-compat-tests

So far last stable fails on PHPUnit 12, but last dev passes. Time for release.

Any improvements or failing CI are welcomed. I'll add it here as part of CI checks, so know it works 👍

[TomasVotruba]

Let's get patch count to zero, to save us some troubles :).

Next one that to be removed: https://github.com/rectorphp/rector-src/pull/6232#issuecomment-3423162697