Apply inverse condition narrowing after while and do-while loops

Author: AJenbo

docs/CHANGELOG.md

@@ -50,7 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Infinite loop on array key reassignment patterns.** Files containing `$arr['key'] = f($arr['key'])` or similar read-then-write patterns on the same array key no longer hang the analyzer.
 - **Stack overflow on large codebases and large files.** The `analyze` command and files with hundreds of class definitions no longer crash with stack overflows.
 - **`analyze` and `fix` commands run at consistent speed regardless of invocation style.** Running from within a project directory is no longer ~8x slower than using `--project-root`.
-- **Type narrowing.** Comprehensive fixes to type narrowing: non-exiting `if` branches no longer leak narrowed types into post-merge scope; `is_float()`, `is_null()`, and other `is_*()` guards correctly narrow multi-member unions; negated `instanceof` on nullable unions preserves `null`; `is_object()` narrows to all class types in a union (not just the first); `instanceof` on `mixed` or `object` narrows to the checked type; `=== null` and `== null` narrow to `null` in the truthy branch; `assert($x instanceof Foo)` followed by any `if` block no longer loses the narrowed type; falsy guard clauses strip both `false` and `null`; template-constrained types are expanded before guard filtering. `is_numeric()`, `is_bool()`, and `is_scalar()` guard clauses now narrow union types correctly. `exit` and `die` are recognized as unconditional exits for guard clause narrowing. Property access expressions (`$a->foo`) are now narrowed through chained if/elseif/else conditions using `is_string()`, `instanceof`, and other type guards. Array shape keys are now narrowed through null-check guard clauses and conditional reassignment (e.g. `$a["test"]` resolves to `int` instead of `?int` after `if ($a["test"] === null) { return; }`). After `instanceof` check plus reassignment inside the if-body, the post-merge type collapses to the common parent class instead of producing a redundant `Child|Parent` union. Property access after OR'd `instanceof` checks (`$a instanceof B || $a instanceof C`) now resolves to the union of all branches' property types.
+- **Type narrowing.** Comprehensive fixes to type narrowing: non-exiting `if` branches no longer leak narrowed types into post-merge scope; `is_float()`, `is_null()`, and other `is_*()` guards correctly narrow multi-member unions; negated `instanceof` on nullable unions preserves `null`; `is_object()` narrows to all class types in a union (not just the first); `instanceof` on `mixed` or `object` narrows to the checked type; `=== null` and `== null` narrow to `null` in the truthy branch; `assert($x instanceof Foo)` followed by any `if` block no longer loses the narrowed type; falsy guard clauses strip both `false` and `null`; template-constrained types are expanded before guard filtering. `is_numeric()`, `is_bool()`, and `is_scalar()` guard clauses now narrow union types correctly. `exit` and `die` are recognized as unconditional exits for guard clause narrowing. Property access expressions (`$a->foo`) are now narrowed through chained if/elseif/else conditions using `is_string()`, `instanceof`, and other type guards. Array shape keys are now narrowed through null-check guard clauses and conditional reassignment (e.g. `$a["test"]` resolves to `int` instead of `?int` after `if ($a["test"] === null) { return; }`). After `instanceof` check plus reassignment inside the if-body, the post-merge type collapses to the common parent class instead of producing a redundant `Child|Parent` union. Property access after OR'd `instanceof` checks (`$a instanceof B || $a instanceof C`) now resolves to the union of all branches' property types. After `while` and `do-while` loops exit, the loop condition's inverse is applied to narrow variable types (e.g. `do { $a = $a->next; } while ($a)` narrows `$a` to `null` after the loop; `while ($a instanceof Foo)` narrows `$a` to the excluded type after exit). Branch merging no longer loses nullable information when two branches produce the same class with different nullability (e.g. merging `A` with `A|null` correctly yields `A|null` instead of silently dropping the null).
 - **Mixin resolution.** Static method calls on instances of a class with `@mixin` now resolve through the mixin. `@method` and `@property` tags declared on a mixin class are propagated to the consuming class. `$this` return types on mixin methods resolve to the consumer class. `IteratorIterator` is now patched with `@template` parameters and `@mixin TIterator` (matching PHPStan's stubs).
 - **Generics.** Constructor generic inference now works through inherited constructors: child classes without their own constructor infer template parameters from the parent's constructor arguments, with correct remapping through multi-level `@extends` chains (including swapped or renamed template parameters). Function-level `@template` parameters bound to generic wrapper types (e.g. `@param Container<TItem> $c`) are inferred from arguments that extend the wrapper class. Method calls are now case-insensitive, matching PHP semantics (e.g. `$obj->getId()` finds `getID()`). Closure literals passed to `@template` parameters are recognised as `Closure`. Class-level template parameters are preserved through chained method calls. Template parameters fall back to their declared bound (or `mixed` when unbounded) when subclasses omit `@extends` or `@use` annotations. Unbound template parameters at call sites resolve to their declared bounds or `mixed` instead of leaking raw names. Method-level templates resolve correctly through generic wrappers and nested call chains. Return type generic arguments are preserved for template substitution, fixing false "expects TRelatedModel, got Translation" diagnostics. Iterating over a subclass that extends a generic collection with scalar type arguments (e.g. `IntCollection extends Collection<int, int>`) now yields the concrete scalar type instead of the raw template parameter name. Calling a method on a union of generic types (e.g. `$var->get()` where `$var` is `C<A>|C<B>`) now resolves to the union of each branch's return type (`A|B`) instead of only the first branch. Empty array literals passed to generic constructors or functions infer `never` for element type parameters (e.g. `new ArrayCollection([])` resolves to `ArrayCollection<never, never>`).
 - **Vendor functions and constants.** Functions and constants defined in vendor packages are now indexed at startup, eliminating false-positive "Function not found" diagnostics.

docs/todo/bugs.md

@@ -70,13 +70,13 @@ only by this infrastructure limitation.
 **Discovered:** SKIP audit of `tests/psalm_assertions/loop_do.php`,
 `loop_while.php`, `loop_foreach.php`.
 
-After a loop exits (via condition failure or break), the variable
-type is not properly narrowed. Examples:
+After a loop exits (via condition failure or break), some variable
+types are not properly narrowed. Remaining issues:
 
-- `do { $a = getA(); } while ($a !== null);` — after the loop,
-  `$a` should be `null` but resolves to `A|null`
-- `while ($a = getA()) { ... }` — after the loop, `$a` should
-  be narrowed by the falsy exit condition
+- Cross-namespace class resolution in multi-namespace test files:
+  when multiple namespaces define a class `A`, property type
+  resolution picks up the wrong class, preventing nullable
+  propagation through the loop
 - `foreach` loop variable not narrowed away from initial `null`
   after non-empty iteration
 - Break-in-else branch type not merged with loop variable
@@ -86,8 +86,8 @@ type is not properly narrowed. Examples:
 Related to T20 (type narrowing reconciliation engine) and T29
 (definite vs possible variable existence tracking).
 
-**Tests:** SKIPs in `tests/psalm_assertions/loop_do.php` (lines
-63, 80), `loop_while.php` (lines 39, 91, 115, 152),
+**Tests:** SKIPs in `tests/psalm_assertions/loop_do.php` (line 80),
+`loop_while.php` (lines 91, 115, 152),
 `loop_foreach.php` (lines 81, 128, 156, 188, 208).
 
 

src/completion/variable/forward_walk.rs

@@ -1867,7 +1867,36 @@ impl ScopeState {
     pub fn merge_branch(&mut self, other: &ScopeState) {
         for (name, other_types) in &other.locals {
             let entry = self.locals.entry(*name).or_default();
-            ResolvedType::extend_unique(entry, other_types.clone());
+
+            // Merge other_types into entry.  When an incoming entry
+            // shares a class name with an existing entry but has a
+            // broader type_string (e.g. `?A` vs `A`), widen the
+            // existing entry's type_string instead of discarding
+            // the incoming one.  This prevents post-loop merges from
+            // losing nullable information.
+            for rt in other_types.iter() {
+                let mut merged_into_existing = false;
+                if let Some(ref rt_cls) = rt.class_info {
+                    for existing in entry.iter_mut() {
+                        if let Some(ref ex_cls) = existing.class_info
+                            && ex_cls.name == rt_cls.name
+                        {
+                            // Same class.  If the incoming type is
+                            // broader, adopt it.
+                            if existing.type_string != rt.type_string
+                                && existing.type_string.is_subset_of(&rt.type_string)
+                            {
+                                existing.type_string = rt.type_string.clone();
+                            }
+                            merged_into_existing = true;
+                            break;
+                        }
+                    }
+                }
+                if !merged_into_existing {
+                    ResolvedType::push_unique(entry, rt.clone());
+                }
+            }
 
             // Remove entries whose type is subsumed by a broader entry.
             // E.g. `string|null` ⊆ `int|string|null` → drop the former.
@@ -6781,6 +6810,11 @@ fn process_while<'b>(while_stmt: &'b While<'b>, scope: &mut ScopeState, ctx: &Fo
     *scope = pre_loop_scope;
     scope.merge_branch(&post_loop);
 
+    // After the loop, the condition evaluated to false (that's why the
+    // loop exited).  Apply the inverse of the condition to narrow types.
+    // For example: `while ($a) { $a = $a->parent; }` => after loop, $a is null.
+    apply_condition_narrowing_inverse(while_stmt.condition, scope, ctx);
+
     // Remove synthetic property access keys that were seeded by
     // condition narrowing.  These represent narrowed types that only
     // hold inside the loop body (where the condition is true).
@@ -6934,6 +6968,12 @@ fn process_do_while<'b>(dw: &'b DoWhile<'b>, scope: &mut ScopeState, ctx: &Forwa
         walk_body_forward(std::iter::once(dw.statement), scope, ctx);
     }
 
+    // After the do-while loop, the condition evaluated to false (that's
+    // why the loop exited).  Apply the inverse of the condition to narrow
+    // types.  For example: `do { $a = getA(); } while ($a !== null);`
+    // => after loop, $a is null.
+    apply_condition_narrowing_inverse(dw.condition, scope, ctx);
+
     leave_loop(loop_depth);
 }
 
@@ -8035,6 +8075,12 @@ fn apply_null_narrowing_inverse<'b>(
     if let Some(var_name) = extract_falsy_check_var(condition) {
         strip_null_from_scope(&var_name, scope);
     }
+    // When the condition is a bare `$x` (truthy check), the inverse means
+    // $x is falsy.  For nullable types (`T|null`), narrow to null.
+    // This handles `while ($a) { ... }` => after loop, $a is null.
+    if let Some(var_name) = expr_to_var_name(condition) {
+        narrow_to_null_in_scope(&var_name, scope);
+    }
     // `isset($x)` — inverse (else) means $x was null: narrow to null.
     for var_name in extract_isset_vars(condition) {
         seed_synthetic_key_if_needed(&var_name, scope, ctx);

tests/psalm_assertions/loop_do.php

@@ -60,7 +60,7 @@ function takesA(A $a): void {}
         $a = $a->a;
     } while ($a);
 
-    assertType('null', $a); // SKIP — got A|null, loop exit type not narrowed
+    assertType('null', $a);
 }
 
 // Test: doWhileWithMethodCall
@@ -77,6 +77,6 @@ public function getParent(): ?A {
         $a = $a->getParent();
     } while ($a);
 
-    assertType('null', $a); // SKIP — got A, loop exit type not narrowed
+    assertType('null', $a); // SKIP — cross-namespace class resolution: A resolves to wrong namespace
 }
 

tests/psalm_assertions/loop_while.php

@@ -88,7 +88,7 @@ function makeA(): A {
         $a = $a->parent;
     }
 
-    assertType('null', $a); // SKIP
+    assertType('null', $a); // SKIP — cross-namespace class resolution: A resolves to wrong namespace
 }
 
 // Test: objectValueWithAnd
@@ -149,6 +149,6 @@ function takesA(A $a): void {}
       $a = $a->a;
     };
 
-    assertType('null', $a); // SKIP
+    assertType('null', $a); // SKIP — cross-namespace class resolution: A resolves to wrong namespace
 }