From 69e2a22468a5c8bb58fad27b6dd8f886518b2030 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 3 Sep 2026 10:32:54 +0200 Subject: [PATCH 1/3] Install the right container before every test and data provider on PHPUnit 9 too PHPStanPHPUnitExtension implements the PHPUnit >= 10 extension interface and is registered through , which PHPUnit 9 rejects outright ("Element 'bootstrap': This element is not expected. Expected is ( extension )"). The "Tests with old PHPUnit" jobs therefore ran without its two subscribers, so nothing made a data provider or a test see its own test class's container - and because ParaTest reuses one worker process for many test files, whatever container the previous file installed stayed installed. That is what flaked IntersectionTypeTest::testIsAcceptedBy on the 8.1 jobs: a bleeding edge test file left BleedingEdgeToggle on, dataIsAcceptedBy() built its ConstantArrayTypes sealed (the constructor reads the toggle), dataGetEnumCases() then called createReflectionProvider() and switched the toggle back off, and every data set expecting Maybe came out No. Reproduced deterministically with paratest --runner WrapperRunner -p 1 -c over a bleeding edge test file followed by IntersectionTypeTest. The same gap made VerbosityLevelTest and 29 other files fail with MissingStaticAccessorInstanceException whenever they were the first file a worker picked up. setUp() now always installs the test class's own container, tearDownAfterClass() restores the base one for the next file's data providers, and the bootstrap installs it for the first file's - which makes $reinitializeContainerBeforeEachTest redundant - and InitContainerBeforeTestSubscriber too, so setUp() is now the one mechanism for tests on every PHPUnit version. InitContainerBeforeDataProviderSubscriber stays: data providers run while the suite is being built, and a single-process PHPUnit >= 10 run calls every class's providers before the first test, so a provider that installs its own container would otherwise leak it into the next class's providers. PHPUnit 9 has no such hook, so a data provider of a test class whose container is not the base one still has to ask for it itself there, the way TypeGetFiniteTypesTest now does. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019isZYNh3Zms6xGKGGXqxgp --- src/Testing/PHPStanTestCase.php | 28 ++++++++----- src/Testing/PHPStanTestCaseTrait.php | 41 ++++++++++++++++++- .../InitContainerBeforeTestSubscriber.php | 32 --------------- .../PHPUnit/PHPStanPHPUnitExtension.php | 3 -- .../BleedingEdgeContainerStateTest.php | 31 ++++++++++++++ tests/PHPStan/Testing/ContainerStateTest.php | 18 ++++++++ .../Type/Accessory/HasPropertyTypeTest.php | 5 --- .../Type/Generic/GenericObjectTypeTest.php | 5 --- tests/PHPStan/Type/ObjectTypeTest.php | 5 --- tests/PHPStan/Type/TypeCombinatorTest.php | 5 --- tests/PHPStan/Type/TypeGetFiniteTypesTest.php | 6 +++ tests/bootstrap.php | 8 ++++ 12 files changed, 120 insertions(+), 67 deletions(-) delete mode 100644 src/Testing/PHPUnit/InitContainerBeforeTestSubscriber.php create mode 100644 tests/PHPStan/Testing/BleedingEdgeContainerStateTest.php create mode 100644 tests/PHPStan/Testing/ContainerStateTest.php diff --git a/src/Testing/PHPStanTestCase.php b/src/Testing/PHPStanTestCase.php index d06c9e3b41a..b80e313e11e 100644 --- a/src/Testing/PHPStanTestCase.php +++ b/src/Testing/PHPStanTestCase.php @@ -22,6 +22,7 @@ use PHPStan\Reflection\ReflectionProvider; use PHPStan\Reflection\ReflectionProvider\DirectReflectionProviderProvider; use PHPStan\Rules\Properties\PropertyReflectionFinder; +use PHPStan\Testing\PHPUnit\ContainerInitializer; use PHPStan\Type\Constant\OversizedArrayBuilder; use PHPStan\Type\ExpressionTypeResolverExtension; use PHPStan\Type\OperatorTypeSpecifyingExtensionRegistry; @@ -43,21 +44,28 @@ abstract class PHPStanTestCase extends TestCase use PHPStanTestCaseTrait; /** - * Re-register the runtime container as the global static reflection provider before - * every test. Enable this in tests that construct Type objects directly and assert - * PHP-version-dependent reflection results, so a foreign PhpVersion leaked by another - * test can't flake them. See https://github.com/phpstan/phpstan/issues/14860 + * Re-register this test class's container as the owner of the global static state + * (the reflection provider, the PhpVersion, the bleeding edge toggle) before every + * test, so a container another test class installed can't flake tests that build + * Type objects directly. See https://github.com/phpstan/phpstan/issues/14860 + * + * This is the only hook that does it, on every PHPUnit version. Data providers + * are covered separately - see PHPStanTestCaseTrait::restoreBaseContainer(). */ - protected bool $reinitializeContainerBeforeEachTest = false; - #[Override] protected function setUp(): void { - if (!$this->reinitializeContainerBeforeEachTest) { - return; - } + ContainerInitializer::initialize(static::class); + } + + #[Override] + public static function tearDownAfterClass(): void + { + parent::tearDownAfterClass(); - self::getContainer(); + // The next test class's data providers run before any hook this test case has + // on PHPUnit 9, in the same ParaTest worker process - see restoreBaseContainer(). + self::restoreBaseContainer(); } public static function getParser(): Parser diff --git a/src/Testing/PHPStanTestCaseTrait.php b/src/Testing/PHPStanTestCaseTrait.php index 14a0e68cc63..8a0e45a0a27 100644 --- a/src/Testing/PHPStanTestCaseTrait.php +++ b/src/Testing/PHPStanTestCaseTrait.php @@ -22,11 +22,43 @@ trait PHPStanTestCaseTrait public static function getContainer(): Container { - $additionalConfigFiles = [__DIR__ . '/TestCase.neon']; + $additionalConfigFiles = [self::getBaseConfigFile()]; foreach (static::getAdditionalConfigFiles() as $configFile) { $additionalConfigFiles[] = $configFile; } - $composerAutoloaderProjectPaths = static::getComposerAutoloaderProjectPaths(); + + return self::getContainerForConfigFiles($additionalConfigFiles, static::getComposerAutoloaderProjectPaths()); + } + + /** + * Installs the global state of the container built from just the base config - + * the bleeding edge toggle, the static reflection provider, the PhpVersion and + * the type caches all live in static properties that the last initialized + * container owns. + * + * PHPStanTestCase::setUp() installs the right container before every test. Data + * providers are called while the test suite is being built, so on PHPUnit >= 10 + * InitContainerBeforeDataProviderSubscriber installs it before each of them. + * PHPUnit 9 - which the "Tests with old PHPUnit" jobs run - rejects that extension + * and has no hook that fires before a data provider, while ParaTest reuses one + * worker process for many test files. A + * class whose container turns on e.g. bleeding edge would therefore leak that + * state into the data providers of the next test file, which build Type objects + * that read the toggle in their constructor. Restoring the base state when a test + * class is done keeps those data providers deterministic on every supported + * PHPUnit version. + */ + public static function restoreBaseContainer(): void + { + self::getContainerForConfigFiles([self::getBaseConfigFile()], []); + } + + /** + * @param string[] $additionalConfigFiles + * @param string[] $composerAutoloaderProjectPaths + */ + private static function getContainerForConfigFiles(array $additionalConfigFiles, array $composerAutoloaderProjectPaths): Container + { $cacheKey = hash('sha256', implode("\n", $additionalConfigFiles) . "\0" . implode("\n", $composerAutoloaderProjectPaths)); if (!isset(self::$containers[$cacheKey])) { @@ -66,6 +98,11 @@ public static function getContainer(): Container return self::$containers[$cacheKey]; } + private static function getBaseConfigFile(): string + { + return __DIR__ . '/TestCase.neon'; + } + /** * @return string[] */ diff --git a/src/Testing/PHPUnit/InitContainerBeforeTestSubscriber.php b/src/Testing/PHPUnit/InitContainerBeforeTestSubscriber.php deleted file mode 100644 index 768d6d3e456..00000000000 --- a/src/Testing/PHPUnit/InitContainerBeforeTestSubscriber.php +++ /dev/null @@ -1,32 +0,0 @@ -test(); - if (!$test->isTestMethod()) { - // skip PHPT tests - return; - } - - $testClassName = $test->className(); - - if (!is_a($testClassName, PHPStanTestCase::class, true)) { - return; - } - - ContainerInitializer::initialize($testClassName); - } - -} diff --git a/src/Testing/PHPUnit/PHPStanPHPUnitExtension.php b/src/Testing/PHPUnit/PHPStanPHPUnitExtension.php index 7a4ec992670..ff52d637b92 100644 --- a/src/Testing/PHPUnit/PHPStanPHPUnitExtension.php +++ b/src/Testing/PHPUnit/PHPStanPHPUnitExtension.php @@ -21,9 +21,6 @@ public function bootstrap( $facade->registerSubscriber( new InitContainerBeforeDataProviderSubscriber(), ); - $facade->registerSubscriber( - new InitContainerBeforeTestSubscriber(), - ); } } diff --git a/tests/PHPStan/Testing/BleedingEdgeContainerStateTest.php b/tests/PHPStan/Testing/BleedingEdgeContainerStateTest.php new file mode 100644 index 00000000000..222abde328a --- /dev/null +++ b/tests/PHPStan/Testing/BleedingEdgeContainerStateTest.php @@ -0,0 +1,31 @@ +assertTrue(BleedingEdgeToggle::isBleedingEdge()); + } + +} diff --git a/tests/PHPStan/Testing/ContainerStateTest.php b/tests/PHPStan/Testing/ContainerStateTest.php new file mode 100644 index 00000000000..d00e071e662 --- /dev/null +++ b/tests/PHPStan/Testing/ContainerStateTest.php @@ -0,0 +1,18 @@ +assertFalse(BleedingEdgeToggle::isBleedingEdge()); + } + +} diff --git a/tests/PHPStan/Type/Accessory/HasPropertyTypeTest.php b/tests/PHPStan/Type/Accessory/HasPropertyTypeTest.php index 5771e7d52da..8219bc5eab3 100644 --- a/tests/PHPStan/Type/Accessory/HasPropertyTypeTest.php +++ b/tests/PHPStan/Type/Accessory/HasPropertyTypeTest.php @@ -23,11 +23,6 @@ class HasPropertyTypeTest extends PHPStanTestCase { - // Pin the runtime container so a foreign PhpVersion leaked by another test - // can't flake the version-dependent Closure data set below. - // See https://github.com/phpstan/phpstan/issues/14860 - protected bool $reinitializeContainerBeforeEachTest = true; - public static function dataIsSuperTypeOf(): array { return [ diff --git a/tests/PHPStan/Type/Generic/GenericObjectTypeTest.php b/tests/PHPStan/Type/Generic/GenericObjectTypeTest.php index faca30db63a..16c80efdc7a 100644 --- a/tests/PHPStan/Type/Generic/GenericObjectTypeTest.php +++ b/tests/PHPStan/Type/Generic/GenericObjectTypeTest.php @@ -32,11 +32,6 @@ class GenericObjectTypeTest extends PHPStanTestCase { - // Pin the runtime container so a foreign PhpVersion leaked by another test - // can't flake the version-dependent data sets (reflected variance of built-in - // generics). See https://github.com/phpstan/phpstan/issues/14860 - protected bool $reinitializeContainerBeforeEachTest = true; - public static function dataIsSuperTypeOf(): array { return [ diff --git a/tests/PHPStan/Type/ObjectTypeTest.php b/tests/PHPStan/Type/ObjectTypeTest.php index 9df34437fcf..0c218d5063b 100644 --- a/tests/PHPStan/Type/ObjectTypeTest.php +++ b/tests/PHPStan/Type/ObjectTypeTest.php @@ -53,11 +53,6 @@ class ObjectTypeTest extends PHPStanTestCase { - // Pin the runtime container so a foreign PhpVersion leaked by another test - // can't flake the version-dependent Closure data sets (dynamic-property - // handling). See https://github.com/phpstan/phpstan/issues/14860 - protected bool $reinitializeContainerBeforeEachTest = true; - public static function dataIsIterable(): array { return [ diff --git a/tests/PHPStan/Type/TypeCombinatorTest.php b/tests/PHPStan/Type/TypeCombinatorTest.php index ad6dfe96a33..c4da5fd93ee 100644 --- a/tests/PHPStan/Type/TypeCombinatorTest.php +++ b/tests/PHPStan/Type/TypeCombinatorTest.php @@ -74,11 +74,6 @@ class TypeCombinatorTest extends PHPStanTestCase { - // Pin the runtime container so a foreign PhpVersion leaked by another test - // can't flake the version-dependent data sets (dynamic-property handling of - // final classes). See https://github.com/phpstan/phpstan/issues/14860 - protected bool $reinitializeContainerBeforeEachTest = true; - public static function dataAddNull(): array { return [ diff --git a/tests/PHPStan/Type/TypeGetFiniteTypesTest.php b/tests/PHPStan/Type/TypeGetFiniteTypesTest.php index e7e0ae2de6e..eb934142f8b 100644 --- a/tests/PHPStan/Type/TypeGetFiniteTypesTest.php +++ b/tests/PHPStan/Type/TypeGetFiniteTypesTest.php @@ -17,6 +17,12 @@ class TypeGetFiniteTypesTest extends PHPStanTestCase public static function dataGetFiniteTypes(): iterable { + // The expected ConstantArrayTypes below read the bleeding edge toggle in their + // constructor, so this class's container has to be the installed one already + // while they are built. On PHPUnit 9 nothing installs it before a data provider + // runs - see PHPStanTestCaseTrait::restoreBaseContainer(). + self::getContainer(); + yield [ IntegerRangeType::fromInterval(0, 5), [ diff --git a/tests/bootstrap.php b/tests/bootstrap.php index aeb8edcb42e..8f777c18c12 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -1,5 +1,6 @@ Date: Thu, 3 Sep 2026 10:33:12 +0200 Subject: [PATCH 2/3] Do not identify the last initialized container by its spl_object_id() An id is recycled as soon as its object is freed, so a container created after an earlier one was destroyed can inherit that one's id - and postInitializeContainer() would then return early and leave the static reflection provider, the PhpVersion and the feature toggles pointing at the container that is gone. Hold a WeakReference and compare identities instead. ExprHandlerRegistry and StmtHandlerRegistry memoize handler instances under the same recycled id, and a handler carries its own container's services and parameters, so both memos are dropped whenever another container takes over. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019isZYNh3Zms6xGKGGXqxgp --- src/Analyser/ExprHandlerRegistry.php | 12 ++++++++++ src/Analyser/StmtHandlerRegistry.php | 9 ++++++++ src/DependencyInjection/ContainerFactory.php | 23 +++++++++++++++----- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/Analyser/ExprHandlerRegistry.php b/src/Analyser/ExprHandlerRegistry.php index e04dcac8cbc..66709a33960 100644 --- a/src/Analyser/ExprHandlerRegistry.php +++ b/src/Analyser/ExprHandlerRegistry.php @@ -18,6 +18,18 @@ final class ExprHandlerRegistry /** @var array|false>> */ private static array $exprHandlersByClass = []; + /** + * Handlers belong to the container they came from, and spl_object_id() is + * recycled as soon as a container is freed - so ContainerFactory drops the + * memo whenever another container takes over the global state. + * + * @internal + */ + public static function clearCache(): void + { + self::$exprHandlersByClass = []; + } + /** * @return ExprHandler|null */ diff --git a/src/Analyser/StmtHandlerRegistry.php b/src/Analyser/StmtHandlerRegistry.php index aac7b43c669..793e9f4cfa7 100644 --- a/src/Analyser/StmtHandlerRegistry.php +++ b/src/Analyser/StmtHandlerRegistry.php @@ -23,6 +23,15 @@ final class StmtHandlerRegistry /** @var array, StmtHandler|false>> */ private static array $stmtHandlersByClass = []; + /** + * @see ExprHandlerRegistry::clearCache() + * @internal + */ + public static function clearCache(): void + { + self::$stmtHandlersByClass = []; + } + /** * @return StmtHandler|null */ diff --git a/src/DependencyInjection/ContainerFactory.php b/src/DependencyInjection/ContainerFactory.php index 25bac54af6b..653e493b906 100644 --- a/src/DependencyInjection/ContainerFactory.php +++ b/src/DependencyInjection/ContainerFactory.php @@ -17,6 +17,8 @@ use Nette\Utils\Validators; use Phar; use PhpParser\Parser; +use PHPStan\Analyser\ExprHandlerRegistry; +use PHPStan\Analyser\StmtHandlerRegistry; use PHPStan\BetterReflection\BetterReflection; use PHPStan\BetterReflection\Reflector\Reflector; use PHPStan\BetterReflection\SourceLocator\SourceStubber\PhpStormStubsSourceStubber; @@ -32,6 +34,7 @@ use PHPStan\ShouldNotHappenException; use PHPStan\Type\ObjectType; use PHPStan\Type\TypeCombinator; +use WeakReference; use function array_diff_key; use function array_intersect; use function array_key_exists; @@ -49,7 +52,6 @@ use function is_file; use function is_readable; use function is_string; -use function spl_object_id; use function sprintf; use function str_ends_with; use function substr; @@ -66,7 +68,8 @@ final class ContainerFactory private string $configDirectory; - private static ?int $lastInitializedContainerId = null; + /** @var WeakReference|null */ + private static ?WeakReference $lastInitializedContainer = null; private bool $journalContainer = false; @@ -172,12 +175,22 @@ public function create( /** @internal */ public static function postInitializeContainer(Container $container): void { - $containerId = spl_object_id($container); - if ($containerId === self::$lastInitializedContainerId) { + // Comparing spl_object_id()s would be wrong: an id is recycled as soon as its + // container is freed, so a fresh container could inherit the last initialized + // one's id and silently skip installing its own global state. + $lastInitializedContainer = self::$lastInitializedContainer !== null ? self::$lastInitializedContainer->get() : null; + if ($lastInitializedContainer === $container) { return; } - self::$lastInitializedContainerId = $containerId; + self::$lastInitializedContainer = WeakReference::create($container); + + // Both registries memoize handler instances per container, keyed by the + // container's spl_object_id() - a recycled id would otherwise hand this + // container the freed one's handlers, which carry the other container's + // services and parameters. + ExprHandlerRegistry::clearCache(); + StmtHandlerRegistry::clearCache(); /** @var SourceLocator $sourceLocator */ $sourceLocator = $container->getService('betterReflectionSourceLocator'); From 02874ac4b32faf8f9064831e1c5e8dc44909e5a8 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 25 Sep 2026 07:19:43 +0200 Subject: [PATCH 3/3] Pin the closure node in ClosureTypeResolver's object-id keyed cache PHP hands out the spl_object_id() of a freed object again. The parsed file's nodes live for its whole analysis, but a closure node that is built or parsed and dropped mid-file - by a handler desugaring a call, or by an extension or a test walking its own nodes through NodeScopeResolver - frees its id, and the next closure allocated may get it. With an equal context key the cache then answered the freed node's types for an unrelated closure: that is what flaked ExpressionResultTest's `(fn() => exit())();` on the 7.4 and 8.0 jobs, where an earlier data set's `fn() => yield (exit());` left a Generator return type behind and the immediately invoked arrow function read it before walking its own body. Each entry now pins the node it was built for and answers for that very node only. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01MKj5Zgji9eL5MoSuaTEJxF --- .../Helper/ClosureTypeResolver.php | 36 +++++++++++++------ .../PHPStan/Analyser/ExpressionResultTest.php | 36 ++++++++++++++++--- 2 files changed, 57 insertions(+), 15 deletions(-) diff --git a/src/Analyser/ExprHandler/Helper/ClosureTypeResolver.php b/src/Analyser/ExprHandler/Helper/ClosureTypeResolver.php index cb8411fed35..0ae11f6cdfa 100644 --- a/src/Analyser/ExprHandler/Helper/ClosureTypeResolver.php +++ b/src/Analyser/ExprHandler/Helper/ClosureTypeResolver.php @@ -72,12 +72,15 @@ final class ClosureTypeResolver implements PerFileAnalysisResettable * file's analysis ends - the per-file reset releases the Types and * throw/impure points with the rest of the file's result graph. * - * Keyed by the closure node's spl_object_id(). The keys are AST nodes that - * live for the whole file's analysis (the parser cache retains them), so - * ids of live entries never collide; the per-file reset empties the map - * before another file could reuse them. + * Keyed by the closure node's spl_object_id(), which PHP hands out again + * as soon as a node is freed. The parsed file's nodes live for its whole + * analysis, but a closure node that is built or parsed and dropped mid-file + * - by a handler desugaring a call, or by an extension or a test walking + * its own nodes through NodeScopeResolver - frees its id for the next + * allocation. Each entry therefore pins the node it was built for, and + * findCachedTypes() answers for that very node only. * - * @var array> + * @var array}> */ private array $cachedTypes = []; @@ -93,6 +96,19 @@ public function resetFileAnalysisState(): void $this->cachedTypes = []; } + /** + * @return array + */ + private function findCachedTypes(Node\Expr\Closure|ArrowFunction $expr): array + { + $entry = $this->cachedTypes[spl_object_id($expr)] ?? null; + if ($entry === null || $entry['expr'] !== $expr) { + return []; + } + + return $entry['types']; + } + /** * Resolves a closure/arrow function type by walking its body itself. Used by * the paths that have no prior walk to read return/yield types from. A @@ -128,7 +144,7 @@ public function getClosureType( ); } - $cachedTypes = $this->cachedTypes[spl_object_id($expr)] ?? []; + $cachedTypes = $this->findCachedTypes($expr); $cacheKey = $this->closureContextCacheKey($scope, $expr, $callableParameters, $parameters); if (array_key_exists($cacheKey, $cachedTypes)) { return $this->createClosureTypeFromCache($expr, $parameters, $isVariadic, $cachedTypes[$cacheKey]); @@ -387,7 +403,7 @@ public function seedCacheFromClosureWalk(MutatingScope $scope, Node\Expr\Closure [$parameters, , $callableParameters] = $this->buildParametersAndAcceptors($scope, $expr); $phpdocKey = $this->closureContextCacheKey($scope, $expr, $callableParameters, $parameters); - $cachedTypes = $this->cachedTypes[spl_object_id($expr)] ?? []; + $cachedTypes = $this->findCachedTypes($expr); if (!array_key_exists($phpdocKey, $cachedTypes)) { return; } @@ -395,7 +411,7 @@ public function seedCacheFromClosureWalk(MutatingScope $scope, Node\Expr\Closure $promotedScope = $scope->doNotTreatPhpDocTypesAsCertain(); [$promotedParameters, , $promotedCallableParameters] = $this->buildParametersAndAcceptors($promotedScope, $expr); $cachedTypes[$this->closureContextCacheKey($promotedScope, $expr, $promotedCallableParameters, $promotedParameters)] = $cachedTypes[$phpdocKey]; - $this->cachedTypes[spl_object_id($expr)] = $cachedTypes; + $this->cachedTypes[spl_object_id($expr)] = ['expr' => $expr, 'types' => $cachedTypes]; } /** @@ -912,7 +928,7 @@ private function assembleClosureType( $impurePointsForClosureType = array_map(static fn (ImpurePoint $impurePoint) => new SimpleImpurePoint($impurePoint->getIdentifier(), $impurePoint->getDescription(), $impurePoint->isCertain()), $impurePoints); if ($writeCache) { - $cachedTypes = $this->cachedTypes[spl_object_id($expr)] ?? []; + $cachedTypes = $this->findCachedTypes($expr); $cacheKey ??= $this->closureContextCacheKey($scope, $expr, null, $parameters); $cachedTypes[$cacheKey] = [ 'returnType' => $returnType, @@ -921,7 +937,7 @@ private function assembleClosureType( 'invalidateExpressions' => $invalidateExpressions, 'usedVariables' => $usedVariables, ]; - $this->cachedTypes[spl_object_id($expr)] = $cachedTypes; + $this->cachedTypes[spl_object_id($expr)] = ['expr' => $expr, 'types' => $cachedTypes]; } $mustUseReturnValue = TrinaryLogic::createNo(); diff --git a/tests/PHPStan/Analyser/ExpressionResultTest.php b/tests/PHPStan/Analyser/ExpressionResultTest.php index 4da555b362f..c612ddd55eb 100644 --- a/tests/PHPStan/Analyser/ExpressionResultTest.php +++ b/tests/PHPStan/Analyser/ExpressionResultTest.php @@ -2,6 +2,9 @@ namespace PHPStan\Analyser; +use PhpParser\Node\Expr\ArrowFunction; +use PhpParser\Node\Expr\Exit_; +use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Stmt; use PHPStan\Parser\Parser; use PHPStan\ShouldNotHappenException; @@ -193,9 +196,33 @@ public function testIsAlwaysTerminating( if (!$stmts[0] instanceof Stmt\Expression) { throw new ShouldNotHappenException('Expecting code contains a single statement expression, got: ' . get_class($stmts[0])); } - $stmt = $stmts[0]; - $expr = $stmt->expr; + $this->assertSame($expectedIsAlwaysTerminating, $this->processExpressionStatement($stmts[0])->isAlwaysTerminating()); + } + + public function testClosureTypeIsNotReusedForClosureWithRecycledObjectId(): void + { + /** @var Parser $parser */ + $parser = self::getContainer()->getService('currentPhpVersionRichParser'); + /** @var Stmt\Expression $generatorStmt */ + $generatorStmt = $parser->parseString(' yield (exit());')[0]; + $generatorArrowFunction = $generatorStmt->expr; + $this->assertInstanceOf(ArrowFunction::class, $generatorArrowFunction); + $this->assertFalse($this->processExpressionStatement($generatorStmt)->isAlwaysTerminating()); + + // PHP hands the most recently freed object id to the next allocation - drop the + // generator arrow function last, and build the next arrow function right after, + // so it gets the same spl_object_id() unless something still holds the first one + $exit = new Exit_(); + unset($generatorStmt); + unset($generatorArrowFunction); + $arrowFunction = new ArrowFunction(['expr' => $exit], ['isImmediatelyInvokedClosure' => true, 'immediatelyInvokedClosureArgs' => []]); + + $this->assertTrue($this->processExpressionStatement(new Stmt\Expression(new FuncCall($arrowFunction)))->isAlwaysTerminating()); + } + + private function processExpressionStatement(Stmt\Expression $stmt): ExpressionResult + { /** @var NodeScopeResolver $nodeScopeResolver */ $nodeScopeResolver = self::getContainer()->getByType(NodeScopeResolver::class); /** @var ScopeFactory $scopeFactory */ @@ -204,16 +231,15 @@ public function testIsAlwaysTerminating( ->assignVariable('x', new IntegerType(), new IntegerType(), TrinaryLogic::createYes()) ->assignVariable('arr', new ArrayType(new MixedType(), new MixedType()), new ArrayType(new MixedType(), new MixedType()), TrinaryLogic::createYes()); - $result = $nodeScopeResolver->processExprNode( + return $nodeScopeResolver->processExprNode( $stmt, - $expr, + $stmt->expr, $scope, new ExpressionResultStorage(), static function (): void { }, ExpressionContext::createTopLevel(), ); - $this->assertSame($expectedIsAlwaysTerminating, $result->isAlwaysTerminating()); } }