Skip to content

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

Merged
TomasVotruba merged 3 commits intorectorphp:masterfrom
mpdude:patch-1
Mar 4, 2021
Merged

Use the autoloader from where Rector was installed, not started#5772
TomasVotruba merged 3 commits intorectorphp:masterfrom
mpdude:patch-1

Conversation

@mpdude
Copy link
Copy Markdown
Contributor

@mpdude mpdude commented Mar 4, 2021 •
edited
Loading

#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.

samsonasik
samsonasik reviewed Mar 4, 2021
View reviewed changes
bin/rector.php Outdated

$autoloadIncluder->loadIfExistsAndNotLoadedYet(__DIR__ . '/../vendor/scoper-autoload.php');
$autoloadIncluder->loadIfExistsAndNotLoadedYet(getcwd() . '/vendor/autoload.php');
$autoloadIncluder->loadIfExistsAndNotLoadedYet(__DIR__ . '/../vendor/autoload.php');
Copy link
Copy Markdown
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.

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
Copy Markdown
Contributor Author

@mpdude mpdude Mar 4, 2021

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
Copy Markdown
Member

@samsonasik samsonasik Mar 4, 2021 •
edited
Loading

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
Copy Markdown
Contributor Author

@mpdude mpdude Mar 4, 2021

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
Copy Markdown
Contributor Author

@mpdude mpdude Mar 4, 2021

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
Copy Markdown
Contributor Author

@mpdude mpdude Mar 4, 2021

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
Copy Markdown
Contributor Author

@mpdude mpdude Mar 4, 2021

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
Copy Markdown
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.

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

Copy link
Copy Markdown
Contributor Author

@mpdude mpdude Mar 4, 2021

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
Copy Markdown
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.

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
@mpdude
Remove autoloader already covered in AutoloadIncluder::includeDepende…
e604910 
…ncyOrRepositoryVendorAutoloadIfExists()
@mpdude
Fix path to project autoloader
bc9a579 
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
Copy Markdown
Member

TomasVotruba commented Mar 4, 2021

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
This was referenced Mar 4, 2021
Merged
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 25, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@samsonasik samsonasik samsonasik left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mpdude @TomasVotruba @samsonasik