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

Allow autoloaders to require/require_once for #4836 via a fake read #491

Merged
merged 1 commit into from
Apr 11, 2021

Conversation

rrazor
Copy link
Contributor

@rrazor rrazor commented Apr 10, 2021

Overview

PHPStan issue #4836 occurs when a custom autoloader uses require or require_once and PHPStan tries to process a file that contains an anonymous class definition with dependencies that must be autoloaded (see issue for details).

Approach

The goal of FileReadTrapStreamWrapper is to use a custom autoloader's knowledge of where to find a file, grab the path, and shut down the rest of the include by returning false from stream_open.

This PR allows require and require_once to be used in a custom autoloader by allowing the entire file read to occur, with an empty string returned from stream_read() to serve as dummy data that is also valid PHP.

Testing

I ran the complete PHPStan test suite and observed no errors. I also confirmed that the updated code succeeded in my sample repository.

Potential issues

Since we simulate a file read from beginning to end, include_once and require_once will not try to load the same path again. I think this is okay because I assume PHPStan loads the file itself later.

@ondrejmirtes
Copy link
Member

I copied your test project to PHPStan's E2E tests: phpstan/phpstan@2bef7e4

After it fails the build, I'll merge this. Thank you very much!

@ondrejmirtes ondrejmirtes merged commit 161c383 into phpstan:master Apr 11, 2021
@ondrejmirtes
Copy link
Member

BTW Feel free to also submit this to https://github.com/Roave/BetterReflection, thank you.

// Dummy return value that is also valid PHP for require(). We'll read
// and process the file elsewhere, so it's OK to provide dummy data for
// this read.
return '';
Copy link
Contributor

Choose a reason for hiding this comment

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

This causes problems when OPcache is enabled. OPcache will remember this empty string when the file is next autoloaded (which might not be within FileReadTrapStreamWrapper).

Copy link
Contributor

Choose a reason for hiding this comment

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

It also causes problems when require_once or include_once is used, regardless of whether opcache is enabled or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants