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

Improve autoloading #50

Closed
wants to merge 3 commits into from
Closed

Conversation

tyx
Copy link
Contributor

@tyx tyx commented Dec 26, 2016

I just get back one part of the @lvht work from #48 as I was going to do the same ^^

I also added another way to customize autoloading like behat does.

The idea is to use phpstan globally on our side

@shouze
Copy link

shouze commented Dec 26, 2016

ping @ondrejmirtes WDYT?

@ondrejmirtes
Copy link
Member

@shouze @tyx Thanks, I mostly like it. I have some questions/requests:

  • Does it work when PHPStan is ran from a global installation and it's not in a directory with discoverable getcwd() . '/vendor/autoload.php')? The branch with !class_exists('PHPStan\Command\AnalyseCommand', true) is executed and I'm not sure what happens then.
  • Please describe the basics of the new option and behaviour in README.

Other issues I found will be commented in the code.

@@ -39,6 +40,11 @@ public function getAliases(): array

protected function execute(InputInterface $input, OutputInterface $output): int
{
$autoloadFile = $input->getOption('autoload-file');
if ($autoloadFile && is_file($autoloadFile)) {
Copy link
Member

Choose a reason for hiding this comment

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

Please write $autoloadFile !== null.

if (!is_file($composerAutoloadFile)) {
$composerAutoloadFile = __DIR__ . '/../../../autoload.php';
if (is_file($autoload = getcwd() . '/vendor/autoload.php')) {
require $autoload;
Copy link
Member

Choose a reason for hiding this comment

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

require_once

@tyx
Copy link
Contributor Author

tyx commented Dec 27, 2016

Does it work when PHPStan is ran from a global installation and it's not in a directory with discoverable getcwd() . '/vendor/autoload.php')? The branch with !class_exists('PHPStan\Command\AnalyseCommand', true) is executed and I'm not sure what happens then.

No it will not. In this case you should use the new option introduced by @lvht. Without, PHPStan will pop ClassNotFoundException everywhere. I have updated the README to explain

It will allow people to use phpstan globally just by adding their own autoload.php with the path they want inside.
@shouze
Copy link

shouze commented Dec 29, 2016

ping @ondrejmirtes is it ok to you now?

@shouze shouze mentioned this pull request Dec 29, 2016
@ondrejmirtes
Copy link
Member

Thanks. I merged it as 3c71eda and 57f2ebe. I will merge the README with some changes later.

@ondrejmirtes
Copy link
Member

This is how I changed the README: c867e71

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.

None yet

4 participants