Fix a couple of var assignment issues.

Author: AJenbo

docs/CHANGELOG.md

@@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - **`instanceof self/static/parent` narrowing.** Type narrowing with `instanceof self`, `instanceof static`, and `instanceof parent` now works correctly in all contexts (assert, if-blocks, guard clauses, compound conditions). Previously these keywords were ignored because the AST parser produces dedicated node types rather than identifiers, and `parent` was not recognized as a type hint during resolution.
+- **Variable assignments inside foreach loops.** Variables conditionally reassigned inside a `foreach` body are now visible after the loop. Previously, `$x = null; foreach (...) { $x = new Foo(); } $x->method();` lost the `Foo` type because the foreach body was only scanned when the cursor was inside it. The `while`, `for`, and `do-while` loops were not affected.
+- **Variable-to-variable type propagation.** Assignments like `$found = $pen` now resolve `$found` to the type of `$pen`. Previously, bare variable references on the right-hand side of an assignment were ignored, so patterns like `foreach ($pens as $pen) { $found = $pen; }` left `$found` untyped. This also eliminates false-positive `scalar_member_access` diagnostics when the initial assignment was `$found = null` — the type from the later reassignment now takes precedence.
 - **Variable type after reassignment.** When a method parameter is reassigned mid-body (e.g. `$file = $result->getFile()`), subsequent member accesses now resolve against the new type instead of the original parameter type. Previously the diagnostic subject cache reused the first resolution for the entire method scope, producing false-positive "not found" warnings on members that exist only on the reassigned type.
 - **Null-safe method chain resolution.** Null-safe method calls (`$obj?->method()`) now resolve the return type correctly for variable type inference, including cross-file chains. Previously `?->` calls were ignored by the RHS resolution pipeline, losing the type for any variable assigned from a null-safe chain.
 - **Nullable and generic types in class lookup.** Variables typed as `?ClassName` or `Collection<Item>` now resolve correctly across all code paths. Previously the `?` prefix and generic parameters were not stripped before class lookup, causing the type engine to treat them as unknown types. This fixes completion, hover, go-to-definition, and false-positive diagnostics for any variable whose type uses the nullable shorthand or carries generic arguments.

docs/todo/bugs.md

@@ -53,39 +53,3 @@ check with exit should produce negative narrowing for subsequent code.
 issue that affects any code using the early-return-after-instanceof
 pattern.
 
----
-
-#### B11. Variables initialized to `null` and conditionally reassigned lose their type
-
-| | |
-|---|---|
-| **Impact** | Medium |
-| **Effort** | Medium |
-
-When a variable is initialized as `$x = null` and then conditionally
-reassigned inside a loop or branch (e.g. `$x = $transaction`), PHPantom
-resolves `$x` to `null` at the usage site even after a truthiness guard
-like `if (!$x) { continue; }` or `if ($x !== null)`.
-
-Three common patterns:
-
-1. **Null-init + loop assignment + guard:**
-   `$x = null; foreach (...) { if (cond) { $x = $expr; } } if ($x) { $x->method(); }`
-
-2. **Null coalesce + guard:**
-   `$x = $arr[$key] ?? null; if (!$x) { continue; } $x->property;`
-
-3. **assertNotNull:**
-   `$day = $arr['key'] ?? null; self::assertNotNull($day); $day->from;`
-
-The root cause is that PHPantom's variable resolution picks the first
-(or dominant) assignment and does not consider all assignment sites to
-build a union type. For pattern 1, only the `= null` is seen. For
-patterns 2 and 3, the `?? null` makes the type nullable but the
-subsequent guard or assertion is not recognized as narrowing.
-
-Pattern 3 overlaps with custom assert narrowing (`@phpstan-assert`) which
-requires recognizing `assertNotNull` as a type guard.
-
-**Triage count:** ~8 scalar_member_access diagnostics in luxplus/shared
-(AltapayGateway, PCNService, CustomerService, CoolrunnerClientTest).

example.php

@@ -458,6 +458,37 @@ public function demo(): void
 }
 
 
+// ── Null-Init + Conditional Reassignment ────────────────────────────────────
+
+class NullInitReassignDemo
+{
+    /** @param list<Pen> $pens */
+    public function demo(array $pens): void
+    {
+        // Pattern 1: null-init + foreach reassignment + truthiness guard
+        $found = null;
+        foreach ($pens as $pen) {
+            if ($pen->color() === 'blue') {
+                $found = $pen;
+            }
+        }
+        if ($found) {
+            $found->write();                      // Pen from foreach reassignment
+        }
+
+        // Pattern 2: null-coalesce + guard inside foreach
+        /** @var array<string, Pen> $lookup */
+        $lookup = getUnknownValue();
+        $keys = ['a', 'b'];
+        foreach ($keys as $key) {
+            $item = $lookup[$key] ?? null;
+            if (!$item) { continue; }
+            $item->write();                       // Pen from array access via coalesce
+        }
+    }
+}
+
+
 // ── Foreach & Array Access ──────────────────────────────────────────────────
 
 class ForeachArrayAccessDemo
@@ -4389,6 +4420,17 @@ function runDemoAssertions(): void
     assert(StaticAssert::isRock(new Rock()) === true, 'StaticAssert::isRock(Rock) must return true');
     assert(StaticAssert::isNotRock(new Banana()) === true, 'StaticAssert::isNotRock(Banana) must return true');
 
+    // ── Null-init + foreach reassignment (B11) ──────────────────────────
+    $pens = [new Pen('blue'), new Pen('red')];
+    $found = null;
+    foreach ($pens as $pen) {
+        if ($pen->color() === 'blue') {
+            $found = $pen;
+        }
+    }
+    assert($found instanceof Pen, 'Null-init + foreach reassign must resolve to Pen');
+    assert(method_exists($found, 'write'), 'Pen from foreach must have write()');
+
     // ── instanceof self/static/parent ───────────────────────────────────
     $sedan = new ScaffoldingSedan();
     assert($sedan instanceof ScaffoldingMotor, 'ScaffoldingSedan must extend ScaffoldingMotor');

src/completion/variable/raw_type_inference.rs

@@ -839,6 +839,48 @@ fn resolve_rhs_raw_type<'b>(rhs: &'b Expression<'b>, ctx: &VarResolutionCtx<'_>)
             }
             combined
         }
+        // ── Bare variable: `$a = $b` ────────────────────────────────
+        // Resolve the RHS variable's raw type so that hover shows the
+        // propagated type.  Try the raw-type accumulator first (handles
+        // regular assignments); fall back to the completion pipeline
+        // which also handles foreach value/key bindings.
+        Expression::Variable(Variable::Direct(dv)) => {
+            let var_name = dv.name.to_string();
+            // Guard: never recurse into the same variable.
+            if var_name == ctx.var_name {
+                return None;
+            }
+            // Try raw type resolution first (cheap, same pipeline).
+            if let Some(raw) = resolve_variable_assignment_raw_type(
+                &var_name,
+                ctx.content,
+                ctx.cursor_offset,
+                Some(ctx.current_class),
+                ctx.all_classes,
+                ctx.class_loader,
+                ctx.function_loader,
+            ) {
+                return Some(raw);
+            }
+            // Fall back to the completion pipeline which handles foreach
+            // value/key bindings and other sources the raw-type pipeline
+            // cannot reach.
+            let classes = super::resolution::resolve_variable_types(
+                &var_name,
+                ctx.current_class,
+                ctx.all_classes,
+                ctx.content,
+                ctx.cursor_offset,
+                ctx.class_loader,
+                ctx.function_loader,
+            );
+            if !classes.is_empty() {
+                let names: Vec<&str> = classes.iter().map(|c| c.name.as_str()).collect();
+                return Some(names.join("|"));
+            }
+            // Last resort: docblock scan (e.g. `@var` inline annotation).
+            super::foreach_resolution::extract_rhs_iterable_raw_type(rhs, ctx)
+        }
         // ── Call / property access — delegate to iterable extractor,
         //    with a source-scan fallback for standalone function calls
         //    when no `function_loader` is available. ──

src/completion/variable/resolution.rs

@@ -1053,7 +1053,10 @@ fn walk_foreach_statement<'b>(
 ) {
     let body_span = foreach.body.span();
     let header_start = foreach.foreach.span().start.offset;
-    if ctx.cursor_offset >= header_start && ctx.cursor_offset <= body_span.end.offset {
+    let cursor_inside =
+        ctx.cursor_offset >= header_start && ctx.cursor_offset <= body_span.end.offset;
+
+    if cursor_inside {
         // ── Foreach value/key type from generic iterables ──
         // When the variable we're resolving is the foreach
         // *value* variable, try to infer its type from the
@@ -1081,14 +1084,21 @@ fn walk_foreach_statement<'b>(
             conditional,
         );
         super::foreach_resolution::try_resolve_foreach_key_type(foreach, ctx, results, conditional);
+    }
 
-        match &foreach.body {
-            ForeachBody::Statement(inner) => {
-                check_statement_for_assignments(inner, ctx, results, true);
-            }
-            ForeachBody::ColonDelimited(body) => {
-                walk_statements_for_assignments(body.statements.iter(), ctx, results, true);
-            }
+    // Always walk the foreach body for variable assignments, even when
+    // the cursor is after the foreach.  A foreach body may execute zero
+    // or more times, so any assignment inside is conditional.
+    //
+    // Without this, `$x = null; foreach (...) { $x = new Foo(); }
+    // $x->method();` would lose the `Foo` assignment because the body
+    // was only walked when the cursor was inside the foreach (B11).
+    match &foreach.body {
+        ForeachBody::Statement(inner) => {
+            check_statement_for_assignments(inner, ctx, results, true);
+        }
+        ForeachBody::ColonDelimited(body) => {
+            walk_statements_for_assignments(body.statements.iter(), ctx, results, true);
         }
     }
 }

src/completion/variable/rhs_resolution.rs

@@ -127,6 +127,28 @@ pub(in crate::completion) fn resolve_rhs_expression<'b>(
             }
             vec![]
         }
+        // ── Bare variable: `$a = $b` ────────────────────────────────
+        // Resolve the RHS variable's type by walking assignments before
+        // this point.  The caller (`check_expression_for_assignment`)
+        // already set `ctx.cursor_offset` to the assignment's start
+        // offset, so the recursive resolution only considers
+        // assignments *before* the current one, preventing cycles.
+        Expression::Variable(Variable::Direct(dv)) => {
+            let rhs_var = dv.name.to_string();
+            // Guard: never recurse into the same variable (self-assignment).
+            if rhs_var == ctx.var_name {
+                return vec![];
+            }
+            super::resolution::resolve_variable_types(
+                &rhs_var,
+                ctx.current_class,
+                ctx.all_classes,
+                ctx.content,
+                ctx.cursor_offset,
+                ctx.class_loader,
+                ctx.function_loader,
+            )
+        }
         _ => vec![],
     }
 }

src/diagnostics/unknown_members.rs

@@ -3623,4 +3623,81 @@ class Service {
             "expected exactly one 'onlyOnA' diagnostic (after reassignment), got: {relevant:?}"
         );
     }
+
+    /// B11: `$found = null; foreach (...) { $found = $pen; } $found->write()`
+    /// must not produce a scalar_member_access diagnostic when the foreach
+    /// value variable has a known type.
+    #[test]
+    fn no_false_positive_null_init_foreach_var_to_var_reassign() {
+        let php = r#"<?php
+class Pen {
+    public function write(): void {}
+    public function color(): string { return ''; }
+}
+class Svc {
+    /** @param list<Pen> $pens */
+    public function find(array $pens): void {
+        $found = null;
+        foreach ($pens as $pen) {
+            if ($pen->color() === 'blue') {
+                $found = $pen;
+            }
+        }
+        if ($found) {
+            $found->write();
+        }
+    }
+}
+"#;
+        let backend = Backend::new_test();
+        let diags = collect(&backend, "file:///test.php", php);
+        let scalar_diags: Vec<_> = diags
+            .iter()
+            .filter(|d| d.code == Some(NumberOrString::String("scalar_member_access".to_string())))
+            .collect();
+        assert!(
+            scalar_diags.is_empty(),
+            "should not flag scalar_member_access on $found->write() after foreach reassign, got: {scalar_diags:?}"
+        );
+        let unknown_diags: Vec<_> = diags
+            .iter()
+            .filter(|d| d.message.contains("write"))
+            .collect();
+        assert!(
+            unknown_diags.is_empty(),
+            "should not flag unknown member 'write' on $found after foreach reassign, got: {unknown_diags:?}"
+        );
+    }
+
+    /// B11: direct instantiation inside foreach body (no var-to-var).
+    #[test]
+    fn no_false_positive_null_init_foreach_direct_reassign() {
+        let php = r#"<?php
+class Transaction {
+    public function commit(): void {}
+}
+class Svc {
+    /** @param list<string> $items */
+    public function process(array $items): void {
+        $tx = null;
+        foreach ($items as $item) {
+            $tx = new Transaction();
+        }
+        if ($tx) {
+            $tx->commit();
+        }
+    }
+}
+"#;
+        let backend = Backend::new_test();
+        let diags = collect(&backend, "file:///test.php", php);
+        let bad_diags: Vec<_> = diags
+            .iter()
+            .filter(|d| d.message.contains("commit") || d.message.contains("null"))
+            .collect();
+        assert!(
+            bad_diags.is_empty(),
+            "should not flag commit() or scalar null after foreach reassign, got: {bad_diags:?}"
+        );
+    }
 }