From 14fc334ac2112c40191031ecd4c15b15d96ce5cf Mon Sep 17 00:00:00 2001 From: staabm <120441+staabm@users.noreply.github.com> Date: Mon, 22 Jun 2026 13:18:15 +0000 Subject: [PATCH] Re-apply global state in `ContainerFactory::postInitializeContainer()` even for an already-initialized container - Restrict the `spl_object_id()` early-return guard to only skip the expensive `BetterReflection::populate()` call, which wires up one-time global state in the BetterReflection facade per container. - Always re-apply the remaining mutable global state on every call: `ReflectionProviderStaticAccessor`, `PhpVersionStaticAccessor`, `ObjectType::resetCaches()`, the `typeSpecifier` service, `BleedingEdgeToggle` and `ReportUnsafeArrayStringKeyCastingToggle`. - This fixes flaky tests (GenericObjectTypeTest, IntersectionTypeTest): two test classes sharing the same cached container would short-circuit the guard, so global state mutated by one test (e.g. the bleeding-edge toggle controlling sealed array shapes, or a static ReflectionProvider/PhpVersion selecting the ReflectionClass variance stub) leaked into the next. - Add ContainerFactoryPostInitializeTest covering that returning to an already-initialized container restores the bleeding-edge toggle and the static PhpVersion / ReflectionProvider accessors. --- src/DependencyInjection/ContainerFactory.php | 49 ++++++----- .../ContainerFactoryPostInitializeTest.php | 87 +++++++++++++++++++ 2 files changed, 114 insertions(+), 22 deletions(-) create mode 100644 tests/PHPStan/DependencyInjection/ContainerFactoryPostInitializeTest.php diff --git a/src/DependencyInjection/ContainerFactory.php b/src/DependencyInjection/ContainerFactory.php index 67f8d4ea1aa..9d5c3123686 100644 --- a/src/DependencyInjection/ContainerFactory.php +++ b/src/DependencyInjection/ContainerFactory.php @@ -172,29 +172,34 @@ public function create( public static function postInitializeContainer(Container $container): void { $containerId = spl_object_id($container); - if ($containerId === self::$lastInitializedContainerId) { - return; - } - - self::$lastInitializedContainerId = $containerId; - - /** @var SourceLocator $sourceLocator */ - $sourceLocator = $container->getService('betterReflectionSourceLocator'); - /** @var Reflector $reflector */ - $reflector = $container->getService('betterReflectionReflector'); - - /** @var Parser $phpParser */ - $phpParser = $container->getService('phpParserDecorator'); - - BetterReflection::populate( - $container->getByType(PhpVersion::class)->getVersionId(), - $sourceLocator, - $reflector, - $phpParser, - $container->getByType(PhpStormStubsSourceStubber::class), - $container->getByType(Printer::class), - ); + // Populating BetterReflection wires up global static state in the BetterReflection + // facade, so it only needs to run once per container. The remaining global state below + // (static accessors, caches, feature toggles) can be mutated by other containers, + // tests or data providers in the meantime, so it must be re-applied on every call - + // even when returning to a container that was already populated. Otherwise unrelated + // tests sharing the same container leak global state into each other. + if ($containerId !== self::$lastInitializedContainerId) { + self::$lastInitializedContainerId = $containerId; + + /** @var SourceLocator $sourceLocator */ + $sourceLocator = $container->getService('betterReflectionSourceLocator'); + + /** @var Reflector $reflector */ + $reflector = $container->getService('betterReflectionReflector'); + + /** @var Parser $phpParser */ + $phpParser = $container->getService('phpParserDecorator'); + + BetterReflection::populate( + $container->getByType(PhpVersion::class)->getVersionId(), + $sourceLocator, + $reflector, + $phpParser, + $container->getByType(PhpStormStubsSourceStubber::class), + $container->getByType(Printer::class), + ); + } ReflectionProviderStaticAccessor::registerInstance($container->getByType(ReflectionProvider::class)); PhpVersionStaticAccessor::registerInstance($container->getByType(PhpVersion::class)); diff --git a/tests/PHPStan/DependencyInjection/ContainerFactoryPostInitializeTest.php b/tests/PHPStan/DependencyInjection/ContainerFactoryPostInitializeTest.php new file mode 100644 index 00000000000..a3494bef638 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/ContainerFactoryPostInitializeTest.php @@ -0,0 +1,87 @@ +bleedingEdgeBackup = BleedingEdgeToggle::isBleedingEdge(); + $this->phpVersionBackup = PhpVersionStaticAccessor::getInstance(); + $this->reflectionProviderBackup = ReflectionProviderStaticAccessor::getInstance(); + } + + #[Override] + protected function tearDown(): void + { + BleedingEdgeToggle::setBleedingEdge($this->bleedingEdgeBackup); + PhpVersionStaticAccessor::registerInstance($this->phpVersionBackup); + ReflectionProviderStaticAccessor::registerInstance($this->reflectionProviderBackup); + } + + public function testReappliesGlobalStateForAlreadyInitializedContainer(): void + { + // A separate container whose ReflectionProvider stands in for state leaked by an + // unrelated test. Building it also makes it the "last initialized" container. + $leakedReflectionProvider = $this->createSeparateReflectionProvider(); + + $container = self::getContainer(); + + // Make this container the last initialized one again, so the final + // postInitializeContainer() call below exercises the early-return guard. + ContainerFactory::postInitializeContainer($container); + + $expectedBleedingEdge = BleedingEdgeToggle::isBleedingEdge(); + $expectedPhpVersion = $container->getByType(PhpVersion::class); + $expectedReflectionProvider = $container->getByType(ReflectionProvider::class); + + self::assertNotSame($expectedReflectionProvider, $leakedReflectionProvider); + + // Simulate another test / data provider leaking global state while this container + // stays the "last initialized" one (e.g. two test classes sharing the same container). + BleedingEdgeToggle::setBleedingEdge(!$expectedBleedingEdge); + PhpVersionStaticAccessor::registerInstance(new PhpVersion(70100)); + ReflectionProviderStaticAccessor::registerInstance($leakedReflectionProvider); + + // Returning to the same container must restore all global state, even though the + // expensive BetterReflection population is skipped for an already-initialized container. + ContainerFactory::postInitializeContainer($container); + + self::assertSame($expectedBleedingEdge, BleedingEdgeToggle::isBleedingEdge()); + self::assertSame($expectedPhpVersion->getVersionId(), PhpVersionStaticAccessor::getInstance()->getVersionId()); + self::assertSame($expectedReflectionProvider, ReflectionProviderStaticAccessor::getInstance()); + } + + private function createSeparateReflectionProvider(): ReflectionProvider + { + $rootDir = __DIR__ . '/../../..'; + $fileHelper = new FileHelper($rootDir); + $rootDir = $fileHelper->normalizePath($rootDir, '/'); + $containerFactory = new ContainerFactory($rootDir); + $tmpDir = sys_get_temp_dir() . '/phpstan-tests'; + $container = $containerFactory->create($tmpDir, [ + $containerFactory->getConfigDirectory() . '/config.level8.neon', + __DIR__ . '/../../../src/Testing/TestCase.neon', + ], []); + + return $container->getByType(ReflectionProvider::class); + } + +}