Rector: [Install] Conflict with PHP-Parser

[carusogabriel]

Recently, I tried to use Rector with Siler. As Siler requires PHPStan, and PHPStan in its turn requires PHP-Parser v3, I got an conflict:

PHP Fatal error:  Declaration of PhpParser\Node\Stmt\PropertyProperty::getSubNodeNames() must be compatible with PhpParser\Node::getSubNodeNames(): array in /var/www/siler/vendor/nikic/php-parser/lib/PhpParser/Node/Stmt/PropertyProperty.php on line 30

I tried to solve it, but when looking into rector_bootstrap.php, I find out that the $possibleAutoloadPaths are the same:

$possibleAutoloadPaths = [
    // repository
    __DIR__ . '/../vendor/autoload.php',
    // composer create-project
    __DIR__ . '/../vendor/autoload.php',
    // composer require
    __DIR__ . '/../../../../vendor/autoload.php',
];

I first try to require it:

composer require --dev rector/rector 'dev-master' nikic/php-parser '4.0.x-dev'

And got the versions conflict.

Then tried to use my version of:

composer create-project rector/rector:'dev-master' path-to-rector

And find out this conflict.

@TomasVotruba How should we solve it? I know that is one more issue, but I couldn't figure out how :disappointed:

[TomasVotruba]

There are 2 ways how to solve this:

• use phpstan shim as dependency: https://github.com/rectorphp/rector/blob/5b11f5137f95171522e8700109d67ad046127e2a/composer.json#L30, which keeps phpstan's dependencies out of project scope - we use it here in Rector
• use composer create-project rector/rector ... which keeps rector's dependencies out of project scope

[carusogabriel]

Both solutions did not work :cry:

Could you reproduce what I report?

[TomasVotruba]

Oh, Rector probably looks for it's autoloader:
https://github.com/rectorphp/rector/blob/bddf7586545ba781a108a2ae9fdb33d946696f6b/packages/BetterReflection/src/SourceLocator/SourceLocatorFactory.php#L93

Which creates the conflict. Could try comment this line if that helps?

Maybe we should handle autoloading in explicit way, like phpstan does. But we still need to get to dependencies in /vendor in some way. E.g. while detecting it's instance of PHPUnit\Framework\TestCase

What do you think?

[carusogabriel]

@TomasVotruba I'm trying to figure out how we can skip the conflict, but accessing our autoloader.

Maybe we should handle autoloading in explicit way, like phpstan does

What do you suggest?

[TomasVotruba]

To solve this we'd actually need this approach: https://github.com/fprochazka/phpstan-compiler

We could use the same approach as PHPStan with 2 clear choices: https://github.com/phpstan/phpstan#installation

• 
  
  1. composer require ... where it dependencies are ok
  
  
• 
  
  1. rector-shim, where it doesn't work
  
  
• (with docker as an option)

That means: remove composer create-project ...

@lexinek Hey, thanks for today's conversation. This is actually solution to the problem we've talked about. Anything else would not work. Could you look at that compiler and prepare similar package for Rector? That would help a lot

[TomasVotruba]

Ref: https://github.com/rectorphp/rector/commit/7b3d68fc03d6548fa22a36dbea6e6f14d8bda4fe

[ondrejmirtes]

Hi, PHPStan author here. phpstan-shim does not work in this case because I don't prefix PhpParser namespace - it's used in public-facing extension interfaces in PHPStan.

I don't see when you are encountering this error. While running PHPStan? While running Rector?

PHP Fatal error:  Declaration of PhpParser\Node\Stmt\PropertyProperty::getSubNodeNames() must be compatible with PhpParser\Node::getSubNodeNames(): array in /var/www/siler/vendor/nikic/php-parser/lib/PhpParser/Node/Stmt/PropertyProperty.php on line 30

...to help you how to prevent it. One option is to start using composer-bin-plugin but I don't have any experience with it whether it actually helps.

[TomasVotruba]

Hey Ondra,

the issue comes up while using Rector in project with PHPStan as dependency. They both use different mayor version of Php-parser (4 is not tagged yet). One class has typehint and the other doesn't -> crash.

Too bad shim approach won't help us here, since Rector also uses few of PHP parser classes in public API.

Thanks for link to plugin, I will check it.
I want to test this approach too:
https://medium.com/@tfidry/managing-your-dependencies-in-php-321d584441ab

[ondrejmirtes]

So it crashes when Rector stumbles upon a PHPStan class or what? Can we exclude some files so that it does not happen?

[carusogabriel]

Hello @ondrejmirtes, thanks for your comment. I’ve reported all bugs that I can found to @TomasVotruba, while running Rector, so we can prepare it to the real world applications.
But, I guess all this conflicts are because of one, and only one key: Rector uses PHP-Parser version 4, with is still in beta.
Maybe we can face some problems in the future with autoloader, and I’ll study your solutions (thanks for them).
I’m working on other bugs and improvements now, but I really thank all enford of our community to bring Rector to life 😁

[carusogabriel]

@ondrejmirtes Perhaps the best to do now is wait until PHP-Parser 4 rits a stable version, and we can update every tool that uses it, or maybe add support!

[TomasVotruba]

@ondrejmirtes It crashes on using different PHP-Parser versions, see the issues description. There is the error in highlight.

[ondrejmirtes]

Does it crash on every run or on a specific file?

On Thu, 4 Jan 2018 at 15:04, Tomáš Votruba [e-mail] wrote:

@ondrejmirtes https://github.com/ondrejmirtes It crashes on using
different PHP-Parser versions, see the issues description. There is the
error in highlight.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rectorphp/rector/issues/258#issuecomment-355289735,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGZuG9mDDlifOJQvmI8tWt2218yCIrwks5tHNpRgaJpZM4RQQLm
.

>

Ondřej Mirtes

[carusogabriel]

@OndraM I guess there's nothing to do with PHPStan, I'm really sorry to bore you with it. Everything that we should do now is to wait for PHP-Parser hits version 4, and we can upgrade PHPStan. So we can ensure that Rector runs on applications that require PHPStan in require-dev section.

[TomasVotruba]

I've tried approach suggested by @theofidry

composer require bamarni/composer-bin-plugin --dev
composer bin rector require rector/rector:dev-master#6e7297b --dev nikic/php-parser 4.0.x-dev
vendor/bin/rector

It works on version conflict test case with older PHPUnit diff.

@carusogabriel Could you try on your use case?

[theofidry]

👍

Also if you want a more complete list of possible solutions I recommend you this article :)

[TomasVotruba]

@theofidry I know, I linked it above :)
If you read this issue, do you suggest something better to use for this project?

Thanks a lot for the post btw! We looked for solution for this for over a month now

[carusogabriel]

@TomasVotruba @theofidry Thanks for the help

[theofidry]

I linked it above :)

Oups missed that

do you suggest something better to use for this project?

I either recommend PHARs (isolated if executes code) or the approach with bamarni. If you don't want to think too much about it I would recommend the bamarni approach as the other ones requires a bit more thinking for keeping track of the PHAR and if the PHAR is not isolated it can require some work for it...

TL:DR; the approach you suggested is perfectly fine, I use it extensively

[TomasVotruba]

@theofidry Thanks, that gives me peaceful mind :dove:

I have very bad experience with PHARs as maintainer (caused 50 % issues) and user. PHAR even in this case would only cause errors, since both the autoloading and version conflicts need to be covered.

I'm amazed by solutions that PHP community comes with, really happy to be part of this

[theofidry]

To be honest I couldn’t dig much into the auto loading issue yet: is rector
trying to load the auto loader of the analysed project?

On Sat 6 Jan 2018 at 17:17, Tomáš Votruba [e-mail] wrote:

@theofidry https://github.com/theofidry Thanks, that gives me peaceful
mind 🕊

I have very bad experience with PHARs as maintainer (caused 50 % issues)
and user. PHAR even in this case would only cause errors, since both the
autoloading and version conflicts need to be covered.

I'm amazed with solutions that PHP community comes with, really happy to
be part of this

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rectorphp/rector/issues/258#issuecomment-355757170,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AE76gR9ry2FeMhZmcfegG3_nEAvrVWw9ks5tH5yagaJpZM4RQQLm
.

[TomasVotruba]

Now it is as part of BetterReflection: https://github.com/rectorphp/rector/blob/8fca502b51d90809eab34ac608c3feffaef6abab/packages/BetterReflection/src/SourceLocator/SourceLocatorFactory.php#L93

But thanks to the new solution, we could move away from this. BetterReflection is the slowest component here and aims on not-autoloading classes, which brings no value to this projects, because it basically ends up parsing whole /vendor directory.

[carusogabriel]

Closing as we need to wait for PHP-Parser v4 to fix this conflict...

[TomasVotruba]

@carusogabriel The vendor-bin plugin didn't help?

[carusogabriel]

Yes, sorry, fixed. I tried to mention that to remove this conflict solution we will need to wait until that :sweat_smile:

[TomasVotruba]

That's great :+1:

I'm happy it solved so many troubles we had

[TomasVotruba]

Follow up in #319