Fix array shape tracking from keyed assignments in conditional loops

Author: AJenbo

docs/CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **`instanceof` narrowing with unresolvable target class.** When the `instanceof` target class cannot be loaded (e.g. it lives inside a phar archive), the variable's type is now treated as unknown instead of keeping the un-narrowed base type. This eliminates false-positive "unknown member" diagnostics for members that only exist on the narrowed subclass. Applies to all narrowing contexts: if-bodies, ternary expressions, `assert()` statements, and inline `&&` chains.
 - **`DB::select()` return type.** `DB::select()`, `DB::selectFromWriteConnection()`, and `DB::selectResultSets()` now return `array<int, stdClass>` instead of bare `array`, and `DB::selectOne()` returns `?stdClass` instead of `mixed`. Property access on query results (e.g. `$result[0]->column_name`) no longer produces false-positive diagnostics. The same fix applies to the underlying `Illuminate\Database\Connection` class.
 - **Redis `Connection` method resolution.** `Illuminate\Redis\Connections\Connection` now has `@mixin \Redis` applied automatically, so Redis commands like `del()`, `get()`, `set()`, etc. resolve through the phpredis stubs instead of producing false-positive diagnostics.
+- **Array shape tracking from keyed assignments inside conditional branches.** When an array is built incrementally with variable keys inside a loop that contains if/else branching (e.g. `$arr[$id] = ['bundle' => $obj, 'count' => 1]` in an else branch), the shape type is now preserved through foreach iteration. Previously the variable resolution depth limit was too low, causing intermediate variables in the shape value to resolve as `mixed` instead of their actual class type.
 
 ### Added
 

docs/todo.md

@@ -23,7 +23,6 @@ within the same impact tier.
 
 | #   | Item                                                                                                                                                      | Impact | Effort     |
 | --- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ---------- |
-| B13 | [Array shape tracking from keyed literal assignments in loops](todo/bugs.md#b13-array-shape-tracking-from-keyed-literal-assignments-in-loops)             | Low    | High       |
 | H10 | [`return.unusedType` — remove unused type from return union](todo/phpstan-actions.md#h10-returnunusedtype--remove-unused-type-from-return-union)          | Medium | Medium     |
 | H6  | `return.type` — update return type to match actual returns                                                                                                | Medium | Medium     |
 |     | **Release 0.7.0**                                                                                                                                         |        |            |
@@ -90,6 +89,7 @@ unlikely to move the needle for most users.
 | C6   | `#[ExpectedValues]` parameter value suggestions                                                                                                                             | Low         | Medium         |
 | C10  | [Deprecation markers on class-name completions from all sources](todo/completion.md#c10-deprecation-markers-on-class-name-completions-from-all-sources)                     | Low         | Low            |
 |      | **[Type Inference](todo/type-inference.md)**                                                                                                                                |             |                |
+| T25  | [Forward-walking scope model](todo/type-inference.md#t25-forward-walking-scope-model-for-variable-type-resolution) (eliminate backward-scanning depth limit)                | High        | Very High      |
 | T19  | [Structured type representation](todo/type-inference.md#t19-structured-type-representation) (replace string-based types with `PhpType` enum)                                | High        | Very High      |
 | T20  | [Type narrowing reconciliation engine](todo/type-inference.md#t20-type-narrowing-reconciliation-engine) (sure/sureNot tracking, AND/OR algebra)                             | Medium-High | High           |
 | T6   | `Closure::bind()` / `Closure::fromCallable()` return type preservation                                                                                                      | Low-Medium  | Low-Medium     |

docs/todo/bugs.md

@@ -1,33 +1,3 @@
 # PHPantom — Bug Fixes
 
-## B13. Array shape tracking from keyed literal assignments in loops
-**Impact: Low · Effort: High**
-
-Pattern:
-```php
-$bundleProductCounts = [];
-foreach ($items as $item) {
-    $bundleProductCounts[$item->id] = [
-        'bundle' => $item->productBundle,
-        'count'  => 1,
-    ];
-}
-foreach ($bundleProductCounts as $entry) {
-    $entry['bundle']->parentProduct();  // unresolved
-}
-```
-
-PHPantom tracks array value types from variable-key assignments
-(`$arr[$key] = $value`), but when the value is an array literal with
-string keys (a shape), the element type is not preserved as a shape.
-Subsequent access like `$entry['bundle']->method()` requires knowing
-that `'bundle'` maps to a specific class type.
-
-**Observed in:** `ProductSupplyAmountChangeListener:58` — array built
-with `['bundle' => $productBundle, 'count' => 1]` in a loop, then
-iterated; `$bundleProductCount['bundle']->parentProduct()` is
-unresolvable because the shape is lost.
-
-**Depends on:** T19 (structured type representation) or at minimum
-a basic array shape inference that preserves `array{key: Type}` from
-literal array constructors and propagates it through foreach.
+No outstanding items.

docs/todo/type-inference.md

@@ -527,3 +527,80 @@ with `stdClass` property access suppression, but the general gap
 remains for any `$arr[$i] instanceof Foo` pattern.
 
 ---
+
+## T25. Forward-walking scope model for variable type resolution
+**Impact: High · Effort: Very High**
+
+PHPantom resolves variable types lazily: when the user triggers
+completion on `$x->`, it walks backward from the cursor to find
+where `$x` was assigned, then recursively resolves any variables
+referenced in the RHS. Each level of indirection adds a call to
+`resolve_variable_types`, which re-parses the file and re-walks the
+AST. A global depth counter (`MAX_VAR_RESOLUTION_DEPTH`, currently 4)
+caps the recursion to prevent stack overflows.
+
+PHPStan, Psalm, and Mago all use the opposite strategy: an eager,
+single-pass, forward-walking scope model. They walk statements
+top-to-bottom, carrying a mutable type map (`expressionTypes` in
+PHPStan, `locals` in Mago). When they encounter `$a = $b->prop`,
+they look up `$b` in the already-populated map, resolve the property,
+and store `$a`'s type. Variable lookup is a flat O(1) map fetch with
+zero recursion regardless of assignment chain depth.
+
+The backward-scanning approach causes several problems:
+
+- **Depth limit fragility.** Every real-world pattern that adds one
+  more level of indirection (e.g. array shape literals referencing
+  variables assigned from foreach bindings inside conditional
+  branches) requires bumping the limit. The limit was 3, then 4;
+  it will need to grow again.
+- **Redundant work.** Each recursive call re-parses the source and
+  re-walks the AST from the top. A forward pass would parse once and
+  walk once.
+- **No scope threading.** Narrowing, guard clauses, and branch-aware
+  resolution are bolted on as post-hoc corrections rather than
+  flowing naturally through the scope.
+
+**Depth limits eliminated by this item:**
+
+| Constant | Value | Location | Why it exists |
+|---|---|---|---|
+| `MAX_VAR_RESOLUTION_DEPTH` | 4 | `completion/variable/resolution.rs` | Each variable in an assignment chain triggers a full re-parse and re-walk. Was 3, bumped to 4 for array shapes in conditional loop branches. |
+| `MAX_CLOSURE_INFER_DEPTH` | 4 | `completion/variable/closure_resolution.rs` | `infer_callable_params_from_receiver` resolves the receiver type to infer closure parameter types, which can trigger another variable resolution cycle. |
+
+Both share the same root cause: resolving a variable's type triggers
+a full re-parse and re-walk from scratch, which recurses when the RHS
+references another variable. In a forward-walking model, both would
+be flat map lookups with zero recursion.
+
+The remaining depth limits (`MAX_INHERITANCE_DEPTH`,
+`MAX_TRAIT_DEPTH`, `MAX_MIXIN_DEPTH`, `MAX_ALIAS_DEPTH`) guard
+against walking PHP class/type hierarchies and are unrelated to the
+resolution architecture. They mirror limits that PHPStan and Mago
+also have and should stay as-is.
+
+A forward-walking scope model would:
+
+1. Parse the enclosing function body once.
+2. Walk statements in order, maintaining a `HashMap<VarName, PhpType>`
+   scope that is updated on each assignment, foreach binding, catch
+   clause, and narrowing point.
+3. At the cursor position, read the variable's type from the map.
+
+This eliminates the depth limit entirely and makes the resolution
+cost proportional to the number of statements before the cursor,
+not the depth of the assignment chain.
+
+**Depends on:** T19 (structured type representation) should land
+first so the scope map stores `PhpType` values instead of strings.
+
+**Migration path:** Start with a parallel implementation behind a
+feature flag. The existing backward-scanning resolver stays as a
+fallback. Migrate one resolution context at a time (completion,
+hover, diagnostics) once the forward walker covers enough cases.
+
+**Reference:** PHPStan's `MutatingScope.expressionTypes` +
+`NodeScopeResolver.processStmtNode`, Mago's `BlockContext.locals` +
+statement analyzers. Both converge on the same architecture: the
+scope is the single source of truth, populated eagerly as the walk
+progresses.

example.php

@@ -3286,6 +3286,34 @@ public function demo(array $pens): void
     }
 }
 
+class ConditionalLoopShapeDemo
+{
+    /** @param list<Pen> $pens */
+    public function demo(array $pens): void
+    {
+        // Array built with variable keys inside a loop where the assignment
+        // is inside a conditional branch (if/else). The shape type from
+        // the array literal is preserved through foreach iteration.
+        $grouped = [];
+        foreach ($pens as $pen) {
+            $key = $pen->color();
+            if (array_key_exists($key, $grouped)) {
+                $grouped[$key]['count']++;
+            } else {
+                $grouped[$key] = [
+                    'tool'  => $pen,
+                    'count' => 1,
+                ];
+            }
+        }
+
+        // Foreach over the built array resolves shape keys
+        foreach ($grouped as $entry) {
+            $entry['tool']->write();      // Pen method via shape tracking
+        }
+    }
+}
+
 
 // ═══════════════════════════════════════════════════════════════════════════
 // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
@@ -5968,6 +5996,24 @@ function runDemoAssertions(): void
     $labFound = $labIndexed['blue'] ?? null;
     assert($labFound instanceof Pen, 'Null-coalesce on variable-key array must resolve to Pen');
 
+    // ── Conditional loop shape (keyed assignment in if/else) ────────────
+    $shapePens = [new Pen('red'), new Pen('blue'), new Pen('red')];
+    $shapeGrouped = [];
+    foreach ($shapePens as $shapePen) {
+        $shapeKey = $shapePen->color();
+        if (array_key_exists($shapeKey, $shapeGrouped)) {
+            $shapeGrouped[$shapeKey]['count']++;
+        } else {
+            $shapeGrouped[$shapeKey] = [
+                'tool'  => $shapePen,
+                'count' => 1,
+            ];
+        }
+    }
+    foreach ($shapeGrouped as $shapeEntry) {
+        assert($shapeEntry['tool'] instanceof Pen, 'Shape key from conditional loop must resolve to Pen');
+    }
+
     echo "All assertions passed.\n";
 }
 

src/completion/variable/resolution.rs

@@ -69,17 +69,18 @@ thread_local! {
 
 /// Maximum nesting depth for `resolve_variable_types` calls.
 ///
-/// Three levels covers legitimate nested resolution such as:
-///   depth 0: resolve `$item` in a foreach body
-///   depth 1: resolve `$collection` (the foreach iterator expression)
-///   depth 2: resolve `$node` (RHS of `$collection = $node->method()`)
+/// Four levels covers legitimate nested resolution such as:
+///   depth 0: resolve `$arr` built via `$arr[$key] = ['k' => $var]`
+///   depth 1: resolve `$var` (array element variable in the shape literal)
+///   depth 2: resolve `$item->prop` (RHS of `$var = $item->prop`)
+///   depth 3: resolve `$item` (foreach value binding → iterable param)
 ///
 /// Cycles like `foreach ($x->method() as $x)` are caught by a
 /// targeted check in `try_resolve_foreach_value_type` (which skips
 /// resolution when the value variable shadows the iterator receiver)
 /// rather than by this depth limit alone.  The depth limit is a
 /// safety net for any remaining recursive patterns.
-const MAX_VAR_RESOLUTION_DEPTH: u8 = 3;
+const MAX_VAR_RESOLUTION_DEPTH: u8 = 4;
 
 /// Returns `true` when any `resolve_variable_types` call on the
 /// current stack has returned empty because the depth guard fired.

tests/fixtures/foreach/array_shape_from_keyed_loop_assignment.fixture

@@ -0,0 +1,52 @@
+// test: foreach over array built with variable-keyed shape assignments in conditional loop
+// feature: completion
+// Array built inside a loop with if/else branching (one branch increments,
+// the other assigns a shape literal) should preserve the shape type through
+// foreach iteration. Reproduces the real-world pattern from B13.
+// expect: parentProduct(
+---
+<?php
+
+class Product
+{
+    public function parentProduct(): self { return $this; }
+}
+
+class ProductBundle
+{
+    public int $id = 1;
+    public Product $product;
+
+    public function parentProduct(): Product { return $this->product; }
+}
+
+class Item
+{
+    public int $id = 0;
+    public ProductBundle $productBundle;
+}
+
+class Listener
+{
+    /** @param Item[] $items */
+    public function handle(array $items): void
+    {
+        $bundleProductCounts = [];
+        foreach ($items as $item) {
+            $productBundle = $item->productBundle;
+
+            if (array_key_exists($productBundle->id, $bundleProductCounts)) {
+                $bundleProductCounts[$productBundle->id]['count']++;
+            } else {
+                $bundleProductCounts[$productBundle->id] = [
+                    'bundle' => $productBundle,
+                    'count'  => 1,
+                ];
+            }
+        }
+
+        foreach ($bundleProductCounts as $bundleProductCount) {
+            $bundleProductCount['bundle']-><>
+        }
+    }
+}

tests/fixtures/foreach/array_shape_from_keyed_loop_method_chain.fixture

@@ -0,0 +1,44 @@
+// test: foreach over array built with variable-keyed shape assignments resolves chained method
+// feature: completion
+// The shape value type ('bundle' => ProductBundle) must survive the
+// variable-key merge and foreach extraction so that chaining
+// $entry['bundle']->parentProduct()-> resolves Product members.
+// expect: getName(
+---
+<?php
+
+class Product
+{
+    public function getName(): string { return ''; }
+}
+
+class ProductBundle
+{
+    public int $id = 1;
+
+    public function parentProduct(): Product { return new Product(); }
+}
+
+class Item
+{
+    public int $id = 0;
+    public ProductBundle $productBundle;
+}
+
+class Listener
+{
+    /** @param Item[] $items */
+    public function handle(array $items): void
+    {
+        $bundleProductCounts = [];
+        foreach ($items as $item) {
+            $bundleProductCounts[$item->id] = [
+                'bundle' => $item->productBundle,
+                'count'  => 1,
+            ];
+        }
+        foreach ($bundleProductCounts as $entry) {
+            $entry['bundle']->parentProduct()-><>
+        }
+    }
+}