Skip to content

Commit

Permalink
work around bug with autoload collistion with custom PHPStan rule test
Browse files Browse the repository at this point in the history
  • Loading branch information
@TomasVotruba
TomasVotruba committed May 1, 2022
1 parent d5c927a commit 1c828ee
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion build/target-repository/bootstrap.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@
// prefixed version autoload
$composerAutoloader = require __DIR__ . '/vendor/autoload.php';
}
$composerAutoloader->loadClass($class);

// some weird collision with PHPStan custom rule tests
if (! is_int($composerAutoloader)) {
$composerAutoloader->loadClass($class);
}
}

// aliased by php-scoper, that's why its missing
Expand Down

6 comments on commit 1c828ee

@ondrejmirtes
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI @TomasVotruba In my testing (https://github.com/luxemate/phpstan-rector-error) this prevents the loadClass() on int error but actually doesn't make PHPStan work and find the class.

An idea - PHPStan needs this bootstrap.php because it's distributed as a PHAR, but Rector is distributed with normal directory structure, so it actually doesn't need such autoloading at all, it could just get by with PSR-4 configuration. WDYT?

@TomasVotruba
Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for sharing. I did not dig much into this issue (traveling and only merging/working).
This was a hotfix, when PHPStan rules tests were failing in one private project and in Symplify.

To the idea - I think you might be right. IIRC this file is actually a left-over from bootstrap times. Could you verify if removing this part make PHPStan and Rector both work? If so, let's drop it.

@ondrejmirtes
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sure that if rector/rector ships with composer.json that defines psr-4 or classmap in autoload for all Rector\\ and all RectorPrefix classes, and removes the bootstrap altogether, there's no more anything special about it, and it'd definitely work :)

The only challenge left is to generate the correct autoload entries for this, especially for the dependencies in vendor I guess :)

I did a quickfix for the current Rector version so that it works phpstan/phpstan-src@46f51e7 but I'd still like for you to get rid of this custom bootstrap. Thanks!

@TomasVotruba
Copy link
Member Author

Choose a reason for hiding this comment

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

I see.

I don't have space for this ATM. @samsonasik Could you check and process this one?

@samsonasik
Copy link
Member

Choose a reason for hiding this comment

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

@TomasVotruba I currently still on holiday with slow laptop so it probably will take slow update on try - error when there is error on scoped build to test locally.

@ondrejmirtes I don't understand what bootstrap is it, and the phpstan and rector autoload itself possibly overlapped, ref issue :

it is this file https://github.com/rectorphp/rector/blob/main/bootstrap.php ? Could you create a PR for it? Thank you.

@TomasVotruba
Copy link
Member Author

Choose a reason for hiding this comment

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

@samsonasik Feel free to check when you get back. We have this patched now in Rector and PHPStan, so this is just optimization.

Please sign in to comment.