Widen variables aliased by & array items when the array is passed to a call

[phpstan-bot]

Summary

When a variable is put into an array by reference ([&$retry]) and that array is passed to a function, the callee can write into the array slot and the write lands in the caller's variable — copying an array in PHP preserves reference elements. PHPStan kept the variable's narrow type across such a call, so the reporter's code produced a bogus If condition is always false. for a branch that really does execute.

The fix makes NodeScopeResolver::processArgs() treat a by-reference array item passed by value as a possible write into the referenced expression, widening it to the corresponding offset of the parameter type.

Changes

• src/Analyser/NodeScopeResolver.php
  • New pass over the arguments at the end of processArgs(): for every argument that is not unpacked, not passed to a by-reference parameter, and that carries by-reference array slots, each referenced expression is virtual-assigned the union of its current type and the parameter type walked down the slot's offset path (mixed when there is no known parameter).
  • findByRefArrayItemSlots() — resolves the slots of an argument, either from an array literal written at the call site or from an array variable that was built with by-reference items.
  • collectByRefArrayLiteralSlots() — walks an array literal (recursing into nested literals), mirroring AssignHandler::processArrayByRefItems()'s implicit-index bookkeeping so explicit, implicit and string keys all resolve to the right offset.
  • resolveByRefArrayOffsetPath() — turns a recorded $array[0][1] slot expression into the list of offset types to walk.
  • isByRefArrayItemWritable() / processByRefArrayItemsPassedByValue() — the write itself, reusing processVirtualAssign() exactly like the existing by-reference parameter writeback does.
• src/Analyser/MutatingScope.php
  • getByRefArrayItemSlots() exposes the IntertwinedVariableByReferenceWithExpr entries that AssignHandler::processArrayByRefItems() records for $array = [&$v], filtered to the array-slot direction, so the "array variable passed later" case can find its aliases.

Call-like siblings all funnel through processArgs(), so one change covers the whole family. Each was probed with its own failing assertion and is now covered by a test:

• function call, method call, static method call, new, closure call, __invoke, call to an unknown callable
• referenced expression kinds: variable, property fetch, static property fetch, array offset
• array shapes: implicit integer keys, explicit string keys, nested array literals, several by-reference items in one literal, unknown array value type (→ mixed)
• array variable built with by-reference items (including nested) and only then passed

Probed and deliberately not changed:

• Local writes through a by-reference item ($args = [&$v]; $args[0] = true;) already propagate to $v; a test now locks that in so the new code cannot coarsen it.
• Arguments passed to a by-reference parameter keep going through the existing by-reference writeback, which already propagates through the intertwined entries.
• Unpacked arguments (f(...[&$v])) are skipped — PHP does not carry the reference through unpacking, and spread elements cannot be mapped to parameters here.
• Array literals hidden behind a ternary/match arm are still not tracked; the reference never reaches a recognizable argument expression in that case.

Root cause

PHPStan already models by-reference aliasing in two places: AssignHandler records IntertwinedVariableByReferenceWithExpr links for $b = &$a and for $array = [&$v], and processArgs() writes back parameter types for &$param arguments. The missing piece was the third escape route: an array that contains a reference is copied on every by-value pass, but the reference elements survive the copy, so the callee can still write through them. Nothing invalidated or widened the referenced variable at the call, so it kept the type it had before the call — here false, which made the following if ($retry) look always-false.

The fix closes that route at the single place all call-like nodes go through, using the same processVirtualAssign() + parameter-type mechanism the by-reference parameter writeback already uses. The assigned type is unioned with the pre-call type rather than replacing it, because unlike a &$param contract nothing guarantees the callee writes into the slot at all.

Test

• tests/PHPStan/Analyser/nsrt/bug-15116.php — the reporter's playground snippet verbatim; assertType('bool', $retry) after makeCoffee(["cappucino", &$retry, $cupsWanted]). Fails with false before the fix.
• tests/PHPStan/Analyser/nsrt/array-by-ref-item-passed-to-call.php — 13 cases covering the whole family listed above (call kinds, referenced-expression kinds, key/nesting shapes, unknown value type, the array-variable case) plus two guard cases: the local $args[0] = true write must stay true, and a non-reference item ([$retry]) must not widen anything. All 12 widening assertions fail before the fix.

Full test suite and make phpstan are green; self-analysis wall time is unchanged (52.5s vs. 52.8s baseline).

Fixes phpstan/phpstan#15116

[staabm]

Can this be simplified by adjusting pre-existing IntertwinedVariableByReferenceWithExpr logic instead?

[phpstan-bot]

Addressed. Yes — it could be simplified, in both directions the review pointed at.

What changed

1. One place resolves & array items to their slots (1d238a076)

The implicit-index bookkeeping I had added in NodeScopeResolver::collectByRefArrayLiteralSlots() was a copy of AssignHandler::processArrayByRefItems() — the very code that records the IntertwinedVariableByReferenceWithExpr entries. Extracted into src/Analyser/ArrayByRefItemSlots.php, parameterized by the expression the slots are rooted at. AssignHandler now consumes it (−32 lines) and only does the two assignExpression() calls that record the intertwined directions.

That extraction also fixed a latent bug in the pre-existing code: AssignHandler did not handle unpacked items, so [...$arr, &$a] gave &$a a wrongly-counted constant index. New test in bug-14333.php; before the change $b[1] = 'one' narrowed $a to 'one', now it's 1|string.

2. Slot types come from the pre-existing intertwined resolving (499bbe364)

MutatingScope::resolveIntertwinedAssignedType() already resolves a recorded slot expression against a root type — it's what assignVariable() uses to propagate through the intertwined entries. So:

• MutatingScope::getByRefArrayItemSlots() became resolveByRefArrayItemTypes(string $variableName, Type $arrayType), returning the aliased expressions already priced against the passed array type via that helper. NodeScopeResolver::resolveByRefArrayOffsetPath() is gone.
• For a literal written at the call site, rooting the shared walk at a TypeExpr of the parameter type makes ordinary dim-fetch reading resolve the offsets — the manual getOffsetValueType() loop is gone too.

Net: NodeScopeResolver −145 lines, AssignHandler −32, and the only remaining new logic there is "union with the pre-call type, then processVirtualAssign".

Verification

• The two nsrt files still fail on HEAD~2 src and pass now (bug-15116.php: false vs bool), and the new bug-14333 case fails before / passes after.
• make tests: 21108 tests, 95914 assertions, 97 skipped, green.
• make phpstan: no errors. make cs: clean.

Pushed to create-pull-request/patch-x3t2po9.