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

Compatibility with composer-bin-plugin #1897

Open
remorhaz opened this issue Nov 22, 2023 · 3 comments
Open

Compatibility with composer-bin-plugin #1897

remorhaz opened this issue Nov 22, 2023 · 3 comments

Comments

@remorhaz
Copy link

Is your feature request related to a problem? Please describe.
I don't know if it's bug report or feature request. I'm using bamarni/composer-bin-plugin to isolate my tools' dependencies, and I would like to be able to use it with Infection, too.

In my case the problem was thecodingmachine/safe package - it throwed a fatal error during autoloading (thecodingmachine/safe#422, probably):

PHP Fatal error:  Cannot redeclare Safe\mysqli_get_client_stats() (previously declared in /app/vendor-bin/infection/vendor/thecodingmachine/safe/deprecated/mysqli.php:16) in /app/vendor/thecodingmachine/safe/generated/mysqli.php on line 41

I've started to investigate and found that to fix #795 issue Infection includes project's autoloader. But this solution creates incompatibility issue with bamarni/composer-bin-plugin.

What in fact happens: composer-bin-plugin introduces "namespaces" for Composer installations and enables installing tools independently in sub-directories like vendor-bin/infection. When I install Infection in such manner and try to run vendor-bin/infection/vendor/bin/infection --version, I'm getting an error, because Infection also includes project autoloader (that uses older version of Safe package with unsolved thecodingmachine/safe#253 issue).

Describe the solution you'd like
The origin of the problem is probably in thecodingmachine/safe package, but it's Infection that triggers it. Maybe it's possible to avoid this behavior? Remove it or, at least, enable switching it off.

Describe alternatives you've considered
Alternative is to use Infection as PHAR, but it's impossible to use PHAR for tools that use plugins (such as Psalm). It would be convenient to use a single technique for all tools.

@theofidry
Copy link
Member

Hi!

I would like to be able to use it with Infection, too.

I think it's a bad idea in the current state of things. As you noted, infection need to autoload your code. So if you install it in a different composer.json (which is what bamarni/composer-bin-plugin does), then you can end up with various issues:

  • The same class is declared in both infection (or its dependencies) and your project: in this case the second declaration will not be loaded (and their implementation may differ, PHP will just see "this class has been loaded already" and won't load it again).
  • Same with functions, in which case if they don't have the if (!function_exists(...)) which you will end up with declaring twice a function which crashes PHP.

Using Infection PHAR would be no different if it was a regular PHAR. However Infection uses PHP-Scoper to address those issues.

As the main contributor and a heavy user of bamarni/composer-bin-plugin, I totally understand the concern of wanting to isolate tools dependencies. However, I think this approach cannot be systematic due to the reasons outlined above. To be able to do so, you need either a tool that doesn't autoload your code, or a tool for which its code is scoped (like Infection PHAR, or like the phpstan package which ships the scoped PHPStan PHAR).

A possible solution however, would be to do like PHPStan, i.e. have a proxy package which ships the isolated PHAR. I do not know how that works with extensions though, IIRC PHPStan managed to get around with it, I do not know what it entails for Infection.

@remorhaz
Copy link
Author

remorhaz commented Nov 22, 2023

Thank you for the fast and detailed answer!

Of course, if Infection needs to autoload project dependencies, it makes it impossible to use it with composer-bin-plugin and I'll have to switch to PHAR version; thanks to PHP-Scoper, it's possible.

But I still don't completely understand why Infection needs to load project's dependencies. I thought that it uses PHPUnit CLI to run tests and PHP-Parser to parse sources; but perhaps I'm missing something, maybe reflection is also used. Anyway, the tool is great and helps a lot.

@theofidry
Copy link
Member

but perhaps I'm missing something, maybe reflection is also used.

Yes :) It uses parses the AST of the source code and may leverage reflection for some mutations. It could be delegated to BetterReflection at some point, but not sure what the performance impact would be.

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

No branches or pull requests

2 participants