From 2a99875be191f736cb00dda605a576ee8739b090 Mon Sep 17 00:00:00 2001 From: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Date: Thu, 9 Apr 2026 22:11:27 +0000 Subject: [PATCH 1/2] Fix false positive with array_key_exists after by-reference assignment from ArrayDimFetch - When creating a reference from an ArrayDimFetch (e.g. $ref = &$arr['key']), set up IntertwinedVariableByReferenceWithExpr so writes through $ref propagate back to $arr - Added propagateRefTypeToArrayDimFetch() to handle constant-dim ArrayDimFetch targets using setOffsetValueType instead of HasOffsetValueType intersection which would produce never type for incompatible constant array shapes - New regression test in tests/PHPStan/Rules/Comparison/data/bug-14449.php --- src/Analyser/ExprHandler/AssignHandler.php | 25 +++++ src/Analyser/MutatingScope.php | 102 +++++++++++++++++- ...mpossibleCheckTypeFunctionCallRuleTest.php | 6 ++ .../Rules/Comparison/data/bug-14449.php | 36 +++++++ 4 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-14449.php diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 51995ac6657..ae7f7d61ad0 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -155,6 +155,31 @@ static function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $contex ); $scope = $result->getScope(); + if ( + $expr instanceof AssignRef + && $expr->var instanceof Variable + && is_string($expr->var->name) + && $expr->expr instanceof ArrayDimFetch + ) { + $rootExpr = $expr->expr; + while ($rootExpr instanceof ArrayDimFetch) { + $rootExpr = $rootExpr->var; + } + + if ($rootExpr instanceof Variable && is_string($rootExpr->name)) { + $varName = $expr->var->name; + $type = $scope->getType($expr->var); + $nativeType = $scope->getNativeType($expr->var); + + // When $varName is assigned, update the ArrayDimFetch expression + $scope = $scope->assignExpression( + new IntertwinedVariableByReferenceWithExpr($varName, $expr->expr, new Variable($varName)), + $type, + $nativeType, + ); + } + } + if ( $expr instanceof AssignRef && $expr->var instanceof Variable diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index a43ba35dda5..758eb5585a2 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -109,6 +109,7 @@ use function array_map; use function array_merge; use function array_pop; +use function array_reverse; use function array_slice; use function array_unique; use function array_values; @@ -2623,17 +2624,108 @@ public function assignVariable(string $variableName, Type $type, Type $nativeTyp if ($targetRootVar !== null && in_array($targetRootVar, $intertwinedPropagatedFrom, true)) { continue; } - $scope = $scope->assignExpression( - $expressionType->getExpr()->getExpr(), - $scope->getType($expressionType->getExpr()->getAssignedExpr()), - $scope->getNativeType($expressionType->getExpr()->getAssignedExpr()), - ); + $targetExpr = $expressionType->getExpr()->getExpr(); + $newType = $scope->getType($expressionType->getExpr()->getAssignedExpr()); + $newNativeType = $scope->getNativeType($expressionType->getExpr()->getAssignedExpr()); + + if ($targetExpr instanceof Expr\ArrayDimFetch && $targetRootVar !== null && $this->hasConstantDimInChain($targetExpr)) { + $scope = $this->propagateRefTypeToArrayDimFetch($scope, $targetExpr, $newType, $newNativeType, $intertwinedPropagatedFrom, $variableName); + } else { + $scope = $scope->assignExpression( + $targetExpr, + $newType, + $newNativeType, + ); + } } } return $scope; } + private function hasConstantDimInChain(Expr\ArrayDimFetch $dimFetch): bool + { + $current = $dimFetch; + while ($current instanceof Expr\ArrayDimFetch) { + if ($current->dim !== null) { + $dimType = $this->getType($current->dim)->toArrayKey(); + if ($dimType->isConstantScalarValue()->yes()) { + return true; + } + } + $current = $current->var; + } + + return false; + } + + /** + * @param list $intertwinedPropagatedFrom + */ + private function propagateRefTypeToArrayDimFetch( + self $scope, + Expr\ArrayDimFetch $targetExpr, + Type $newType, + Type $newNativeType, + array $intertwinedPropagatedFrom, + string $variableName, + ): self + { + // Collect the chain of ArrayDimFetch from leaf to root + $dimStack = []; + $current = $targetExpr; + while ($current instanceof Expr\ArrayDimFetch) { + $dimStack[] = $current->dim; + $current = $current->var; + } + + // Build intermediate types from root to leaf + $dimStack = array_reverse($dimStack); + $rootType = $scope->getType($current); + $rootNativeType = $scope->getNativeType($current); + + $intermediateTypes = [$rootType]; + $intermediateNativeTypes = [$rootNativeType]; + $currentType = $rootType; + $currentNativeType = $rootNativeType; + for ($i = 0; $i < count($dimStack) - 1; $i++) { + $dim = $dimStack[$i]; + if ($dim === null) { + return $scope->assignExpression($targetExpr, $newType, $newNativeType); + } + $dimType = $scope->getType($dim)->toArrayKey(); + $currentType = $currentType->getOffsetValueType($dimType); + $currentNativeType = $currentNativeType->getOffsetValueType($dimType); + $intermediateTypes[] = $currentType; + $intermediateNativeTypes[] = $currentNativeType; + } + + // Build up from the leaf using setOffsetValueType + $builtType = $newType; + $builtNativeType = $newNativeType; + for ($i = count($dimStack) - 1; $i >= 0; $i--) { + $dim = $dimStack[$i]; + if ($dim === null) { + return $scope->assignExpression($targetExpr, $newType, $newNativeType); + } + $dimType = $scope->getType($dim)->toArrayKey(); + $builtType = $intermediateTypes[$i]->setOffsetValueType($dimType, $builtType); + $builtNativeType = $intermediateNativeTypes[$i]->setOffsetValueType($dimType, $builtNativeType); + } + + if ($current instanceof Variable && is_string($current->name)) { + return $scope->assignVariable( + $current->name, + $builtType, + $builtNativeType, + TrinaryLogic::createYes(), + array_merge($intertwinedPropagatedFrom, [$variableName]), + ); + } + + return $scope->assignExpression($targetExpr, $newType, $newNativeType); + } + private function isDimFetchPathReachable(self $scope, Expr\ArrayDimFetch $dimFetch): bool { if ($dimFetch->dim === null) { diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index cbb3ed25a77..6d41fc1a0a9 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -1226,4 +1226,10 @@ public function testBug12063(): void $this->analyse([__DIR__ . '/data/bug-12063.php'], []); } + public function testBug14449(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-14449.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-14449.php b/tests/PHPStan/Rules/Comparison/data/bug-14449.php new file mode 100644 index 00000000000..5e8450a8338 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-14449.php @@ -0,0 +1,36 @@ + + */ +function DataGenerator() : \Generator +{ + yield [ 'id' => 1 ] ; + yield [ 'id' => 1 ] ; +} + +function () : void { + $results = []; + + $generator = DataGenerator(); + foreach ( $generator as $data ) + { + $id = $data['id']; + if ( !array_key_exists($id, $results ) ) + { + $results[$id] = []; + } + if ( !array_key_exists('data',$results[$id]) ) + { + $results[$id]['data'] = []; + } + + $resultData = &$results[$id]['data']; + if ( !array_key_exists('id', $results[$id]['data']) ) + { + $resultData['id'] = $id; + } + } +}; From 3f3deaa991a5de02bfa6ad16edff88d0c872d69a Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Thu, 9 Apr 2026 23:24:22 +0000 Subject: [PATCH 2/2] Simplify propagateRefTypeToArrayDimFetch to use single loop Replace the two-pass approach (forward pass to collect intermediate types, backward pass to rebuild) with a single while loop that walks from leaf to root, getting each parent's type directly from scope. Also remove unused array_reverse import. Co-Authored-By: Claude Opus 4.6 --- src/Analyser/MutatingScope.php | 55 ++++++++-------------------------- 1 file changed, 13 insertions(+), 42 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 758eb5585a2..b27eef5856d 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -109,7 +109,6 @@ use function array_map; use function array_merge; use function array_pop; -use function array_reverse; use function array_slice; use function array_unique; use function array_values; @@ -2671,53 +2670,25 @@ private function propagateRefTypeToArrayDimFetch( string $variableName, ): self { - // Collect the chain of ArrayDimFetch from leaf to root - $dimStack = []; - $current = $targetExpr; - while ($current instanceof Expr\ArrayDimFetch) { - $dimStack[] = $current->dim; - $current = $current->var; - } - - // Build intermediate types from root to leaf - $dimStack = array_reverse($dimStack); - $rootType = $scope->getType($current); - $rootNativeType = $scope->getNativeType($current); - - $intermediateTypes = [$rootType]; - $intermediateNativeTypes = [$rootNativeType]; - $currentType = $rootType; - $currentNativeType = $rootNativeType; - for ($i = 0; $i < count($dimStack) - 1; $i++) { - $dim = $dimStack[$i]; - if ($dim === null) { - return $scope->assignExpression($targetExpr, $newType, $newNativeType); - } - $dimType = $scope->getType($dim)->toArrayKey(); - $currentType = $currentType->getOffsetValueType($dimType); - $currentNativeType = $currentNativeType->getOffsetValueType($dimType); - $intermediateTypes[] = $currentType; - $intermediateNativeTypes[] = $currentNativeType; - } + $expr = $targetExpr; + $type = $newType; + $nativeType = $newNativeType; - // Build up from the leaf using setOffsetValueType - $builtType = $newType; - $builtNativeType = $newNativeType; - for ($i = count($dimStack) - 1; $i >= 0; $i--) { - $dim = $dimStack[$i]; - if ($dim === null) { + while ($expr instanceof Expr\ArrayDimFetch) { + if ($expr->dim === null) { return $scope->assignExpression($targetExpr, $newType, $newNativeType); } - $dimType = $scope->getType($dim)->toArrayKey(); - $builtType = $intermediateTypes[$i]->setOffsetValueType($dimType, $builtType); - $builtNativeType = $intermediateNativeTypes[$i]->setOffsetValueType($dimType, $builtNativeType); + $dimType = $scope->getType($expr->dim)->toArrayKey(); + $type = $scope->getType($expr->var)->setOffsetValueType($dimType, $type); + $nativeType = $scope->getNativeType($expr->var)->setOffsetValueType($dimType, $nativeType); + $expr = $expr->var; } - if ($current instanceof Variable && is_string($current->name)) { + if ($expr instanceof Variable && is_string($expr->name)) { return $scope->assignVariable( - $current->name, - $builtType, - $builtNativeType, + $expr->name, + $type, + $nativeType, TrinaryLogic::createYes(), array_merge($intertwinedPropagatedFrom, [$variableName]), );