Skip to content

Commit 14fc334

Browse files
committed
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.
1 parent 302796a commit 14fc334

2 files changed

Lines changed: 114 additions & 22 deletions

File tree

src/DependencyInjection/ContainerFactory.php

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -172,29 +172,34 @@ public function create(
172172
public static function postInitializeContainer(Container $container): void
173173
{
174174
$containerId = spl_object_id($container);
175-
if ($containerId === self::$lastInitializedContainerId) {
176-
return;
177-
}
178-
179-
self::$lastInitializedContainerId = $containerId;
180-
181-
/** @var SourceLocator $sourceLocator */
182-
$sourceLocator = $container->getService('betterReflectionSourceLocator');
183175

184-
/** @var Reflector $reflector */
185-
$reflector = $container->getService('betterReflectionReflector');
186-
187-
/** @var Parser $phpParser */
188-
$phpParser = $container->getService('phpParserDecorator');
189-
190-
BetterReflection::populate(
191-
$container->getByType(PhpVersion::class)->getVersionId(),
192-
$sourceLocator,
193-
$reflector,
194-
$phpParser,
195-
$container->getByType(PhpStormStubsSourceStubber::class),
196-
$container->getByType(Printer::class),
197-
);
176+
// Populating BetterReflection wires up global static state in the BetterReflection
177+
// facade, so it only needs to run once per container. The remaining global state below
178+
// (static accessors, caches, feature toggles) can be mutated by other containers,
179+
// tests or data providers in the meantime, so it must be re-applied on every call -
180+
// even when returning to a container that was already populated. Otherwise unrelated
181+
// tests sharing the same container leak global state into each other.
182+
if ($containerId !== self::$lastInitializedContainerId) {
183+
self::$lastInitializedContainerId = $containerId;
184+
185+
/** @var SourceLocator $sourceLocator */
186+
$sourceLocator = $container->getService('betterReflectionSourceLocator');
187+
188+
/** @var Reflector $reflector */
189+
$reflector = $container->getService('betterReflectionReflector');
190+
191+
/** @var Parser $phpParser */
192+
$phpParser = $container->getService('phpParserDecorator');
193+
194+
BetterReflection::populate(
195+
$container->getByType(PhpVersion::class)->getVersionId(),
196+
$sourceLocator,
197+
$reflector,
198+
$phpParser,
199+
$container->getByType(PhpStormStubsSourceStubber::class),
200+
$container->getByType(Printer::class),
201+
);
202+
}
198203

199204
ReflectionProviderStaticAccessor::registerInstance($container->getByType(ReflectionProvider::class));
200205
PhpVersionStaticAccessor::registerInstance($container->getByType(PhpVersion::class));
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\DependencyInjection;
4+
5+
use Override;
6+
use PHPStan\File\FileHelper;
7+
use PHPStan\Php\PhpVersion;
8+
use PHPStan\Reflection\PhpVersionStaticAccessor;
9+
use PHPStan\Reflection\ReflectionProvider;
10+
use PHPStan\Reflection\ReflectionProviderStaticAccessor;
11+
use PHPStan\Testing\PHPStanTestCase;
12+
use function sys_get_temp_dir;
13+
14+
final class ContainerFactoryPostInitializeTest extends PHPStanTestCase
15+
{
16+
17+
private bool $bleedingEdgeBackup;
18+
19+
private PhpVersion $phpVersionBackup;
20+
21+
private ReflectionProvider $reflectionProviderBackup;
22+
23+
#[Override]
24+
protected function setUp(): void
25+
{
26+
$this->bleedingEdgeBackup = BleedingEdgeToggle::isBleedingEdge();
27+
$this->phpVersionBackup = PhpVersionStaticAccessor::getInstance();
28+
$this->reflectionProviderBackup = ReflectionProviderStaticAccessor::getInstance();
29+
}
30+
31+
#[Override]
32+
protected function tearDown(): void
33+
{
34+
BleedingEdgeToggle::setBleedingEdge($this->bleedingEdgeBackup);
35+
PhpVersionStaticAccessor::registerInstance($this->phpVersionBackup);
36+
ReflectionProviderStaticAccessor::registerInstance($this->reflectionProviderBackup);
37+
}
38+
39+
public function testReappliesGlobalStateForAlreadyInitializedContainer(): void
40+
{
41+
// A separate container whose ReflectionProvider stands in for state leaked by an
42+
// unrelated test. Building it also makes it the "last initialized" container.
43+
$leakedReflectionProvider = $this->createSeparateReflectionProvider();
44+
45+
$container = self::getContainer();
46+
47+
// Make this container the last initialized one again, so the final
48+
// postInitializeContainer() call below exercises the early-return guard.
49+
ContainerFactory::postInitializeContainer($container);
50+
51+
$expectedBleedingEdge = BleedingEdgeToggle::isBleedingEdge();
52+
$expectedPhpVersion = $container->getByType(PhpVersion::class);
53+
$expectedReflectionProvider = $container->getByType(ReflectionProvider::class);
54+
55+
self::assertNotSame($expectedReflectionProvider, $leakedReflectionProvider);
56+
57+
// Simulate another test / data provider leaking global state while this container
58+
// stays the "last initialized" one (e.g. two test classes sharing the same container).
59+
BleedingEdgeToggle::setBleedingEdge(!$expectedBleedingEdge);
60+
PhpVersionStaticAccessor::registerInstance(new PhpVersion(70100));
61+
ReflectionProviderStaticAccessor::registerInstance($leakedReflectionProvider);
62+
63+
// Returning to the same container must restore all global state, even though the
64+
// expensive BetterReflection population is skipped for an already-initialized container.
65+
ContainerFactory::postInitializeContainer($container);
66+
67+
self::assertSame($expectedBleedingEdge, BleedingEdgeToggle::isBleedingEdge());
68+
self::assertSame($expectedPhpVersion->getVersionId(), PhpVersionStaticAccessor::getInstance()->getVersionId());
69+
self::assertSame($expectedReflectionProvider, ReflectionProviderStaticAccessor::getInstance());
70+
}
71+
72+
private function createSeparateReflectionProvider(): ReflectionProvider
73+
{
74+
$rootDir = __DIR__ . '/../../..';
75+
$fileHelper = new FileHelper($rootDir);
76+
$rootDir = $fileHelper->normalizePath($rootDir, '/');
77+
$containerFactory = new ContainerFactory($rootDir);
78+
$tmpDir = sys_get_temp_dir() . '/phpstan-tests';
79+
$container = $containerFactory->create($tmpDir, [
80+
$containerFactory->getConfigDirectory() . '/config.level8.neon',
81+
__DIR__ . '/../../../src/Testing/TestCase.neon',
82+
], []);
83+
84+
return $container->getByType(ReflectionProvider::class);
85+
}
86+
87+
}

0 commit comments

Comments
 (0)