From 80280f6a9360345c8d28997eddd8de90ee43b35a Mon Sep 17 00:00:00 2001 From: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Date: Sun, 14 Jun 2026 19:25:48 +0000 Subject: [PATCH 1/3] Keep subtype-absorbed variables as conditional-expression targets when merging branches - In MutatingScope::createConditionalExpressions, a variable whose our-branch type is a subtype of the other branch's type makes the union absorb it (merged === their). Such a variable was dropped from $newVariableTypes entirely, so no conditional expression was recorded for it. With `mixed` (which absorbs `null`) this meant `if (cond) { $x = null; }` produced no "cond => $x is null" relationship, while `int` (where `int|null` does not collapse) did. - Instead of unsetting such variables, collect them in $guardsToExclude and skip them only during type-guard selection. They are poor guards (asserting their narrowed type wouldn't reliably select the branch) but remain valid conditional *targets*, so the relationship is now preserved. - Updated nsrt tests that become more precise as a result (bug-5051, bug-8467b, non-empty-string-str-containing-fns) and TypesAssignedToPropertiesRuleTest (a dynamic property fetch that was previously *ERROR* now resolves to its real type). - Removed a now-redundant `$parametersAcceptor !== null` check in FuncCallHandler (always true given `$functionReflection !== null`) and added @phpstan-ignore comments in OptimizedDirectorySourceLocator where the sharper narrowing of a single-element `$files` array surfaces a provably-false null check and a foreach value variable that is in fact always defined (matching the existing ignore in the sibling function branch). --- src/Analyser/ExprHandler/FuncCallHandler.php | 1 - src/Analyser/MutatingScope.php | 12 +++- .../OptimizedDirectorySourceLocator.php | 4 +- tests/PHPStan/Analyser/nsrt/bug-5051.php | 2 +- tests/PHPStan/Analyser/nsrt/bug-7948.php | 71 +++++++++++++++++++ tests/PHPStan/Analyser/nsrt/bug-8467b.php | 2 +- .../non-empty-string-str-containing-fns.php | 4 +- .../TypesAssignedToPropertiesRuleTest.php | 4 -- 8 files changed, 88 insertions(+), 12 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-7948.php diff --git a/src/Analyser/ExprHandler/FuncCallHandler.php b/src/Analyser/ExprHandler/FuncCallHandler.php index d28a3225dee..ac8a0a80058 100644 --- a/src/Analyser/ExprHandler/FuncCallHandler.php +++ b/src/Analyser/ExprHandler/FuncCallHandler.php @@ -336,7 +336,6 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex if ( $functionReflection !== null && $this->rememberPossiblyImpureFunctionValues - && $parametersAcceptor !== null && $functionReflection->hasSideEffects()->maybe() && !$functionReflection->isBuiltin() ) { diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index c81d35e98fd..e02c18b0da3 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3787,6 +3787,13 @@ private function createConditionalExpressions( ): array { $newVariableTypes = $ourExpressionTypes; + + // When our-branch type is a subtype of their-branch type, the union + // absorbs it (merged === their). Such a variable is a poor *guard* — + // asserting its our-branch type later wouldn't reliably select this + // branch — but it remains a valid conditional *target*, so only exclude + // it from guard selection instead of dropping it entirely. + $guardsToExclude = []; foreach ($theirExpressionTypes as $exprString => $holder) { if (!array_key_exists($exprString, $mergedExpressionTypes)) { continue; @@ -3804,7 +3811,7 @@ private function createConditionalExpressions( continue; } - unset($newVariableTypes[$exprString]); + $guardsToExclude[$exprString] = true; } $typeGuards = []; @@ -3818,6 +3825,9 @@ private function createConditionalExpressions( if (!$holder->getCertainty()->yes()) { continue; } + if (array_key_exists($exprString, $guardsToExclude)) { + continue; + } if ( array_key_exists($exprString, $theirExpressionTypes) diff --git a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php index f2b0f7e3a6b..21f5bebdf22 100644 --- a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php +++ b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php @@ -123,11 +123,11 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier): $fetchedClassNode = current($fetchedClassNodes[$identifierName]); } - if ($fetchedClassNode === null) { + if ($fetchedClassNode === null) { // @phpstan-ignore identical.alwaysFalse return null; } - [$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($file, $identifier); + [$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($file, $identifier); // @phpstan-ignore variable.undefined $classReflection = $this->nodeToReflection($reflector, $fetchedClassNode); $this->cache->save($reflectionCacheKey, $variableCacheKey, $classReflection->exportToCache()); diff --git a/tests/PHPStan/Analyser/nsrt/bug-5051.php b/tests/PHPStan/Analyser/nsrt/bug-5051.php index 94ffc4711c8..a91a87716bd 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-5051.php +++ b/tests/PHPStan/Analyser/nsrt/bug-5051.php @@ -49,7 +49,7 @@ public function testWithBooleans($data): void if ($update) { assertType('10', $data); - assertType('bool', $foo); + assertType('true', $foo); } else { assertType('1|2|3', $data); assertType('bool', $foo); diff --git a/tests/PHPStan/Analyser/nsrt/bug-7948.php b/tests/PHPStan/Analyser/nsrt/bug-7948.php new file mode 100644 index 00000000000..2f8a488afed --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-7948.php @@ -0,0 +1,71 @@ + $name + * @param mixed $value + */ + public function testMixed($name, $value): void + { + if (is_array($name)) { + $value = null; + } + + if (is_array($name)) { + assertType('null', $value); + } + } + + /** + * @param string|array $name + * @param int $value + */ + public function testInt($name, $value): void + { + if (is_array($name)) { + $value = null; + } + + if (is_array($name)) { + assertType('null', $value); + } + } + + /** + * Assigned value (5) is a subtype of the original type (int|string), + * so the merged type absorbs it just like null|mixed does. + * + * @param string|array $name + * @param int|string $value + */ + public function testSubtype($name, $value): void + { + if (is_array($name)) { + $value = 5; + } + + if (is_array($name)) { + assertType('5', $value); + } + } + + /** + * @param string|array $name + * @param mixed $value + */ + public function testMixedNegated($name, $value): void + { + if (!is_array($name)) { + $value = null; + } + + if (!is_array($name)) { + assertType('null', $value); + } + } +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-8467b.php b/tests/PHPStan/Analyser/nsrt/bug-8467b.php index c4971320ffd..5f6e33e8287 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-8467b.php +++ b/tests/PHPStan/Analyser/nsrt/bug-8467b.php @@ -13,7 +13,7 @@ public function foo (?string $cwd, bool $initialClone = false): void { if ($initialClone && isset($origCwd)) { assertType('string', $origCwd); - assertType('string|null', $cwd); // could be null + assertType('null', $cwd); } } diff --git a/tests/PHPStan/Analyser/nsrt/non-empty-string-str-containing-fns.php b/tests/PHPStan/Analyser/nsrt/non-empty-string-str-containing-fns.php index 415f00c5020..19d3199f123 100644 --- a/tests/PHPStan/Analyser/nsrt/non-empty-string-str-containing-fns.php +++ b/tests/PHPStan/Analyser/nsrt/non-empty-string-str-containing-fns.php @@ -112,7 +112,7 @@ public function variants(string $s) { assertType('string', $s); if (strpos($s, ':') === 5) { - assertType('string', $s); // could be non-empty-string + assertType('non-falsy-string', $s); } assertType('string', $s); if (strpos($s, ':') !== 5) { @@ -164,7 +164,7 @@ public function variants(string $s) { assertType('string', $s); if (mb_strpos($s, ':') === 5) { - assertType('string', $s); // could be non-empty-string + assertType('non-falsy-string', $s); } assertType('string', $s); if (mb_strpos($s, ':') !== 5) { diff --git a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php index 6709b487fea..ba95d3af408 100644 --- a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php @@ -172,10 +172,6 @@ public function testTypesAssignedToPropertiesExpressionNames(): void 'Property PropertiesFromArrayIntoObject\Foo::$lall (int) does not accept string.', 69, ], - [ - 'Property PropertiesFromArrayIntoObject\Foo::$foo (string) does not accept float.', - 83, - ], [ 'Property PropertiesFromArrayIntoObject\Foo::$foo (string) does not accept float|int|string.', 97, From d19913dbae88e0dddb76b4c3d405cca21431d1ee Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sun, 14 Jun 2026 21:08:20 +0000 Subject: [PATCH 2/3] Build class reflection inside the loop in OptimizedDirectorySourceLocator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the two @phpstan-ignore comments in the class branch of locateIdentifier() by restructuring instead of suppressing. For a class identifier, $files is always the single-element [$file], which the sharper conditional-expression inference from this PR now resolves to a non-empty array{string}. That made two things visible: - The post-loop `if ($fetchedClassNode === null)` check became dead (identical.alwaysFalse): the loop always runs and always assigns a FetchedNode, so the value is never null. - The post-loop `getCacheKeys($file, ...)` used $file, which PHPStan treats as possibly-undefined after a foreach (variable.undefined). Previously the live `=== null` branch's `!== null` narrowing implicitly re-established $file; once that branch became dead, the masking disappeared and the warning surfaced — it was correct, not a regression. Building the reflection inside the loop and returning from it keeps $file as the live loop variable and drops the dead null-check entirely, so both ignores go away with no behavior change. Co-Authored-By: Claude Opus 4.8 --- .../OptimizedDirectorySourceLocator.php | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php index 21f5bebdf22..5d23eeb6fb9 100644 --- a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php +++ b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php @@ -111,7 +111,6 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier): } if ($identifier->isClass()) { - $fetchedClassNode = null; foreach ($files as $file) { $fetchedClassNodes = $this->fileNodesFetcher->fetchNodes($file)->getClassNodes(); @@ -121,17 +120,13 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier): /** @var FetchedNode $fetchedClassNode */ $fetchedClassNode = current($fetchedClassNodes[$identifierName]); - } - - if ($fetchedClassNode === null) { // @phpstan-ignore identical.alwaysFalse - return null; - } - [$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($file, $identifier); // @phpstan-ignore variable.undefined - $classReflection = $this->nodeToReflection($reflector, $fetchedClassNode); - $this->cache->save($reflectionCacheKey, $variableCacheKey, $classReflection->exportToCache()); + [$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($file, $identifier); + $classReflection = $this->nodeToReflection($reflector, $fetchedClassNode); + $this->cache->save($reflectionCacheKey, $variableCacheKey, $classReflection->exportToCache()); - return $classReflection; + return $classReflection; + } } elseif ($identifier->isFunction()) { $fetchedFunctionNode = null; foreach ($files as $file) { From 65136a323344fb4d5ae08703b596d2e7ec54556a Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sun, 14 Jun 2026 21:54:25 +0000 Subject: [PATCH 3/3] Capture located file in OptimizedDirectorySourceLocator class branch instead of building reflection inside the loop Move the class reflection back out of the foreach loop and drop the now-dead null check, as requested in review. To keep the post-loop `$file` reference defined without an `@phpstan-ignore`, capture it into `$fetchedFile` (initialized before the loop, mirroring `$fetchedClassNode`). Its definedness is then proven by the same non-empty-array narrowing that makes the removed `=== null` check dead, rather than relying on the loop value variable, which PHPStan treats as possibly-undefined after a foreach. Co-Authored-By: Claude Opus 4.8 --- .../OptimizedDirectorySourceLocator.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php index 5d23eeb6fb9..c7edc63cf0f 100644 --- a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php +++ b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php @@ -111,6 +111,8 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier): } if ($identifier->isClass()) { + $fetchedClassNode = null; + $fetchedFile = null; foreach ($files as $file) { $fetchedClassNodes = $this->fileNodesFetcher->fetchNodes($file)->getClassNodes(); @@ -120,13 +122,14 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier): /** @var FetchedNode $fetchedClassNode */ $fetchedClassNode = current($fetchedClassNodes[$identifierName]); + $fetchedFile = $file; + } - [$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($file, $identifier); - $classReflection = $this->nodeToReflection($reflector, $fetchedClassNode); - $this->cache->save($reflectionCacheKey, $variableCacheKey, $classReflection->exportToCache()); + [$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($fetchedFile, $identifier); + $classReflection = $this->nodeToReflection($reflector, $fetchedClassNode); + $this->cache->save($reflectionCacheKey, $variableCacheKey, $classReflection->exportToCache()); - return $classReflection; - } + return $classReflection; } elseif ($identifier->isFunction()) { $fetchedFunctionNode = null; foreach ($files as $file) {