From 80d5c78daa8fe7eee4616054a82592c6f14cba82 Mon Sep 17 00:00:00 2001 From: Theodore Brown Date: Sat, 12 Sep 2026 21:39:34 -0500 Subject: [PATCH] Ask Composer's autoloaders for the file instead of letting them include it AutoloadSourceLocator finds which file declares a class by running the registered autoloaders behind FileReadTrapStreamWrapper, which records the path an include reached for and serves an empty script in its place. That only shadows the real file while the compiler asks the wrapper for the contents. With OPcache already holding the script it does not ask, and the file runs a second time - fatal for one declaring a function, which is the function-per-file layout of php-standard-library and azjezz/psl: their files-autoload bootstrap has already loaded every path their PSR-4 prefix also resolves to. ClassLoader::findFile() answers with the same path loadClass() would include, without running anything, so ask it rather than arranging for the include to be harmless. Nothing is compiled and no cache is consulted, which is what makes this hold wherever PHP runs. Autoloaders still run in registration order: every non-Composer one runs inside the trap as before, one at a time, since an autoloader ahead of Composer's may claim a name Composer would resolve elsewhere. Running them one at a time also stops hoa/compiler's autoloader, registered ahead of the analysed project's loader, from forcing the whole probe down the include path. findFile() concatenates the mapped prefix with the rest of the name, so its answer can carry ../ segments and mixed separators, where PHP resolves an include path before the trap ever sees it. It is resolved here to match, which locateIdentifier() relies on when it compares the located path against ReflectionClass::getFileName() to tell two same-named classes in one file apart. The test drives the real locator in a subprocess under the worker's own OPcache flags: a name whose PSR-4 prefix resolves to a file the process already ran, which must not run it again, and one whose file nothing has loaded, which must still resolve without executing. Closes https://github.com/phpstan/phpstan/issues/15184 Co-Authored-By: Claude Opus 5 (1M context) --- .../SourceLocator/AutoloadSourceLocator.php | 112 ++++++++++++------ .../FileReadTrapStreamWrapperTest.php | 55 +++++++++ .../data/opcache-trap/ColdClass.php | 13 ++ .../data/opcache-trap/driver.php | 52 ++++++++ .../SourceLocator/data/opcache-trap/thing.php | 10 ++ 5 files changed, 208 insertions(+), 34 deletions(-) create mode 100644 tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/opcache-trap/ColdClass.php create mode 100644 tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/opcache-trap/driver.php create mode 100644 tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/opcache-trap/thing.php diff --git a/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php b/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php index 0a5a61c3793..7ead5e901f7 100644 --- a/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php +++ b/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php @@ -2,6 +2,7 @@ namespace PHPStan\Reflection\BetterReflection\SourceLocator; +use Composer\Autoload\ClassLoader; use Override; use ParseError; use PhpParser\Node\Arg; @@ -31,9 +32,11 @@ use function defined; use function function_exists; use function interface_exists; +use function is_array; use function is_file; use function is_string; use function opcache_invalidate; +use function realpath; use function restore_error_handler; use function set_error_handler; use function spl_autoload_functions; @@ -333,58 +336,99 @@ private function locateClassByName(string $className): ?array return null; } + $autoloadFunctions = spl_autoload_functions(); + if ($autoloadFunctions === false) { + return null; + } + $this->silenceErrors(); try { - $result = FileReadTrapStreamWrapper::withStreamWrapperOverride( - static function () use ($className): ?array { - $functions = spl_autoload_functions(); - if ($functions === false) { - return null; - } + return self::locateThroughAutoloaders($autoloadFunctions, $className); + } finally { + restore_error_handler(); + } + } - foreach ($functions as $preExistingAutoloader) { - try { - $preExistingAutoloader($className); - } catch (ParseError) { - // the trap served a parse error instead of the empty - // script, see FileReadTrapStreamWrapper::stream_read(); - // the file was recorded before the include compiled it - } + /** + * Runs the registered autoloaders, in order, to find which file declares a + * class - asking Composer's where it would look instead of letting it get + * there by including the file. + * + * ClassLoader::findFile() answers with the same path loadClass() would + * include, without running anything. Nothing is compiled, so the file cannot + * execute a second time - which it otherwise does whenever OPcache already + * holds the script: the include is then served from the cache without the + * trap being asked for the contents at all, and a file declaring a function + * fatals with "Cannot redeclare". That is the function-per-file layout of + * php-standard-library and azjezz/psl, whose files-autoload bootstrap has + * already loaded every path their PSR-4 prefix also resolves to. + * + * Every other autoloader still runs inside the trap, one at a time so that + * registration order is preserved either way: an autoloader ahead of + * Composer's may well claim a name Composer would resolve elsewhere. + * + * @param list $autoloadFunctions + * @return array{string[], string, int|null}|null + */ + private static function locateThroughAutoloaders(array $autoloadFunctions, string $className): ?array + { + foreach ($autoloadFunctions as $autoloadFunction) { + if (is_array($autoloadFunction) && $autoloadFunction[0] instanceof ClassLoader) { + $file = $autoloadFunction[0]->findFile($className); + if ($file === false) { + continue; + } - /** - * This static variable is populated by the side-effect of the stream wrapper - * trying to read the file path when `include()` is used by an autoloader. - * - * This will not be `null` when the autoloader tried to read a file. - */ - if (FileReadTrapStreamWrapper::$autoloadLocatedFiles !== []) { - return [FileReadTrapStreamWrapper::$autoloadLocatedFiles, $className, null]; - } + // findFile() concatenates the mapped prefix with the rest of the + // name, so its answer can carry ../ segments and mixed + // separators. PHP resolves an include path before the trap ever + // sees it, and locateIdentifier() matches what lands here against + // ReflectionClass::getFileName(), which is resolved too. + $resolvedFile = realpath($file); + + // a class map can outlive the file it points at, and loadClass() + // would move on to the next autoloader just the same + if ($resolvedFile === false || !is_file($resolvedFile)) { + continue; + } + + return [[$resolvedFile], $className, null]; + } + + $locatedFiles = FileReadTrapStreamWrapper::withStreamWrapperOverride( + static function () use ($autoloadFunction, $className): array { + try { + $autoloadFunction($className); + } catch (ParseError) { + // the trap served a parse error instead of the empty + // script, see FileReadTrapStreamWrapper::stream_read(); + // the file was recorded before the include compiled it } - return null; + // populated by the side effect of the stream wrapper being + // asked for the file an include() reached for + return FileReadTrapStreamWrapper::$autoloadLocatedFiles; }, ); - if ($result === null) { - return null; - } - if (!function_exists('opcache_invalidate')) { - return $result; + if ($locatedFiles === []) { + continue; } // the trap's empty script got compiled - and cached, with OPcache // active. Where this call cannot reach the entry, the trap served a // parse error instead, see FileReadTrapStreamWrapper::stream_read() - foreach ($result[0] as $file) { - opcache_invalidate($file, true); + if (function_exists('opcache_invalidate')) { + foreach ($locatedFiles as $locatedFile) { + opcache_invalidate($locatedFile, true); + } } - return $result; - } finally { - restore_error_handler(); + return [$locatedFiles, $className, null]; } + + return null; } private function silenceErrors(): void diff --git a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapperTest.php b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapperTest.php index cd1a78087bc..00b288df605 100644 --- a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapperTest.php +++ b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapperTest.php @@ -3,7 +3,16 @@ namespace PHPStan\Reflection\BetterReflection\SourceLocator; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; +use function escapeshellarg; +use function exec; +use function explode; +use function extension_loaded; +use function implode; +use function sprintf; +use function str_contains; +use const PHP_BINARY; final class FileReadTrapStreamWrapperTest extends TestCase { @@ -29,4 +38,50 @@ public function testResolveServesParseError(int $phpVersionId, bool $opcacheEnab $this->assertSame($expected, FileReadTrapStreamWrapper::resolveServesParseError($phpVersionId, $opcacheEnabled, $path)); } + /** + * Probing a name whose PSR-4 prefix resolves to a file the process already + * ran must not run it again: with OPcache holding the script, an include is + * served from the cache without the trap being asked for the contents. A name + * whose file nothing has loaded must still resolve, without executing it. + * + * Needs its own process: OPcache is only on in the processes PHPStan spawns + * for itself, and the failure is a fatal error. + */ + #[Group('exec')] + public function testTrapSurvivesOpcacheCacheHit(): void + { + if (!extension_loaded('Zend OPcache')) { + self::markTestSkipped('OPcache is not available.'); + } + + exec(sprintf( + '%s -d opcache.enable=1 -d opcache.enable_cli=1 -d opcache.validate_timestamps=0 %s 2>&1', + escapeshellarg(PHP_BINARY), + escapeshellarg(__DIR__ . '/data/opcache-trap/driver.php'), + ), $outputLines, $exitCode); + $output = implode("\n", $outputLines); + + $this->assertSame(0, $exitCode, $output); + + $values = []; + foreach ($outputLines as $outputLine) { + if (!str_contains($outputLine, '=')) { + continue; + } + [$name, $value] = explode('=', $outputLine, 2); + $values[$name] = $value; + } + + // hold whether or not OPcache could be turned on + $this->assertSame('1', $values['survivedLoadedProbe'] ?? null, $output); + $this->assertSame('1', $values['resolvedCold'] ?? null, $output); + $this->assertSame('1', $values['coldFileNotExecuted'] ?? null, $output); + + if (($values['opcacheEnabled'] ?? '0') === '1') { + return; + } + + self::markTestSkipped('OPcache could not be enabled for the CLI, so the cache hit was not exercised.'); + } + } diff --git a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/opcache-trap/ColdClass.php b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/opcache-trap/ColdClass.php new file mode 100644 index 00000000000..b8efd683567 --- /dev/null +++ b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/opcache-trap/ColdClass.php @@ -0,0 +1,13 @@ +addPsr4('OpcacheTrap\\', [__DIR__]); + +$locator = new AutoloadSourceLocator( + PHPStanTestCase::getContainer()->getByType(FileNodesFetcher::class), + true, +); +$reflector = new DefaultReflector($locator); + +// OpcacheTrap\thing is a function, not a class, but PHPStan probes the name as +// a class the same way it does for Psl\Type\optional - and the PSR-4 prefix +// sends the autoloader at the already-loaded function.php +$locator->locateIdentifier($reflector, new Identifier('OpcacheTrap\thing', new IdentifierType(IdentifierType::IDENTIFIER_CLASS))); +echo "survivedLoadedProbe=1\n"; + +// a name whose file nothing has loaded must still resolve +$cold = $reflector->reflectClass('OpcacheTrap\ColdClass'); +echo 'resolvedCold=', $cold->getName() === 'OpcacheTrap\ColdClass' ? '1' : '0', "\n"; +echo 'coldFileNotExecuted=', !function_exists('OpcacheTrap\coldThing') ? '1' : '0', "\n"; diff --git a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/opcache-trap/thing.php b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/opcache-trap/thing.php new file mode 100644 index 00000000000..03ce0d9c529 --- /dev/null +++ b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/data/opcache-trap/thing.php @@ -0,0 +1,10 @@ +