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

AutoloadSourceLocator should restore pre-existing stream handlers #3854

Closed
nreynis opened this issue Sep 10, 2020 · 10 comments
Closed

AutoloadSourceLocator should restore pre-existing stream handlers #3854

nreynis opened this issue Sep 10, 2020 · 10 comments

Comments

@nreynis
Copy link

nreynis commented Sep 10, 2020

Feature request

AutoloadSourceLocator uses stream handlers black magic internally.
This conflict with stream-handler defined by the user. Most notably it breaks db/bypass-finals.

Related issues

User failing to make bypass-final works despite bootstrapping it
#3179

Reproducible setup (issue is mislocated, it's not related to the Symfony plugin)
phpstan/phpstan-symfony#96

@ondrejmirtes
Copy link
Member

I don't think it's possible in PHP to do that. I'd rather make an option to disable AutoloadSourceLocator.

@nreynis
Copy link
Author

nreynis commented Sep 10, 2020

I have a suggestion to handle it.

If you make an option to disable AutoloadSourceLocator wouldn't it cause false-positive or weaken the analysis by skipping some files?

@ondrejmirtes
Copy link
Member

wouldn't it cause false-positive or weaken the analysis by skipping some files?

It wouldn't allow you to use autoloader-based symbols discovering. Which is a fine trade-off IMHO, using hack like bypass-finals is already an edge-case, autoloader-based symbols discovering is another edge-case so not supporting an intersection of these two edge cases is fine by me.

@ondrejmirtes
Copy link
Member

This is what would stop working: https://phpstan.org/user-guide/discovering-symbols#custom-autoloader

@nreynis
Copy link
Author

nreynis commented Sep 11, 2020

If I understand correctly it won't affect a codebase running a standard psr-4 autoloader? That seems acceptable.

@ondrejmirtes
Copy link
Member

Yeah if you define autoload in composer.json you could disable AutoloadSourceLocator 😊

@ondrejmirtes
Copy link
Member

Just realized there's a much easier way to handle this:

  1. Detect that bypass-finals is installed.
  2. Do not consider any class final in ClassReflection::isFinal().

@nreynis
Copy link
Author

nreynis commented Oct 29, 2020

This may work, but this is kind of weird... you are not fixing the compatibility issue, you are reimplementing the functionality in PHPStan codebase.

Also this would still break when using 'setWhitelist' method of bypass-final.

For the moment I'm working around the issue by preloading all the classes during bootstrap. This way they go through bypass-final before the analysis start and the stream wrapper conflict arise. I'm using this classfinder lib to do this https://packagist.org/packages/haydenpierce/class-finder

This is a bit 'brute-force' but I think it's a decent (and safe) fix.

@ondrejmirtes
Copy link
Member

I'm unlikely to work on a fix for this myself, I guess many people already found their own workaround for this (or stopped using bypass-finals).

@github-actions
Copy link

github-actions bot commented May 2, 2021

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants