From 3074a3d8346f25e5c2c6dfc67b65857e0ea11e0b Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Wed, 26 Aug 2026 13:56:40 +0000 Subject: [PATCH 1/3] Widen variables aliased by `&` array items when the array is passed to a call * `NodeScopeResolver::processArgs()` now walks every by-value argument for by-reference array slots and virtual-assigns the referenced expression the union of its current type and the matching offset of the parameter type (`mixed` when the callee is unknown). Copying an array preserves reference elements, so a callee writing into such a slot writes into the caller's variable. * Handles the array literal written directly at the call site (`f([&$retry])`, including string keys, mixed implicit/explicit keys and nested array literals) and an array variable built with by-reference items and only then passed (`$args = [&$retry]; f($args);`). * `MutatingScope::getByRefArrayItemSlots()` exposes the `IntertwinedVariableByReferenceWithExpr` entries recorded by `AssignHandler::processArrayByRefItems()` so the variable case can resolve its offset path. * Because the new code lives in `processArgs()`, every call-like construct is covered by the same path: function calls, method calls, static calls, `new`, closure/`__invoke` calls and calls to unknown callables. Referenced expressions may be variables, property fetches, static property fetches or array offsets. Arguments passed to a by-reference parameter are left to the existing by-ref writeback, and unpacked (`...`) arguments are skipped since PHP does not carry the reference through unpacking. * Probed and left alone: `$arr[0] = true` local writes through a by-reference item already propagate correctly, and array literals hidden behind a ternary/match arm are still not tracked (the reference does not reach a recognizable argument expression there). --- src/Analyser/MutatingScope.php | 32 +++ src/Analyser/NodeScopeResolver.php | 199 ++++++++++++++++++ .../nsrt/array-by-ref-item-passed-to-call.php | 144 +++++++++++++ tests/PHPStan/Analyser/nsrt/bug-15116.php | 45 ++++ 4 files changed, 420 insertions(+) create mode 100644 tests/PHPStan/Analyser/nsrt/array-by-ref-item-passed-to-call.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-15116.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 59e7d30a0d..6ffef6277d 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -2880,6 +2880,38 @@ public function isUndefinedExpressionAllowed(Expr $expr): bool return array_key_exists($exprString, $this->currentlyAllowedUndefinedExpressions); } + /** + * By-reference array items (`$array = [&$v]`) currently aliasing a slot of $variableName. + * The array slot keeps aliasing the referenced expression even after the array is copied, + * so a write into the slot is a write into the referenced expression. + * + * @return list pairs of [referenced expression, slot expression] + */ + public function getByRefArrayItemSlots(string $variableName): array + { + $slots = []; + foreach ($this->expressionTypes as $expressionType) { + $expr = $expressionType->getExpr(); + if (!$expr instanceof IntertwinedVariableByReferenceWithExpr) { + continue; + } + if (!$expressionType->getCertainty()->yes()) { + continue; + } + if ($expr->getVariableName() !== $variableName) { + continue; + } + $assignedExpr = $expr->getAssignedExpr(); + if (!$assignedExpr instanceof Expr\ArrayDimFetch) { + continue; + } + + $slots[] = [$expr->getExpr(), $assignedExpr]; + } + + return $slots; + } + /** * @param list $intertwinedPropagatedFrom */ diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 56dc083900..8e2d981ab3 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -76,10 +76,12 @@ use PHPStan\ShouldNotHappenException; use PHPStan\TrinaryLogic; use PHPStan\Type\ClosureType; +use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\FileTypeMapper; use PHPStan\Type\FunctionParameterClosureThisExtension; use PHPStan\Type\FunctionParameterClosureTypeExtension; use PHPStan\Type\FunctionParameterOutTypeExtension; +use PHPStan\Type\IntegerType; use PHPStan\Type\MethodParameterClosureThisExtension; use PHPStan\Type\MethodParameterClosureTypeExtension; use PHPStan\Type\MethodParameterOutTypeExtension; @@ -2326,6 +2328,41 @@ public function processArgs( } } + foreach ($args as $i => $arg) { + if ($arg->unpack) { + // spread elements land on parameters this loop cannot map, and PHP + // does not carry the reference through the unpacking anyway + continue; + } + + $byRefSlots = $this->findByRefArrayItemSlots($scope, $arg->value); + if (count($byRefSlots) === 0) { + continue; + } + + $currentParameter = null; + if ($writebackParameters !== null) { + if (isset($writebackParameters[$i])) { + $currentParameter = $writebackParameters[$i]; + } elseif (count($writebackParameters) > 0 && $writebackAcceptor->isVariadic()) { + $currentParameter = array_last($writebackParameters); + } + } + + if ($currentParameter !== null && $currentParameter->passedByReference()->createsNewVariable()) { + continue; + } + + $scope = $this->processByRefArrayItemsPassedByValue( + $scope, + $storage, + $stmt, + $byRefSlots, + $currentParameter !== null ? $currentParameter->getType() : new MixedType(), + $nodeCallback, + ); + } + // not storing this, it's scope after processing all args return new ArgsResult( $this->expressionResultFactory->create($scope, $scope, $callLike, $hasYield, $isAlwaysTerminating, $throwPoints, $impurePoints), @@ -2612,6 +2649,168 @@ private function getParameterOutExtensionsType(CallLike $callLike, $calleeReflec return null; } + /** + * A `&$v` item in an array literal keeps aliasing $v after the array is copied into + * the callee, so anything the callee writes into that slot lands in $v. The same holds + * for an array variable built with by-reference items and only then passed to the call. + * + * @return list}> pairs of [referenced expression, offset path] + */ + private function findByRefArrayItemSlots(MutatingScope $scope, Expr $argValue): array + { + $slots = []; + if ($argValue instanceof Expr\Array_) { + $this->collectByRefArrayLiteralSlots($scope, $argValue, [], $slots); + + return $slots; + } + + if (!$argValue instanceof Variable || !is_string($argValue->name)) { + return $slots; + } + + foreach ($scope->getByRefArrayItemSlots($argValue->name) as [$referencedExpr, $slotExpr]) { + $offsetPath = $this->resolveByRefArrayOffsetPath($scope, $slotExpr, $argValue->name); + if ($offsetPath === null) { + continue; + } + + $slots[] = [$referencedExpr, $offsetPath]; + } + + return $slots; + } + + /** + * @param list $offsetPath + * @param list}> $slots + * @param-out list}> $slots + */ + private function collectByRefArrayLiteralSlots(MutatingScope $scope, Expr\Array_ $array, array $offsetPath, array &$slots): void + { + $implicitIndex = 0; + foreach ($array->items as $item) { + if ($item->unpack) { + $implicitIndex = null; + continue; + } + + if ($item->key !== null) { + $keyType = $scope->getType($item->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()) { + // the key could be an integer, but we do not know which one, + // so subsequent implicit indices are unpredictable + $implicitIndex = null; + } + } + } elseif ($implicitIndex !== null) { + $keyType = new ConstantIntegerType($implicitIndex); + $implicitIndex++; + } else { + $keyType = new IntegerType(); + } + + $itemOffsetPath = $offsetPath; + $itemOffsetPath[] = $keyType; + + if ($item->value instanceof Expr\Array_) { + $this->collectByRefArrayLiteralSlots($scope, $item->value, $itemOffsetPath, $slots); + continue; + } + + if (!$item->byRef || !$this->isByRefArrayItemWritable($item->value)) { + continue; + } + + $slots[] = [$item->value, $itemOffsetPath]; + } + } + + private function isByRefArrayItemWritable(Expr $expr): bool + { + if ($expr instanceof Variable) { + return is_string($expr->name); + } + + return $expr instanceof PropertyFetch + || $expr instanceof StaticPropertyFetch + || $expr instanceof ArrayDimFetch; + } + + /** + * Offsets of a by-reference array slot expression rooted at $rootVariableName. + * + * @return list|null + */ + private function resolveByRefArrayOffsetPath(MutatingScope $scope, Expr $slotExpr, string $rootVariableName): ?array + { + if ($slotExpr instanceof Variable && $slotExpr->name === $rootVariableName) { + return []; + } + + if ($slotExpr instanceof ArrayDimFetch && $slotExpr->dim !== null) { + $parentPath = $this->resolveByRefArrayOffsetPath($scope, $slotExpr->var, $rootVariableName); + if ($parentPath === null) { + return null; + } + + $parentPath[] = $scope->getType($slotExpr->dim); + + return $parentPath; + } + + return null; + } + + /** + * @param list}> $slots + * @param callable(Node $node, Scope $scope): void $nodeCallback + */ + private function processByRefArrayItemsPassedByValue( + MutatingScope $scope, + ExpressionResultStorage $storage, + Node\Stmt $stmt, + array $slots, + Type $parameterType, + callable $nodeCallback, + ): MutatingScope + { + foreach ($slots as [$referencedExpr, $offsetPath]) { + if ($referencedExpr instanceof Variable && $referencedExpr->name === 'this') { + continue; + } + + $slotType = $parameterType; + foreach ($offsetPath as $offsetType) { + $slotType = $slotType->getOffsetValueType($offsetType); + } + + if ($scope->hasExpressionType($referencedExpr)->yes()) { + // the callee does not have to write into the slot at all + $slotType = TypeCombinator::union($scope->getType($referencedExpr), $slotType); + } + + $scope = $this->processVirtualAssign( + $scope, + $storage, + $stmt, + $referencedExpr, + new TypeExpr($slotType), + $nodeCallback, + )->getScope(); + } + + return $scope; + } + /** * @param callable(Node $node, Scope $scope): void $nodeCallback */ diff --git a/tests/PHPStan/Analyser/nsrt/array-by-ref-item-passed-to-call.php b/tests/PHPStan/Analyser/nsrt/array-by-ref-item-passed-to-call.php new file mode 100644 index 0000000000..05c078fe40 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/array-by-ref-item-passed-to-call.php @@ -0,0 +1,144 @@ +> $a */ +function takesNested(array $a): void {} + +function takesUntypedArray(array $a): void {} + +final class Holder +{ + + public bool $property = false; + + public static bool $staticProperty = false; + + /** @param array{bool} $a */ + public function __construct(array $a = [false]) + { + } + + /** @param array{bool} $a */ + public function method(array $a): void + { + } + + /** @param array{bool} $a */ + public static function staticMethod(array $a): void + { + } + +} + +function funcCall(): void +{ + $retry = false; + takesShape([&$retry]); + assertType('bool', $retry); +} + +function methodCall(Holder $h): void +{ + $retry = false; + $h->method([&$retry]); + assertType('bool', $retry); +} + +function staticCall(): void +{ + $retry = false; + Holder::staticMethod([&$retry]); + assertType('bool', $retry); +} + +function instantiation(): void +{ + $retry = false; + new Holder([&$retry]); + assertType('bool', $retry); +} + +/** @param callable(array{bool}): void $c */ +function closureCall(callable $c): void +{ + $retry = false; + $c([&$retry]); + assertType('bool', $retry); +} + +function stringKey(): void +{ + $retry = false; + takesKeyedShape(['x' => &$retry]); + assertType('bool', $retry); +} + +function nestedArrayLiteral(): void +{ + $retry = false; + takesNested([[&$retry]]); + assertType('bool', $retry); +} + +function propertyByRef(Holder $h): void +{ + if (!$h->property) { + takesShape([&$h->property]); + assertType('bool', $h->property); + } +} + +function staticPropertyByRef(): void +{ + if (!Holder::$staticProperty) { + takesShape([&Holder::$staticProperty]); + assertType('bool', Holder::$staticProperty); + } +} + +function offsetByRef(): void +{ + $arr = ['k' => false]; + takesShape([&$arr['k']]); + assertType('bool', $arr['k']); +} + +function unknownValueType(): void +{ + $retry = false; + takesUntypedArray([&$retry]); + assertType('mixed', $retry); +} + +function arrayVariablePassedToCall(): void +{ + $retry = false; + $args = [&$retry]; + assertType('false', $retry); + takesShape($args); + assertType('bool', $retry); +} + +function localWriteStillPrecise(): void +{ + $retry = false; + $args = [&$retry]; + $args[0] = true; + assertType('true', $retry); +} + +function byRefParameterNotAffected(): void +{ + $retry = false; + takesShape([$retry]); + assertType('false', $retry); +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-15116.php b/tests/PHPStan/Analyser/nsrt/bug-15116.php new file mode 100644 index 0000000000..1b213bf0f8 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-15116.php @@ -0,0 +1,45 @@ + 1) { + // tell the caller that they can retry (they wanted more cups) + $args[1] = true; + } else { + $args[1] = false; + } + } else { + // for all other types of coffee, make all the cups in the same call + $cupsMade = $args[2]; + $args[1] = false; + } + return $cupsMade; + } +} + +function () { + $retry = false; + $cupsWanted = 10; + $cb = new CoffeeBreak(); + $cupsMade = $cb->makeCoffee(["cappucino", &$retry, $cupsWanted]); + assertType('bool', $retry); + if ($retry) { + $cupsRemaining = $cupsWanted - $cupsMade; + echo "still need $cupsRemaining cups of coffee\n"; + } +}; From 1d238a0765eded935bda6e1bb7649e3d189c284e Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Wed, 26 Aug 2026 14:28:21 +0000 Subject: [PATCH 2/3] Resolve `&` array item slots in one shared place The implicit-index bookkeeping that maps an array literal's `&` items to the slots they alias lived in AssignHandler::processArrayByRefItems(); the call-site widening needs exactly the same walk. Extract it into ArrayByRefItemSlots so there is one implementation, parameterized by the expression the slots are rooted at. Along the way the shared walk also handles unpacked items: a spread shifts every subsequent implicit index by an unknown amount, so the following `&` items land on an unknown int key instead of on a wrongly-counted constant one. Co-Authored-By: Claude Opus 5 --- src/Analyser/ArrayByRefItemSlots.php | 92 ++++++++++++++++++++++ src/Analyser/ExprHandler/AssignHandler.php | 41 +--------- tests/PHPStan/Analyser/nsrt/bug-14333.php | 14 ++++ 3 files changed, 110 insertions(+), 37 deletions(-) create mode 100644 src/Analyser/ArrayByRefItemSlots.php diff --git a/src/Analyser/ArrayByRefItemSlots.php b/src/Analyser/ArrayByRefItemSlots.php new file mode 100644 index 0000000000..9ca302ddb0 --- /dev/null +++ b/src/Analyser/ArrayByRefItemSlots.php @@ -0,0 +1,92 @@ + pairs of [referenced expression, slot expression] + */ + public static function resolve(Scope $scope, Expr\Array_ $array, Expr $rootExpr): array + { + $slots = []; + self::collect($scope, $array, $rootExpr, $slots); + + return $slots; + } + + /** + * @param list $slots + * @param-out list $slots + */ + private static function collect(Scope $scope, Expr\Array_ $array, Expr $parentExpr, array &$slots): void + { + $implicitIndex = 0; + foreach ($array->items as $arrayItem) { + if ($arrayItem->unpack) { + // The unpacked array shifts every subsequent implicit index by an + // unknown amount, and its own items cannot be referenced from here. + $implicitIndex = null; + continue; + } + + 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()); + } + + $dimFetchExpr = new ArrayDimFetch($parentExpr, $dimExpr); + + if ($arrayItem->value instanceof Expr\Array_) { + self::collect($scope, $arrayItem->value, $dimFetchExpr, $slots); + continue; + } + + if (!$arrayItem->byRef) { + continue; + } + + $slots[] = [$arrayItem->value, $dimFetchExpr]; + } + } + +} diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 4c52ec6abe..7bc39da9ed 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -21,6 +21,7 @@ use PhpParser\Node\Expr\Variable; use PhpParser\Node\Name; use PhpParser\Node\Stmt; +use PHPStan\Analyser\ArrayByRefItemSlots; use PHPStan\Analyser\AssignTargetWalkMode; use PHPStan\Analyser\ConditionalExpressionHolder; use PHPStan\Analyser\ExpressionContext; @@ -68,7 +69,6 @@ use PHPStan\Type\ConstantTypeHelper; use PHPStan\Type\ErrorType; use PHPStan\Type\IntegerRangeType; -use PHPStan\Type\IntegerType; use PHPStan\Type\MixedType; use PHPStan\Type\NeverType; use PHPStan\Type\NullType; @@ -86,7 +86,6 @@ use function array_slice; use function count; use function in_array; -use function is_int; use function is_string; /** @@ -1695,44 +1694,12 @@ private function isImplicitArrayCreation(array $dimFetchStack, Scope $scope): Tr 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)) { + foreach (ArrayByRefItemSlots::resolve($scope, $arrayExpr, $parentExpr) as [$referencedExpr, $dimFetchExpr]) { + if (!$referencedExpr instanceof Variable || !is_string($referencedExpr->name)) { continue; } - $refVarName = $arrayItem->value->name; - $dimFetchExpr = new ArrayDimFetch($parentExpr, $dimExpr); + $refVarName = $referencedExpr->name; $refType = $scope->getType(new Variable($refVarName)); $refNativeType = $scope->getNativeType(new Variable($refVarName)); diff --git a/tests/PHPStan/Analyser/nsrt/bug-14333.php b/tests/PHPStan/Analyser/nsrt/bug-14333.php index a01178586b..cece092dd8 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-14333.php +++ b/tests/PHPStan/Analyser/nsrt/bug-14333.php @@ -195,3 +195,17 @@ function moreTest(bool $bool, int $int) { assertType("1|2|3|4|5|'a0'|'a1'|'a2'|'a3'|'a4'|'a5'", $e); assertType("'aKey'", $f); } + +/** @param array $arr */ +function testImplicitIndexAfterUnpack(array $arr): void +{ + $a = 1; + + $b = [...$arr, &$a]; + assertType('1', $a); + + // the unpacked array is of unknown length, so the byref slot's index is + // unknown too - a write to any int key might have hit it + $b[1] = 'one'; + assertType('1|string', $a); +} From 499bbe3641d3bb1446a722a55dd06bf9369dd8e5 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Wed, 26 Aug 2026 14:28:21 +0000 Subject: [PATCH 3/3] Resolve by-ref array item types through the intertwined slots processArgs() walked its own offset paths to price a by-reference array item against the parameter type. Both halves of that walk already exist: - MutatingScope::resolveIntertwinedAssignedType() resolves a recorded slot expression against a root type - the same helper assignVariable() uses to propagate through IntertwinedVariableByReferenceWithExpr entries. Scope now exposes resolveByRefArrayItemTypes(), which hands back the aliased expressions already priced against the passed array type, replacing getByRefArrayItemSlots() and the offset-path resolving in NodeScopeResolver. - For an array literal written at the call site, rooting the shared ArrayByRefItemSlots walk at a TypeExpr of the parameter type makes the ordinary dim fetch reading resolve the offsets. Co-Authored-By: Claude Opus 5 --- src/Analyser/MutatingScope.php | 25 ++-- src/Analyser/NodeScopeResolver.php | 180 ++++++++--------------------- 2 files changed, 63 insertions(+), 142 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 6ffef6277d..31e2c263da 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -2881,15 +2881,16 @@ public function isUndefinedExpressionAllowed(Expr $expr): bool } /** - * By-reference array items (`$array = [&$v]`) currently aliasing a slot of $variableName. - * The array slot keeps aliasing the referenced expression even after the array is copied, - * so a write into the slot is a write into the referenced expression. + * Types of the expressions aliased by `&` array items (`$array = [&$v]`) of $variableName, + * resolved from $arrayType as if that was the array's new value. The slot keeps aliasing + * the referenced expression even after the array is copied, so a write into the slot is + * a write into the referenced expression. * - * @return list pairs of [referenced expression, slot expression] + * @return list pairs of [referenced expression, slot type] */ - public function getByRefArrayItemSlots(string $variableName): array + public function resolveByRefArrayItemTypes(string $variableName, Type $arrayType): array { - $slots = []; + $itemTypes = []; foreach ($this->expressionTypes as $expressionType) { $expr = $expressionType->getExpr(); if (!$expr instanceof IntertwinedVariableByReferenceWithExpr) { @@ -2902,14 +2903,20 @@ public function getByRefArrayItemSlots(string $variableName): array continue; } $assignedExpr = $expr->getAssignedExpr(); - if (!$assignedExpr instanceof Expr\ArrayDimFetch) { + if ( + !$assignedExpr instanceof Expr\ArrayDimFetch + || ScopeOps::getIntertwinedRefRootVariableName($assignedExpr) !== $variableName + ) { continue; } - $slots[] = [$expr->getExpr(), $assignedExpr]; + $itemTypes[] = [ + $expr->getExpr(), + $this->resolveIntertwinedAssignedType($this, $arrayType, $assignedExpr, $variableName, false), + ]; } - return $slots; + return $itemTypes; } /** diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 8e2d981ab3..a5b24cf02f 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -76,12 +76,10 @@ use PHPStan\ShouldNotHappenException; use PHPStan\TrinaryLogic; use PHPStan\Type\ClosureType; -use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\FileTypeMapper; use PHPStan\Type\FunctionParameterClosureThisExtension; use PHPStan\Type\FunctionParameterClosureTypeExtension; use PHPStan\Type\FunctionParameterOutTypeExtension; -use PHPStan\Type\IntegerType; use PHPStan\Type\MethodParameterClosureThisExtension; use PHPStan\Type\MethodParameterClosureTypeExtension; use PHPStan\Type\MethodParameterOutTypeExtension; @@ -2335,11 +2333,6 @@ public function processArgs( continue; } - $byRefSlots = $this->findByRefArrayItemSlots($scope, $arg->value); - if (count($byRefSlots) === 0) { - continue; - } - $currentParameter = null; if ($writebackParameters !== null) { if (isset($writebackParameters[$i])) { @@ -2350,6 +2343,7 @@ public function processArgs( } if ($currentParameter !== null && $currentParameter->passedByReference()->createsNewVariable()) { + // the by-reference writeback above already propagates through the slots continue; } @@ -2357,7 +2351,7 @@ public function processArgs( $scope, $storage, $stmt, - $byRefSlots, + $arg->value, $currentParameter !== null ? $currentParameter->getType() : new MixedType(), $nodeCallback, ); @@ -2654,145 +2648,22 @@ private function getParameterOutExtensionsType(CallLike $callLike, $calleeReflec * the callee, so anything the callee writes into that slot lands in $v. The same holds * for an array variable built with by-reference items and only then passed to the call. * - * @return list}> pairs of [referenced expression, offset path] - */ - private function findByRefArrayItemSlots(MutatingScope $scope, Expr $argValue): array - { - $slots = []; - if ($argValue instanceof Expr\Array_) { - $this->collectByRefArrayLiteralSlots($scope, $argValue, [], $slots); - - return $slots; - } - - if (!$argValue instanceof Variable || !is_string($argValue->name)) { - return $slots; - } - - foreach ($scope->getByRefArrayItemSlots($argValue->name) as [$referencedExpr, $slotExpr]) { - $offsetPath = $this->resolveByRefArrayOffsetPath($scope, $slotExpr, $argValue->name); - if ($offsetPath === null) { - continue; - } - - $slots[] = [$referencedExpr, $offsetPath]; - } - - return $slots; - } - - /** - * @param list $offsetPath - * @param list}> $slots - * @param-out list}> $slots - */ - private function collectByRefArrayLiteralSlots(MutatingScope $scope, Expr\Array_ $array, array $offsetPath, array &$slots): void - { - $implicitIndex = 0; - foreach ($array->items as $item) { - if ($item->unpack) { - $implicitIndex = null; - continue; - } - - if ($item->key !== null) { - $keyType = $scope->getType($item->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()) { - // the key could be an integer, but we do not know which one, - // so subsequent implicit indices are unpredictable - $implicitIndex = null; - } - } - } elseif ($implicitIndex !== null) { - $keyType = new ConstantIntegerType($implicitIndex); - $implicitIndex++; - } else { - $keyType = new IntegerType(); - } - - $itemOffsetPath = $offsetPath; - $itemOffsetPath[] = $keyType; - - if ($item->value instanceof Expr\Array_) { - $this->collectByRefArrayLiteralSlots($scope, $item->value, $itemOffsetPath, $slots); - continue; - } - - if (!$item->byRef || !$this->isByRefArrayItemWritable($item->value)) { - continue; - } - - $slots[] = [$item->value, $itemOffsetPath]; - } - } - - private function isByRefArrayItemWritable(Expr $expr): bool - { - if ($expr instanceof Variable) { - return is_string($expr->name); - } - - return $expr instanceof PropertyFetch - || $expr instanceof StaticPropertyFetch - || $expr instanceof ArrayDimFetch; - } - - /** - * Offsets of a by-reference array slot expression rooted at $rootVariableName. - * - * @return list|null - */ - private function resolveByRefArrayOffsetPath(MutatingScope $scope, Expr $slotExpr, string $rootVariableName): ?array - { - if ($slotExpr instanceof Variable && $slotExpr->name === $rootVariableName) { - return []; - } - - if ($slotExpr instanceof ArrayDimFetch && $slotExpr->dim !== null) { - $parentPath = $this->resolveByRefArrayOffsetPath($scope, $slotExpr->var, $rootVariableName); - if ($parentPath === null) { - return null; - } - - $parentPath[] = $scope->getType($slotExpr->dim); - - return $parentPath; - } - - return null; - } - - /** - * @param list}> $slots * @param callable(Node $node, Scope $scope): void $nodeCallback */ private function processByRefArrayItemsPassedByValue( MutatingScope $scope, ExpressionResultStorage $storage, Node\Stmt $stmt, - array $slots, + Expr $argValue, Type $parameterType, callable $nodeCallback, ): MutatingScope { - foreach ($slots as [$referencedExpr, $offsetPath]) { + foreach ($this->resolveByRefArrayItemTypes($scope, $argValue, $parameterType) as [$referencedExpr, $slotType]) { if ($referencedExpr instanceof Variable && $referencedExpr->name === 'this') { continue; } - $slotType = $parameterType; - foreach ($offsetPath as $offsetType) { - $slotType = $slotType->getOffsetValueType($offsetType); - } - if ($scope->hasExpressionType($referencedExpr)->yes()) { // the callee does not have to write into the slot at all $slotType = TypeCombinator::union($scope->getType($referencedExpr), $slotType); @@ -2811,6 +2682,49 @@ private function processByRefArrayItemsPassedByValue( return $scope; } + /** + * By-reference array item slots of $argValue, each with the type the callee can + * write through the reference - the slot read off $parameterType. + * + * @return list pairs of [referenced expression, slot type] + */ + private function resolveByRefArrayItemTypes(MutatingScope $scope, Expr $argValue, Type $parameterType): array + { + if ($argValue instanceof Expr\Array_) { + // rooting the slot expressions at the parameter type resolves the offsets + // through the usual dim fetch reading + $slots = []; + foreach (ArrayByRefItemSlots::resolve($scope, $argValue, new TypeExpr($parameterType)) as [$referencedExpr, $slotExpr]) { + if (!$this->isByRefArrayItemWritable($referencedExpr)) { + continue; + } + + $slots[] = [$referencedExpr, $scope->getType($slotExpr)]; + } + + return $slots; + } + + if ($argValue instanceof Variable && is_string($argValue->name)) { + // the array was built with by-reference items earlier - the slots are + // already recorded in the scope + return $scope->resolveByRefArrayItemTypes($argValue->name, $parameterType); + } + + return []; + } + + private function isByRefArrayItemWritable(Expr $expr): bool + { + if ($expr instanceof Variable) { + return is_string($expr->name); + } + + return $expr instanceof PropertyFetch + || $expr instanceof StaticPropertyFetch + || $expr instanceof ArrayDimFetch; + } + /** * @param callable(Node $node, Scope $scope): void $nodeCallback */