diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 10b1f135eb..a5bfc624a2 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -69,6 +69,7 @@ use function array_slice; use function count; use function in_array; +use function is_int; use function is_string; /** @@ -315,6 +316,10 @@ public function processAssignVar( foreach ($conditionalExpressions as $exprString => $holders) { $scope = $scope->addConditionalExpressions($exprString, $holders); } + + if ($assignedExpr instanceof Expr\Array_) { + $scope = $this->processArrayByRefItems($scope, $var->name, $assignedExpr, new Variable($var->name)); + } } else { $nameExprResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context); $hasYield = $hasYield || $nameExprResult->hasYield(); @@ -936,6 +941,65 @@ private function isImplicitArrayCreation(array $dimFetchStack, Scope $scope): Tr return $scope->hasVariableType($varNode->name)->negate(); } + private function processArrayByRefItems(MutatingScope $scope, string $rootVarName, Expr\Array_ $arrayExpr, Expr $parentExpr): MutatingScope + { + $implicitIndex = 0; + foreach ($arrayExpr->items as $arrayItem) { + if ($arrayItem->key !== null && $implicitIndex !== null) { + $keyValues = $scope->getType($arrayItem->key)->getConstantScalarValues(); + if (count($keyValues) === 1) { + $keyValue = $keyValues[0]; + if (is_int($keyValue) && $keyValue >= $implicitIndex) { + $implicitIndex = $keyValue + 1; + } + } else { + // Non-constant key makes subsequent implicit indices unpredictable, + // so we stop tracking implicit indices for the rest of the array + $implicitIndex = null; + } + } + + if ($arrayItem->key !== null) { + $dimExpr = $arrayItem->key; + } elseif ($implicitIndex !== null) { + $dimExpr = new Node\Scalar\Int_($implicitIndex); + $implicitIndex++; + } else { + continue; + } + + if ($arrayItem->value instanceof Expr\Array_) { + $dimFetchExpr = new ArrayDimFetch($parentExpr, $dimExpr); + $scope = $this->processArrayByRefItems($scope, $rootVarName, $arrayItem->value, $dimFetchExpr); + } + + if (!$arrayItem->byRef || !$arrayItem->value instanceof Variable || !is_string($arrayItem->value->name)) { + continue; + } + + $refVarName = $arrayItem->value->name; + $dimFetchExpr = new ArrayDimFetch($parentExpr, $dimExpr); + $refType = $scope->getType(new Variable($refVarName)); + $refNativeType = $scope->getNativeType(new Variable($refVarName)); + + // When $rootVarName's array key changes, update $refVarName + $scope = $scope->assignExpression( + new IntertwinedVariableByReferenceWithExpr($rootVarName, new Variable($refVarName), $dimFetchExpr), + $refType, + $refNativeType, + ); + + // When $refVarName changes, update $rootVarName's array key + $scope = $scope->assignExpression( + new IntertwinedVariableByReferenceWithExpr($refVarName, $dimFetchExpr, new Variable($refVarName)), + $refType, + $refNativeType, + ); + } + + return $scope; + } + /** * @param list $dimFetchStack * @param list $offsetTypes diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 42fe96957d..4ce247488f 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -2582,7 +2582,7 @@ public function assignVariable(string $variableName, Type $type, Type $nativeTyp $scope->nativeExpressionTypes[$exprString] = new ExpressionTypeHolder($node, $nativeType, $certainty); } - foreach ($scope->expressionTypes as $expressionType) { + foreach ($scope->expressionTypes as $exprString => $expressionType) { if (!$expressionType->getExpr() instanceof IntertwinedVariableByReferenceWithExpr) { continue; } @@ -2593,6 +2593,16 @@ public function assignVariable(string $variableName, Type $type, Type $nativeTyp continue; } + $assignedExpr = $expressionType->getExpr()->getAssignedExpr(); + if ( + $assignedExpr instanceof Expr\ArrayDimFetch + && !$this->isDimFetchPathReachable($scope, $assignedExpr) + ) { + unset($scope->expressionTypes[$exprString]); + unset($scope->nativeExpressionTypes[$exprString]); + continue; + } + $has = $scope->hasExpressionType($expressionType->getExpr()->getExpr()); if ( $expressionType->getExpr()->getExpr() instanceof Variable @@ -2611,18 +2621,41 @@ public function assignVariable(string $variableName, Type $type, Type $nativeTyp array_merge($intertwinedPropagatedFrom, [$variableName]), ); } else { + $targetRootVar = $this->getIntertwinedRefRootVariableName($expressionType->getExpr()->getExpr()); + 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()), ); } - } return $scope; } + private function isDimFetchPathReachable(self $scope, Expr\ArrayDimFetch $dimFetch): bool + { + if ($dimFetch->dim === null) { + return false; + } + + if (!$dimFetch->var instanceof Expr\ArrayDimFetch) { + return true; + } + + $varType = $scope->getType($dimFetch->var); + $dimType = $scope->getType($dimFetch->dim); + + if (!$varType->hasOffsetValueType($dimType)->yes()) { + return false; + } + + return $this->isDimFetchPathReachable($scope, $dimFetch->var); + } + private function unsetExpression(Expr $expr): self { $scope = $this; @@ -2833,12 +2866,6 @@ public function invalidateExpression(Expr $expressionToInvalidate, bool $require foreach ($expressionTypes as $exprString => $exprTypeHolder) { $exprExpr = $exprTypeHolder->getExpr(); - if ( - $exprExpr instanceof IntertwinedVariableByReferenceWithExpr - && $exprExpr->isVariableToVariableReference() - ) { - continue; - } if (!$this->shouldInvalidateExpression($exprStringToInvalidate, $expressionToInvalidate, $exprExpr, $exprString, $requireMoreCharacters, $invalidatingClass)) { continue; } @@ -2906,8 +2933,32 @@ public function invalidateExpression(Expr $expressionToInvalidate, bool $require ); } + private function getIntertwinedRefRootVariableName(Expr $expr): ?string + { + if ($expr instanceof Variable && is_string($expr->name)) { + return $expr->name; + } + if ($expr instanceof Expr\ArrayDimFetch) { + return $this->getIntertwinedRefRootVariableName($expr->var); + } + return null; + } + private function shouldInvalidateExpression(string $exprStringToInvalidate, Expr $exprToInvalidate, Expr $expr, string $exprString, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null): bool { + if ( + $expr instanceof IntertwinedVariableByReferenceWithExpr + && $exprToInvalidate instanceof Variable + && is_string($exprToInvalidate->name) + && ( + $expr->getVariableName() === $exprToInvalidate->name + || $this->getIntertwinedRefRootVariableName($expr->getExpr()) === $exprToInvalidate->name + || $this->getIntertwinedRefRootVariableName($expr->getAssignedExpr()) === $exprToInvalidate->name + ) + ) { + return false; + } + if ($requireMoreCharacters && $exprStringToInvalidate === $exprString) { return false; } diff --git a/src/Node/Expr/IntertwinedVariableByReferenceWithExpr.php b/src/Node/Expr/IntertwinedVariableByReferenceWithExpr.php index d23d7e4761..2b4358a4a6 100644 --- a/src/Node/Expr/IntertwinedVariableByReferenceWithExpr.php +++ b/src/Node/Expr/IntertwinedVariableByReferenceWithExpr.php @@ -4,9 +4,7 @@ use Override; use PhpParser\Node\Expr; -use PhpParser\Node\Expr\Variable; use PHPStan\Node\VirtualNode; -use function is_string; final class IntertwinedVariableByReferenceWithExpr extends Expr implements VirtualNode { @@ -31,14 +29,6 @@ public function getAssignedExpr(): Expr return $this->assignedExpr; } - public function isVariableToVariableReference(): bool - { - return $this->expr instanceof Variable - && is_string($this->expr->name) - && $this->assignedExpr instanceof Variable - && is_string($this->assignedExpr->name); - } - #[Override] public function getType(): string { diff --git a/tests/PHPStan/Analyser/nsrt/bug-14333.php b/tests/PHPStan/Analyser/nsrt/bug-14333.php new file mode 100644 index 0000000000..d0012fcc6d --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14333.php @@ -0,0 +1,82 @@ + &$a]; + assertType("'hello'", $a); + + $b['key'] = 42; + assertType('42', $a); +} + +function testMultipleByRefInArray(): void +{ + $a = 1; + $c = 'test'; + + $b = [&$a, 'normal', &$c]; + assertType('1', $a); + assertType("'test'", $c); + + $b[0] = 2; + $b[1] = 'foo'; + $b[2] = 'bar'; + + assertType('2', $a); + assertType("'bar'", $c); +} + +function testNonConstantKeyBreaksImplicitIndex(int $key): void +{ + $a = 1; + $c = 'test'; + + $b = [$key => 'x', &$a, &$c]; + assertType('1', $a); + assertType("'test'", $c); + + // Since $key is non-constant, we don't know the implicit indices of &$a and &$c + // so we can't track the reference propagation + $b[0] = 2; + assertType('1', $a); +} + +function testNested(): void +{ + $a = 1; + + $b = [[&$a]]; + assertType('1', $a); + + $b[0][0] = 2; + + assertType('2', $a); + + $b[0] = []; + + assertType('2', $a); + + $b[0][0] = 3; + + assertType('2', $a); +} + +function foo(array &$a): void {} + +function testFunctionCall() { + $b = 1; + + $c = [&$b]; + assertType('array{1}', $c); + + foo($c); + assertType('array', $c); + assertType('mixed', $b); +}