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

Use the autoloader from where Rector was installed, not started #5772

Merged
merged 3 commits into from
Mar 4, 2021

Conversation

mpdude
Copy link
Contributor

@mpdude mpdude commented Mar 4, 2021

#5665 should allow Rector to work without autoloading code from the project that's analyzed. Since I knew about the conflicts between Rector and older Symfony versions, and since I happen to have an older project with Symfony 2.3 around here, I tried to run a Rector Docker image with the changes from #5665 on in.

It fails with

Fatal error: Declaration of Symfony\Component\Console\Style\OutputStyle::write($messages, bool $newline = false, int $type = self::OUTPUT_NORMAL) must be compatible with Symfony\Component\Console\Output\OutputInterface::write($messages, $newline = false, $type = self::OUTPUT_NORMAL) in /rector/vendor/symfony/console/Style/OutputStyle.php on line 52

So, proably some mess-up between my project's version of Symfony and the one included in Rector.

I then modified vendor/autoload.php in my project to throw an exception:

Fatal error: Uncaught Exception in /project/vendor/autoload.php:3
Stack trace:
#0 /rector/bin/rector.php(115): require_once()
#1 /rector/bin/rector.php(30): AutoloadIncluder->loadIfExistsAndNotLoadedYet('/project/vendor...')
#2 /rector/bin/rector(4): require_once('/rector/bin/rec...')
#3 {main}
  thrown in /project/vendor/autoload.php on line 3

bin/rector includes my project's autoloader? That's because bin/rector.php loads vendor/autoload.php from the current cwd. At least in the Docker image, that's the project directory, not the /rector one.

Thus, I think it would make more sense to use __DIR__. That would always be the autoloader from where Rector was installed, not where it is running.

bin/rector.php Outdated
$autoloadIncluder->loadIfExistsAndNotLoadedYet(getcwd() . '/vendor/autoload.php');
$autoloadIncluder->loadIfExistsAndNotLoadedYet(__DIR__ . '/../vendor/autoload.php');
Copy link
Member

Choose a reason for hiding this comment

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

I think add new call is safer instead of change existing value:

Suggested change
$autoloadIncluder->loadIfExistsAndNotLoadedYet(getcwd() . '/vendor/autoload.php');
$autoloadIncluder->loadIfExistsAndNotLoadedYet(__DIR__ . '/../vendor/autoload.php');
$autoloadIncluder->loadIfExistsAndNotLoadedYet(getcwd() . '/vendor/autoload.php');
$autoloadIncluder->loadIfExistsAndNotLoadedYet(__DIR__ . '/../vendor/autoload.php');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I think we have to remove the cwd()-based one.

Copy link
Member

@samsonasik samsonasik Mar 4, 2021

Choose a reason for hiding this comment

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

that will broke when using composer require --dev rector/rector:dev-patch-1 with pointed to your repository. I tried your PR branch, and it got error:

vendor/bin/rector process app

PHP Fatal error:  Uncaught Error: Class "Rector\Core\Console\Style\SymfonyStyleFactory" not found in /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/bin/rector.php:35
Stack trace:
#0 /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/bin/rector(4): require_once()
#1 {main}
  thrown in /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/bin/rector.php on line 35
Fatal error: Uncaught Error: Class "Rector\Core\Console\Style\SymfonyStyleFactory" not found in /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/bin/rector.php:35
Stack trace:
#0 /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/bin/rector(4): require_once()
#1 {main}
  thrown in /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/bin/rector.php on line 35

with the following composer.json config:

	"require-dev": {
		"rector/rector": "dev-patch-1"
	},
	
	"repositories" : [
          {
                 "type" : "vcs",
                  "url" : "https://github.com/mpdude/rector",
		 "no-api": true
        }
    ]

Keep old method call will keep it working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I see... in Composer-based setups, it would have to be __DIR__.'/../../vendor/autoload.php probably?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@samsonasik Does it work if you completely remove the line in question here? So both ones, with cwd() and __DIR__?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not ../../../? (see the latest commit I just pushed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about https://github.com/rectorphp/rector/blob/master/src/Stubs/PHPStanStubLoader.php#L18-L23, but maybe that needs a bit of scrutinity, too...

Copy link
Member

Choose a reason for hiding this comment

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

__DIR__ . '/../../../../vendor/autoload.php' is same with __DIR__ . '/../../../autoload.php'. Just add more "up" to ensure the parent is named "vendor"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does my latest commit work?

Copy link
Member

Choose a reason for hiding this comment

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

yes, that seems works.

@mpdude mpdude changed the title [WIP] Use the autoloader from where Rector was installed, not started Use the autoloader from where Rector was installed, not started Mar 4, 2021
mpdude added 2 commits March 4, 2021 13:17
When Rector is installed as a project dependency through composer,
this script it will be in {vendor}/rector/rectory/bin/rector.php,
so ../../.. is where autoload.php lives.
@TomasVotruba
Copy link
Member

Thank you both 👍

@TomasVotruba TomasVotruba merged commit a2e36d5 into rectorphp:master Mar 4, 2021
@mpdude mpdude deleted the patch-1 branch March 4, 2021 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants