Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure autoloader is loaded in globally installed Rector #1683

Merged
merged 4 commits into from
Jan 16, 2022
Merged

Ensure autoloader is loaded in globally installed Rector #1683

merged 4 commits into from
Jan 16, 2022

Conversation

PhilETaylor
Copy link
Contributor

@PhilETaylor PhilETaylor commented Jan 16, 2022

Fixes rectorphp/rector#6903

When running a globally installed Rector, autoloading of the projects vendors is not happening

This PR changes that

To replicate the problem

git clone git@github.com:MGatner/rector-error.git
cd rector-error
composer install 
composer global install rector/rector
rector process -vv

Before this PR:

 [ERROR] Could not process "src/MyException.php" file, due to:
         "System error: "PHPStan\BetterReflection\Reflection\ReflectionClass
         "CodeIgniter\Exceptions\ExceptionInterface" could not be found in the
         located source"
         Run Rector with "--debug" option and post the report here:
         https://github.com/rectorphp/rector/issues/new". On line: 26

After this PR

Screenshot 2022-01-16 at 00 29 46

Tagging @MGatner

@PhilETaylor
Copy link
Contributor Author

Seems that the unit tests dont like me :-( But this fix does work...

@PhilETaylor PhilETaylor changed the title ensure autoloader is loaded in globally installed Rector Ensure autoloader is loaded in globally installed Rector Jan 16, 2022
Copy link

@MGatner MGatner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find!

@samsonasik
Copy link
Member

@PhilETaylor I rebased your branch

@samsonasik
Copy link
Member

@TomasVotruba it seems there is error in CI that unrelated to it:

Run vendor/bin/easy-ci validate-file-length packages rules src tests

Error: ] Class "Symplify\EasyCI\ValueObject\Option" not found       

even happen on automated PR #1682

@samsonasik
Copy link
Member

@TomasVotruba it seems Symplify\EasyCI\ValueObject is now scoped, and the generated namespace is:

namespace EasyCI20220116\Symplify\EasyCI\ValueObject;

https://github.com/symplify/easy-ci/blob/a21fa9ea29812dbcaaa2f53b430f874cd0e54112/src/ValueObject/Option.php#L4

It resolved by temporary pin symplify/easy-ci to 10.0.9 70953ba

@samsonasik
Copy link
Member

Thank you @PhilETaylor

@samsonasik samsonasik merged commit 72de31e into rectorphp:main Jan 16, 2022
@samsonasik
Copy link
Member

@PhilETaylor it somehow make e2e error on rector/rector, ref https://github.com/rectorphp/rector/runs/4829814122?check_suite_focus=true

I will check if it related to it.

@samsonasik
Copy link
Member

@PhilETaylor it seems indeed cause error on e2e, revert it seems solve it, the possible solution possibly mention in the documentation about it.

@PhilETaylor
Copy link
Contributor Author

Strange that the error is a PHP syntax error though, seems unrelated. In bed now but will dig further tomorrow

@MGatner
Copy link

MGatner commented Jan 16, 2022

I'm actually surprised that this was the approved way to load the project autoloader to being with:

 $this->loadIfExistsAndNotLoadedYet(__DIR__ . '/../../../autoload.php');

Given that Rector could be run from theoretically anywhere starting from __DIR__ seems dangerous. That could easily include some random unrelated vendor autoload by accident.

Surely there is some cleaner way to identify "project root", or "folder containing composer.json"?

@PhilETaylor
Copy link
Contributor Author

The test seem to also be loading from a non standard location involving ../../bin/rector

Run ../../bin/rector process --dry-run --ansi

@samsonasik
Copy link
Member

@MGatner that's trade off for using it globally as far as I know.
@PhilETaylor that's seems fine as pointed to path, not symlink global as far as I know.

I created PR #1685 to revert it with additional doc for using it globally (while not recommended) while we wait if there is possible solution for it.

@MGatner
Copy link

MGatner commented Jan 16, 2022

it with additional doc for using it globally (while not recommended)

I am with Sebastian Bergmann on static analysis tools not being included as project dependencies. It introduces unnecessary extra files and the opportunity for collisions. Rector does an amazing job of running separate from the project code - why restrict external execution? It seems like with all its project knowledge the executable ought to be able to locate autoload.php no matter where it is called from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants