diff --git a/src/DependencyInjection/ContainerFactory.php b/src/DependencyInjection/ContainerFactory.php index 67f8d4ea1a..9d5c312368 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 0000000000..a3494bef63 --- /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); + } + +}