From e003c758e1ce246a3f886ee75b7d9ed9fc46ecda Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Thu, 27 Aug 2026 22:53:19 +0200 Subject: [PATCH] Do not let a bootstrap autoloader's exception abort the analysis Both source locators that ask the registered autoloaders for a class walk the whole spl queue, which is not the order PHP uses: at runtime the class loader that resolves the class first means the other autoloaders are never invoked for that name. An autoloader that throws when the file it found does not declare the class it was asked for therefore throws here for names it can never see at runtime, and the exception surfaces as an internal error that aborts the file. Two widely used ones do that - symfony/error-handler's DebugClassLoader, which throws a RuntimeException in four conditions, and Yii 2's Yii::autoload(), which throws an UnknownClassException when the file it included did not declare the class and YII_DEBUG is on. Swallowing it lets the remaining autoloaders and source locators take their turn, so the class is resolved the way it is at runtime, or reported as not found. The trapped loop in AutoloadSourceLocator has the same defect; it predates the recently reverted work and is fixed here too, since fixing only one of the two leaves any name no static locator resolves still aborting. Unlike the reverted attempts this changes neither the order of the locators nor the way autoloaders are run, and it declines nothing by name. e2e/debug-class-loader is the DebugClassLoader case with the real package: an internal error before, the unknown class reported after. e2e/bug-12972b, the project phpstan/phpstan#14976 is about, is re-enabled. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/e2e-tests.yml | 14 ++- e2e/debug-class-loader/.gitignore | 2 + e2e/debug-class-loader/bootstrap.php | 8 ++ e2e/debug-class-loader/composer.json | 10 ++ e2e/debug-class-loader/phpstan.dist.neon | 12 +++ e2e/debug-class-loader/src/Widget.php | 10 ++ e2e/debug-class-loader/test.php | 17 ++++ .../AutoloadFunctionsSourceLocator.php | 15 ++- .../SourceLocator/AutoloadSourceLocator.php | 15 ++- .../AutoloadFunctionsSourceLocatorTest.php | 97 +++++++++++++++++++ .../AutoloadSourceLocatorTest.php | 38 ++++++++ 11 files changed, 227 insertions(+), 11 deletions(-) create mode 100644 e2e/debug-class-loader/.gitignore create mode 100644 e2e/debug-class-loader/bootstrap.php create mode 100644 e2e/debug-class-loader/composer.json create mode 100644 e2e/debug-class-loader/phpstan.dist.neon create mode 100644 e2e/debug-class-loader/src/Widget.php create mode 100644 e2e/debug-class-loader/test.php create mode 100644 tests/PHPStan/Reflection/BetterReflection/SourceLocator/AutoloadFunctionsSourceLocatorTest.php diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index ae29efb84c7..db673ca35f3 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -199,15 +199,19 @@ jobs: cd e2e/bug-14514 composer install ../../bin/phpstan analyze bug-14515.php - # Fails since the revert of the bootstrap autoloader changes: + # Fails until https://github.com/phpstan/phpstan/issues/14988 is fixed again: # - script: | # cd e2e/bug-14988 # composer install # ../../bin/phpstan analyse - # - script: | - # cd e2e/bug-12972b - # composer install - # ../../bin/phpstan analyze + - script: | + cd e2e/bug-12972b + composer install + ../../bin/phpstan analyze + - script: | + cd e2e/debug-class-loader + composer install + ../../bin/phpstan analyze - script: | cd e2e/bug-12972c composer install diff --git a/e2e/debug-class-loader/.gitignore b/e2e/debug-class-loader/.gitignore new file mode 100644 index 00000000000..de4a392c331 --- /dev/null +++ b/e2e/debug-class-loader/.gitignore @@ -0,0 +1,2 @@ +/vendor +/composer.lock diff --git a/e2e/debug-class-loader/bootstrap.php b/e2e/debug-class-loader/bootstrap.php new file mode 100644 index 00000000000..6ad1c1bb33a --- /dev/null +++ b/e2e/debug-class-loader/bootstrap.php @@ -0,0 +1,8 @@ +autoloadSourceLocator->locateIdentifier($reflector, $identifier); if ($reflection !== null) { return $reflection; diff --git a/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php b/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php index 0a5a61c3793..0843d2f1b2e 100644 --- a/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php +++ b/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php @@ -3,7 +3,6 @@ namespace PHPStan\Reflection\BetterReflection\SourceLocator; use Override; -use ParseError; use PhpParser\Node\Arg; use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Name; @@ -23,6 +22,7 @@ use PHPStan\Type\ConstantTypeHelper; use ReflectionClass; use ReflectionFunction; +use Throwable; use function array_key_exists; use function array_keys; use function class_exists; @@ -346,10 +346,15 @@ static function () use ($className): ?array { 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 + } catch (Throwable) { + // Two unrelated throws land here, and neither one means the class was not found. + // The trap can serve a parse error instead of the empty script, see + // FileReadTrapStreamWrapper::stream_read(). And asking every registered + // autoloader for the class is not the order PHP uses, so an autoloader that + // throws for names outside its own scope throws here for names it is never + // invoked for at runtime. Either exception would abort the analysis of the file + // with an internal error, so both are swallowed - the file the autoloader asked + // for is recorded by the trap before the throw. } /** diff --git a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/AutoloadFunctionsSourceLocatorTest.php b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/AutoloadFunctionsSourceLocatorTest.php new file mode 100644 index 00000000000..272c2aa934d --- /dev/null +++ b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/AutoloadFunctionsSourceLocatorTest.php @@ -0,0 +1,97 @@ +assertFalse(class_exists('ThrowingAutoloaderAlias', false), 'precondition: the alias does not exist yet'); + + $GLOBALS['__phpstanAutoloadFunctions'] = [ + static function (string $class): void { + throw new LogicException('this should not happen'); + }, + static function (string $class): void { + if ($class !== 'ThrowingAutoloaderAlias') { + return; + } + + class_alias(AFoo::class, 'ThrowingAutoloaderAlias'); + }, + ]; + + try { + $locator = $this->createLocator(); + $reflection = $locator->locateIdentifier( + new DefaultReflector($locator), + new Identifier('ThrowingAutoloaderAlias', new IdentifierType(IdentifierType::IDENTIFIER_CLASS)), + ); + + $this->assertNotNull($reflection, 'the class defined by the second autoloader should be located'); + $this->assertSame(AFoo::class, $reflection->getName()); + } finally { + unset($GLOBALS['__phpstanAutoloadFunctions']); + } + } + + /** + * Nothing resolves the name, so the locator declines - the point is that it declines instead of + * letting the autoloader's exception surface as an internal error. + */ + public function testAThrowingAutoloaderMakesTheLocatorDecline(): void + { + $GLOBALS['__phpstanAutoloadFunctions'] = [ + static function (string $class): void { + throw new LogicException('this should not happen'); + }, + ]; + + try { + $locator = $this->createLocator(); + $reflection = $locator->locateIdentifier( + new DefaultReflector($locator), + new Identifier('NeverDefinedByAnyAutoloader', new IdentifierType(IdentifierType::IDENTIFIER_CLASS)), + ); + + $this->assertNull($reflection); + } finally { + unset($GLOBALS['__phpstanAutoloadFunctions']); + } + } + + private function createLocator(): AutoloadFunctionsSourceLocator + { + $container = self::getContainer(); + + return new AutoloadFunctionsSourceLocator( + new AutoloadSourceLocator($container->getByType(FileNodesFetcher::class), false), + new ReflectionClassSourceLocator( + new Locator($container->getService('phpParserDecorator')), + new ReflectionSourceStubber(new Standard()), + ), + ); + } + +} diff --git a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocatorTest.php b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocatorTest.php index 522bc15e224..40d51e5bead 100644 --- a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocatorTest.php +++ b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocatorTest.php @@ -2,6 +2,9 @@ namespace PHPStan\Reflection\BetterReflection\SourceLocator; +use LogicException; +use PHPStan\BetterReflection\Identifier\Identifier; +use PHPStan\BetterReflection\Identifier\IdentifierType; use PHPStan\BetterReflection\Reflection\ReflectionClass; use PHPStan\BetterReflection\Reflector\DefaultReflector; use PHPStan\Reflection\InitializerExprContext; @@ -12,6 +15,8 @@ use TestSingleFileSourceLocator\InCondition; use function array_merge; use function class_alias; +use function spl_autoload_register; +use function spl_autoload_unregister; function testFunctionForLocator(): void // phpcs:disable { @@ -79,6 +84,39 @@ class_alias(AFoo::class, 'A_Foo'); $this->assertSame(AFoo::class, $class->getName()); } + /** + * This locator asks every registered autoloader for the class, which is not the order PHP uses, + * so an autoloader that throws for names outside its own scope must not abort the analysis. + * + * @see https://github.com/phpstan/phpstan/issues/14976 + */ + public function testThrowingAutoloader(): void + { + // Everything that needs autoloading has to be loaded before the throwing autoloader joins + // the queue - it is registered globally, so anything loaded lazily inside the try would + // throw as well. That includes building the container. + $locator = new AutoloadSourceLocator(self::getContainer()->getByType(FileNodesFetcher::class), true); + + $autoloader = static function (string $class): void { + if ($class !== 'NeverDefinedByAnyAutoloader') { + return; + } + + throw new LogicException('this should not happen'); + }; + spl_autoload_register($autoloader); + + try { + $reflection = $locator->locateIdentifier( + new DefaultReflector($locator), + new Identifier('NeverDefinedByAnyAutoloader', new IdentifierType(IdentifierType::IDENTIFIER_CLASS)), + ); + $this->assertNull($reflection); + } finally { + spl_autoload_unregister($autoloader); + } + } + public static function getAdditionalConfigFiles(): array { return array_merge(