Skip to content

Commit 4db281b

Browse files
committed
Fix loop-carried assignment type flow in loops
1 parent 6f129a3 commit 4db281b

6 files changed

Lines changed: 566 additions & 64 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
6666
- **Inline `@var` cast no longer overrides the variable type on the RHS of the same assignment.** `/** @var array<string, mixed> */ $data = $data->toArray()` previously resolved the RHS `$data` as `array<string, mixed>` instead of its previous type, producing false "method not found" errors. The cast now applies only after the assignment completes.
6767
- **Single generic argument on collections bound to the wrong template parameter.** Writing `Collection<SectionTranslation>` on a class with `@template TKey of array-key` and `@template TValue` assigned the argument to `TKey` (by position), leaving `TValue` unsubstituted. When fewer generic arguments are provided than template parameters and the leading parameters have key-like bounds (`array-key`, `int`, `string`), the arguments are now right-aligned so they bind to the value parameters. Method-level template parameters whose bound parameter was not passed at the call site and defaults to `null` now resolve to `null` instead of remaining as raw template names. Together these fixes mean `Collection<SectionTranslation>::first()` correctly resolves to `SectionTranslation|null`.
6868
- **Nullable return types losing `|null` after template substitution.** When a method declared `@return TValue|null` and `TValue` was substituted with a concrete class through `@extends`, the `|null` component was silently dropped. Hover showed `AdminUser` instead of `AdminUser|null` for calls like `AdminUser::first()`. The docblock type extraction pipeline now preserves nullable unions so that `|null` survives through substitution, hover display, and variable type resolution.
69+
- **Loop-body assignments not visible inside the same loop iteration.** When a variable was initialized as `null` and reassigned later in a loop body, code earlier in the loop (reached on subsequent iterations) only saw `null`. The variable resolution walker now pre-scans the entire loop body for assignments before the positional walk, so the union of all assignments is available at every point inside the loop. Combined with `!== null` narrowing, variables like `$lastPaidEnd` correctly resolve to the assigned class type. Applies to `foreach`, `while`, `for`, and `do-while` loops.
6970

7071
## [0.6.0] - 2026-03-26
7172

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ within the same impact tier.
2424
| # | Item | Impact | Effort |
2525
| --- | ------------------------------------------------------------------------------------------------------------------------------------------ | ------ | ---------- |
2626
| T18 | [Method-level template parameter resolution at call sites](todo/type-inference.md#t18-method-level-template-parameter-resolution-at-call-sites) | Medium | Medium |
27-
| B20 | [Loop-body assignments not visible to null narrowing](todo/bugs.md#b20-loop-body-assignments-not-visible-to-null-narrowing-for-null-initialized-variables) | Low | Medium |
2827
| B21 | [Builder `__call` return type drops chain type for dynamic `where{Column}` calls](todo/bugs.md#b21-builder-__call-return-type-drops-chain-type-for-dynamic-wherecolumn-calls) | Medium | Medium |
2928
| L12 | [`App::make` / `App::makeWith` class-string return type dispatch](todo/laravel.md#l12-appmake--appmakewith-class-string-return-type-dispatch) | Medium | Low |
3029
| | **Release 0.7.0** | | |

docs/todo/bugs.md

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,62 +1,5 @@
11
# PHPantom — Bug Fixes
22

3-
#### B20. Loop-body assignments not visible to null narrowing for null-initialized variables
4-
5-
| | |
6-
|---|---|
7-
| **Impact** | Low |
8-
| **Effort** | Medium |
9-
10-
When a variable is initialized as `null` and reassigned inside a
11-
`foreach` or `while` loop body, the assignment type does not flow
12-
into the variable's resolved type for code inside the same loop
13-
iteration. The variable stays typed as `null` even though it was
14-
assigned a class instance on a previous iteration.
15-
16-
**Reproducer:**
17-
18-
```php
19-
$lastPaidEnd = null;
20-
21-
foreach ($periods as $period) {
22-
if ($lastPaidEnd !== null && $lastPaidEnd->diffInDays($periodStart) > 0) {
23-
// Hover on $lastPaidEnd shows `null` — the Carbon assignment
24-
// from the previous iteration is lost.
25-
}
26-
$lastPaidEnd = $period->ending->startOfDay();
27-
}
28-
```
29-
30-
**Expected:** `$lastPaidEnd` should resolve to `null|Carbon` (the
31-
union of the initial `null` and the loop-body assignment). The
32-
`!== null` narrowing (B17, now fixed) would then strip `null`,
33-
leaving `Carbon` on the right side of `&&`.
34-
35-
**Actual:** The loop-body assignment (`$lastPaidEnd = $period->ending->startOfDay()`)
36-
appears after the cursor position within the same iteration, so
37-
the variable resolution walker never picks it up. The resolved
38-
type is just `null`.
39-
40-
**Root cause:** `walk_statements_for_assignments` walks statements
41-
sequentially and only accumulates assignments that appear before
42-
the cursor offset. Inside a loop body, an assignment that appears
43-
textually after the cursor can still be live on subsequent
44-
iterations. The walker would need a second pass (or a pre-scan of
45-
the loop body) to collect all assignments within the loop and
46-
union them into the variable's type for any position inside the
47-
loop.
48-
49-
**Where to fix:**
50-
- `src/completion/variable/resolution.rs` — in `walk_foreach_statement`
51-
and `walk_while_statement`, pre-scan the loop body for assignments
52-
to the target variable and merge their types into the result set
53-
before walking the body normally. This ensures that even assignments
54-
after the cursor contribute to the union type.
55-
56-
**Discovered in:** B17 QA (sandbox.php cases 1, 2, 8).
57-
58-
---
59-
603
#### B21. Builder `__call` return type drops chain type for dynamic `where{Column}` calls
614

625
| | |

example.php

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,37 @@ public function demo(array $pens): void
567567
}
568568

569569

570+
// ── Loop-Carried Assignment ─────────────────────────────────────────────────
571+
// When a variable is initialized as null and reassigned inside a loop body,
572+
// the assignment from a previous iteration is visible at the top of the loop.
573+
574+
class LoopCarriedAssignmentDemo
575+
{
576+
/** @param list<Pen> $pens */
577+
public function demo(array $pens): void
578+
{
579+
// Pattern: null-init + reassignment after the usage point in the loop.
580+
// On the second iteration, $prev holds the Pen from the prior iteration.
581+
$prev = null;
582+
foreach ($pens as $pen) {
583+
if ($prev !== null) {
584+
$prev->write(); // Pen from previous iteration
585+
}
586+
$prev = $pen;
587+
}
588+
589+
// Same pattern with a while loop
590+
$lastOrder = null;
591+
while ($row = rand(0, 1)) {
592+
if ($lastOrder !== null) {
593+
$lastOrder->getStatusCode(); // Response from previous iteration
594+
}
595+
$lastOrder = new Response(200, 'ok');
596+
}
597+
}
598+
}
599+
600+
570601
// ── Null Coalesce (`??`) Refinement ─────────────────────────────────────────
571602

572603
class NullCoalesceDemo
@@ -5565,6 +5596,28 @@ function runDemoAssertions(): void
55655596
assert(is_array($iBoxPens), 'ScaffoldingPenBox::getPens() must return array');
55665597
assert($iBoxPens[0] instanceof Pen, 'ScaffoldingPenBox::getPens()[0] must be Pen');
55675598

5599+
// ── Loop-carried assignment ─────────────────────────────────────────
5600+
$lcPens = [new Pen('a'), new Pen('b')];
5601+
$lcPrev = null;
5602+
foreach ($lcPens as $lcPen) {
5603+
if ($lcPrev !== null) {
5604+
assert($lcPrev instanceof Pen, 'Loop-carried $lcPrev must be Pen on second iteration');
5605+
}
5606+
$lcPrev = $lcPen;
5607+
}
5608+
assert($lcPrev instanceof Pen, '$lcPrev must be Pen after foreach');
5609+
5610+
$lcLast = null;
5611+
$lcIter = 0;
5612+
while ($lcIter < 2) {
5613+
if ($lcLast !== null) {
5614+
assert($lcLast instanceof Response, 'Loop-carried $lcLast must be Response');
5615+
}
5616+
$lcLast = new Response(200, 'ok');
5617+
$lcIter++;
5618+
}
5619+
assert($lcLast instanceof Response, '$lcLast must be Response after while');
5620+
55685621
// ── Constant type inference ─────────────────────────────────────────
55695622
assert(ConstantTypeDemo::TIMEOUT === 30, 'ConstantTypeDemo::TIMEOUT must be 30');
55705623
assert(ConstantTypeDemo::NAME === 'app', 'ConstantTypeDemo::NAME must be "app"');

src/completion/variable/resolution.rs

Lines changed: 136 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,15 +1252,55 @@ pub(in crate::completion) fn walk_statements_for_assignments<'b>(
12521252
Statement::While(while_stmt) => {
12531253
walk_while_statement(while_stmt, ctx, results);
12541254
}
1255-
Statement::For(for_stmt) => match &for_stmt.body {
1256-
ForBody::Statement(inner) => {
1257-
check_statement_for_assignments(inner, ctx, results, true);
1255+
Statement::For(for_stmt) => {
1256+
// ── Pre-scan for loop-carried assignments ──
1257+
let for_body_span = for_stmt.body.span();
1258+
let for_header_start = for_stmt.r#for.span().start.offset;
1259+
let cursor_inside_for = ctx.cursor_offset >= for_header_start
1260+
&& ctx.cursor_offset <= for_body_span.end.offset;
1261+
if cursor_inside_for {
1262+
match &for_stmt.body {
1263+
ForBody::Statement(inner) => {
1264+
prescan_loop_body_for_assignments(
1265+
std::iter::once(*inner),
1266+
for_body_span.end.offset,
1267+
ctx,
1268+
results,
1269+
);
1270+
}
1271+
ForBody::ColonDelimited(body) => {
1272+
prescan_loop_body_for_assignments(
1273+
body.statements.iter(),
1274+
for_body_span.end.offset,
1275+
ctx,
1276+
results,
1277+
);
1278+
}
1279+
}
12581280
}
1259-
ForBody::ColonDelimited(body) => {
1260-
walk_statements_for_assignments(body.statements.iter(), ctx, results, true);
1281+
match &for_stmt.body {
1282+
ForBody::Statement(inner) => {
1283+
check_statement_for_assignments(inner, ctx, results, true);
1284+
}
1285+
ForBody::ColonDelimited(body) => {
1286+
walk_statements_for_assignments(body.statements.iter(), ctx, results, true);
1287+
}
12611288
}
1262-
},
1289+
}
12631290
Statement::DoWhile(dw) => {
1291+
// ── Pre-scan for loop-carried assignments ──
1292+
let dw_body_span = dw.statement.span();
1293+
let dw_header_start = dw.r#do.span().start.offset;
1294+
let cursor_inside_dw = ctx.cursor_offset >= dw_header_start
1295+
&& ctx.cursor_offset <= dw_body_span.end.offset;
1296+
if cursor_inside_dw {
1297+
prescan_loop_body_for_assignments(
1298+
std::iter::once(dw.statement),
1299+
dw_body_span.end.offset,
1300+
ctx,
1301+
results,
1302+
);
1303+
}
12641304
check_statement_for_assignments(dw.statement, ctx, results, true);
12651305
}
12661306
Statement::Try(try_stmt) => {
@@ -2094,6 +2134,41 @@ fn walk_if_branch_aware<'b>(
20942134
None
20952135
}
20962136

2137+
/// Pre-scan a loop body for assignments to the target variable.
2138+
///
2139+
/// In a loop, an assignment that appears textually *after* the cursor
2140+
/// can still be live on subsequent iterations. This helper walks the
2141+
/// loop body with `cursor_offset` set to `body_end` so that all
2142+
/// assignments are visible, and merges any discovered types into
2143+
/// `results` as conditional. The caller should invoke this **before**
2144+
/// the normal positional walk so that narrowing (e.g. `!== null`)
2145+
/// applied during the normal walk can strip types contributed by the
2146+
/// pre-scan.
2147+
///
2148+
/// Only types that are not already present in `results` are added,
2149+
/// avoiding duplicates with assignments that the normal walk will
2150+
/// also discover.
2151+
fn prescan_loop_body_for_assignments<'b>(
2152+
statements: impl Iterator<Item = &'b Statement<'b>>,
2153+
body_end: u32,
2154+
ctx: &VarResolutionCtx<'_>,
2155+
results: &mut Vec<ResolvedType>,
2156+
) {
2157+
// Only pre-scan when the cursor is actually inside the loop body.
2158+
// The caller is responsible for checking this, but as a safety
2159+
// measure we verify that the cursor is before the body end.
2160+
if ctx.cursor_offset >= body_end {
2161+
return;
2162+
}
2163+
2164+
let prescan_ctx = ctx.with_cursor_offset(body_end);
2165+
let mut prescan_results: Vec<ResolvedType> = Vec::new();
2166+
walk_statements_for_assignments(statements, &prescan_ctx, &mut prescan_results, true);
2167+
2168+
// Merge pre-scanned types into results as conditional additions.
2169+
ResolvedType::extend_unique(results, prescan_results);
2170+
}
2171+
20972172
/// Handle `foreach` statements during variable assignment walking.
20982173
///
20992174
/// Resolves the foreach value/key variable and recurses into the body
@@ -2142,6 +2217,32 @@ fn walk_foreach_statement<'b>(
21422217
super::foreach_resolution::try_resolve_foreach_key_type(foreach, ctx, results, conditional);
21432218
}
21442219

2220+
// ── Pre-scan for loop-carried assignments ──
2221+
// When the cursor is inside the loop body, assignments that appear
2222+
// textually after the cursor are live on subsequent iterations.
2223+
// Pre-scan the body to pick them up as conditional types before the
2224+
// normal positional walk (which skips statements after the cursor).
2225+
if cursor_inside {
2226+
match &foreach.body {
2227+
ForeachBody::Statement(inner) => {
2228+
prescan_loop_body_for_assignments(
2229+
std::iter::once(*inner),
2230+
body_span.end.offset,
2231+
ctx,
2232+
results,
2233+
);
2234+
}
2235+
ForeachBody::ColonDelimited(body) => {
2236+
prescan_loop_body_for_assignments(
2237+
body.statements.iter(),
2238+
body_span.end.offset,
2239+
ctx,
2240+
results,
2241+
);
2242+
}
2243+
}
2244+
}
2245+
21452246
// Always walk the foreach body for variable assignments, even when
21462247
// the cursor is after the foreach. A foreach body may execute zero
21472248
// or more times, so any assignment inside is conditional.
@@ -2178,6 +2279,35 @@ fn walk_while_statement<'b>(
21782279
// `while ($line = fgets($fp))` — register the assignment.
21792280
check_condition_for_assignment(while_stmt.condition, ctx, results);
21802281

2282+
// Determine whether the cursor is inside this while loop for the
2283+
// pre-scan. We check both body variants.
2284+
let while_body_span = while_stmt.body.span();
2285+
let while_header_start = while_stmt.r#while.span().start.offset;
2286+
let cursor_inside_while =
2287+
ctx.cursor_offset >= while_header_start && ctx.cursor_offset <= while_body_span.end.offset;
2288+
2289+
// ── Pre-scan for loop-carried assignments ──
2290+
if cursor_inside_while {
2291+
match &while_stmt.body {
2292+
WhileBody::Statement(inner) => {
2293+
prescan_loop_body_for_assignments(
2294+
std::iter::once(*inner),
2295+
while_body_span.end.offset,
2296+
ctx,
2297+
results,
2298+
);
2299+
}
2300+
WhileBody::ColonDelimited(body) => {
2301+
prescan_loop_body_for_assignments(
2302+
body.statements.iter(),
2303+
while_body_span.end.offset,
2304+
ctx,
2305+
results,
2306+
);
2307+
}
2308+
}
2309+
}
2310+
21812311
match &while_stmt.body {
21822312
WhileBody::Statement(inner) => {
21832313
ResolvedType::apply_narrowing(results, |classes| {

0 commit comments

Comments
 (0)