diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 10b1f135eb..a6893b99c0 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -55,6 +55,7 @@ use PHPStan\Type\ConstantTypeHelper; use PHPStan\Type\ErrorType; use PHPStan\Type\IntegerRangeType; +use PHPStan\Type\IntegerType; use PHPStan\Type\MixedType; use PHPStan\Type\ObjectType; use PHPStan\Type\StaticTypeFactory; @@ -69,6 +70,7 @@ use function array_slice; use function count; use function in_array; +use function is_int; use function is_string; /** @@ -315,6 +317,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 +942,67 @@ 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) { + $keyType = $scope->getType($arrayItem->key)->toArrayKey(); + + if ($implicitIndex !== null) { + $keyValues = $keyType->getConstantScalarValues(); + if (count($keyValues) === 1) { + $keyValue = $keyValues[0]; + if (is_int($keyValue) && $keyValue >= $implicitIndex) { + $implicitIndex = $keyValue + 1; + } + } elseif (!$keyType->isInteger()->no()) { + // Key could be an integer, but we don't know which one, + // so subsequent implicit indices are unpredictable + $implicitIndex = null; + } + } + + $dimExpr = $arrayItem->key; + } elseif ($implicitIndex !== null) { + $dimExpr = new Node\Scalar\Int_($implicitIndex); + $implicitIndex++; + } else { + $dimExpr = new TypeExpr(new IntegerType()); + } + + 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/src/Type/Accessory/HasOffsetValueType.php b/src/Type/Accessory/HasOffsetValueType.php index e3b06be521..d7c4d6fe9a 100644 --- a/src/Type/Accessory/HasOffsetValueType.php +++ b/src/Type/Accessory/HasOffsetValueType.php @@ -156,7 +156,7 @@ public function isOffsetAccessLegal(): TrinaryLogic public function hasOffsetValueType(Type $offsetType): TrinaryLogic { - if ($offsetType->isConstantScalarValue()->yes() && $offsetType->equals($this->offsetType)) { + if ($offsetType->isConstantScalarValue()->yes() && $offsetType->toArrayKey()->equals($this->offsetType)) { return TrinaryLogic::createYes(); } @@ -165,7 +165,7 @@ public function hasOffsetValueType(Type $offsetType): TrinaryLogic public function getOffsetValueType(Type $offsetType): Type { - if ($offsetType->isConstantScalarValue()->yes() && $offsetType->equals($this->offsetType)) { + if ($offsetType->isConstantScalarValue()->yes() && $offsetType->toArrayKey()->equals($this->offsetType)) { return $this->valueType; } diff --git a/tests/PHPStan/Analyser/nsrt/bug-14333.php b/tests/PHPStan/Analyser/nsrt/bug-14333.php new file mode 100644 index 0000000000..19268f078e --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14333.php @@ -0,0 +1,165 @@ + &$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 correctly track the reference propagation + $b[2] = 2; + assertType("1|2|'test'|'x'", $a); // Could be 1|2 + assertType("1|2|'test'|'x'", $c); // Could be 'test'|2 +} + +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 testMultipleScalarKeyValues(bool $key): void +{ + $a = 1; + + // $key is true|false, so it maps to int 1 or 0 — two possible scalar values + $b = [$key => &$a]; + assertType('1', $a); + + // $key could be 0 (false) so $b[0] = 2 might update $a through the reference + $b[0] = 2; + assertType('1|2', $a); + + // $key could be 1 (true) so $b[1] = 3 might also update $a + $b[1] = 3; + assertType('1|2|3', $a); +} + +/** @param 'a'|'b' $key */ +function testMultipleScalarKey(string $key): void +{ + $a = 1; + + $b = [$key => 'x', &$a]; + assertType('1', $a); + + // $key has multiple possible values but both are strings, + // so the implicit index for &$a is still 0 + $b[0] = 2; + assertType('2', $a); +} + +/** @param 0|1 $key */ +function testMultipleIntScalarKey(int $key): void +{ + $a = 1; + + $b = [$key => 'x', &$a]; + assertType('1', $a); + + // $key could be 0 or 1, so index could be 1 or 2 — unpredictable + $b[1] = 2; + assertType("1|2|'x'", $a); // Could be 1|2 +} + +function testStringNumericKey(): void +{ + $a = 1; + + // PHP coerces string "2" to int 2 as array key, so next implicit index is 3 + $b = ['2' => 'x', &$a]; + assertType('1', $a); + + $b[3] = 2; + 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); +} + +function moreTest(bool $bool, int $int) { + $a = 1; + $b = 2; + $c = 3; + $d = 4; + $e = 5; + $f = 6; + + $array = [&$a, '2' => &$b, &$c, $int => &$d, &$e, 'key' => &$f]; + + $array[0] = 'a0'; + $array[1] = 'a1'; + $array[2] = 'a2'; + $array[3] = 'a3'; + $array[4] = 'a4'; + $array[5] = 'a5'; + $array['key'] = 'aKey'; + + assertType("'a0'", $a); + assertType("'a2'", $b); + assertType("'a3'", $c); + assertType("1|2|3|4|5|6|'a0'|'a1'|'a2'|'a3'|'a4'|'a5'|'aKey'", $d); + assertType("1|2|3|4|5|6|'a0'|'a1'|'a2'|'a3'|'a4'|'a5'|'aKey'", $e); + assertType("'aKey'", $f); +}