Guard clause narrowing across instanceof branches

Author: AJenbo

docs/CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Guard clause narrowing across instanceof branches.** After `if ($x instanceof Y) { return; }`, subsequent `instanceof` checks on the same variable no longer incorrectly resolve to `Y`. Previously the diagnostic cache reused the narrowed type from inside the first branch for all later accesses to the same variable, producing false-positive "not found" warnings (e.g. "Property 'value' not found on class 'Stringable'" inside a `BackedEnum` branch).
 - **`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.

docs/todo/bugs.md

@@ -15,41 +15,4 @@ within the same impact tier.
 
 ---
 
-#### B10. Negative narrowing after early return not applied
-
-| | |
-|---|---|
-| **Impact** | Low-Medium |
-| **Effort** | Medium |
-
-After `if ($x instanceof Y) { return; }`, the variable `$x` should be
-narrowed to exclude `Y` for all subsequent code in the same scope.
-PHPantom does not apply this negative narrowing via early return.
-
-**Reproduce:**
-
-```php
-public static function toString(mixed $value): string
-{
-    if ($value instanceof Stringable) {
-        return $value->__toString();
-    }
-    if ($value instanceof BackedEnum) {
-        $value = $value->value; // PHPantom resolves $value as Stringable here
-    }
-}
-```
-
-The diagnostic reports `Property 'value' not found on class 'Stringable'`
-because `$value` is still resolved as `Stringable` inside the
-`BackedEnum` branch, even though that branch is only reachable when
-`$value` is NOT `Stringable`.
-
-Guard-clause narrowing already works for `if (!$x instanceof Y) { return; }`
-(positive narrowing after negated check). This is the inverse: positive
-check with exit should produce negative narrowing for subsequent code.
-
-**Triage count:** ~2 diagnostics directly, but a general correctness
-issue that affects any code using the early-return-after-instanceof
-pattern.
-
+No outstanding items.

example.php

@@ -316,6 +316,18 @@ public function demo(): void
         if (!$unknown instanceof Rock) return;    // single-statement guard (no braces)
         $unknown->crush();                        // narrowed to Rock
     }
+
+    /** Positive instanceof + early return on a mixed parameter. */
+    public function mixedGuard(mixed $value): void
+    {
+        if ($value instanceof Banana) {
+            return;                               // $value is Banana → exit
+        }
+        // After the guard, $value is NOT Banana.
+        if ($value instanceof Rock) {
+            $value->crush();                      // narrowed to Rock (not Banana)
+        }
+    }
 }
 
 
@@ -4936,7 +4948,19 @@ function runDemoAssertions(): void
         assert($guardSubject instanceof Rock, 'Guard: not Banana must be Rock');
     } else {
         assert($guardSubject instanceof Banana, 'Guard: else must be Banana');
-    }
+        }
+
+        // ── Guard clause: positive instanceof + early return on mixed ────
+        // After `if ($x instanceof Y) { return; }`, $x is NOT Y.
+        $mixedGuardVal = rand(0, 1) ? new Rock() : 'scalar';
+        if ($mixedGuardVal instanceof Banana) {
+            // would return in real code
+            assert(false, 'Guard: should not reach here (Banana branch)');
+        }
+        // $mixedGuardVal is NOT Banana after the guard
+        if ($mixedGuardVal instanceof Rock) {
+            assert(is_string($mixedGuardVal->crush()), 'Guard: mixed narrowed to Rock');
+        }
 
     // ── Ternary narrowing ───────────────────────────────────────────────
     $ternaryThing = pickRockOrBanana();

src/analyse.rs

@@ -238,65 +238,87 @@ pub async fn run(options: AnalyseOptions) -> i32 {
                         }
 
                         let mut raw = Vec::new();
-                        let file_start = std::time::Instant::now();
-
-                        macro_rules! timed_collect {
-                            ($name:expr, $call:expr) => {{
-                                let t0 = std::time::Instant::now();
-                                $call;
-                                (t0.elapsed(), $name)
-                            }};
+
+                        // In debug builds, time each collector and warn
+                        // about slow files.  In release builds, just call
+                        // the collectors directly.
+                        #[cfg(debug_assertions)]
+                        {
+                            macro_rules! timed_collect {
+                                ($name:expr, $call:expr) => {{
+                                    let t0 = std::time::Instant::now();
+                                    $call;
+                                    (t0.elapsed(), $name)
+                                }};
+                            }
+
+                            let file_start = std::time::Instant::now();
+                            let timings = [
+                                timed_collect!(
+                                    "fast",
+                                    backend.collect_fast_diagnostics(uri, content, &mut raw)
+                                ),
+                                timed_collect!(
+                                    "unknown_class",
+                                    backend
+                                        .collect_unknown_class_diagnostics(uri, content, &mut raw)
+                                ),
+                                timed_collect!(
+                                    "unknown_member",
+                                    backend
+                                        .collect_unknown_member_diagnostics(uri, content, &mut raw)
+                                ),
+                                timed_collect!(
+                                    "unknown_function",
+                                    backend.collect_unknown_function_diagnostics(
+                                        uri, content, &mut raw,
+                                    )
+                                ),
+                                timed_collect!(
+                                    "argument_count",
+                                    backend
+                                        .collect_argument_count_diagnostics(uri, content, &mut raw)
+                                ),
+                                timed_collect!(
+                                    "implementation",
+                                    backend.collect_implementation_error_diagnostics(
+                                        uri, content, &mut raw,
+                                    )
+                                ),
+                                timed_collect!(
+                                    "deprecated",
+                                    backend.collect_deprecated_diagnostics(uri, content, &mut raw)
+                                ),
+                            ];
+
+                            let file_elapsed = file_start.elapsed();
+                            if file_elapsed.as_secs() >= 5 {
+                                let display =
+                                    files[i].strip_prefix(root).unwrap_or(&files[i]).display();
+                                let breakdown: Vec<String> = timings
+                                    .iter()
+                                    .filter(|(d, _)| d.as_millis() > 0)
+                                    .map(|(d, name)| format!("{}={:.1}s", name, d.as_secs_f64()))
+                                    .collect();
+                                eprintln!(
+                                    "\n  \u{26a0} slow file ({:.1}s): {}\n    {}",
+                                    file_elapsed.as_secs_f64(),
+                                    display,
+                                    breakdown.join(", "),
+                                );
+                            }
                         }
 
-                        let timings = [
-                            timed_collect!(
-                                "fast",
-                                backend.collect_fast_diagnostics(uri, content, &mut raw)
-                            ),
-                            timed_collect!(
-                                "unknown_class",
-                                backend.collect_unknown_class_diagnostics(uri, content, &mut raw)
-                            ),
-                            timed_collect!(
-                                "unknown_member",
-                                backend.collect_unknown_member_diagnostics(uri, content, &mut raw)
-                            ),
-                            timed_collect!(
-                                "unknown_function",
-                                backend
-                                    .collect_unknown_function_diagnostics(uri, content, &mut raw)
-                            ),
-                            timed_collect!(
-                                "argument_count",
-                                backend.collect_argument_count_diagnostics(uri, content, &mut raw)
-                            ),
-                            timed_collect!(
-                                "implementation",
-                                backend.collect_implementation_error_diagnostics(
-                                    uri, content, &mut raw
-                                )
-                            ),
-                            timed_collect!(
-                                "deprecated",
-                                backend.collect_deprecated_diagnostics(uri, content, &mut raw)
-                            ),
-                        ];
-
-                        let file_elapsed = file_start.elapsed();
-                        if file_elapsed.as_secs() >= 5 {
-                            let display =
-                                files[i].strip_prefix(root).unwrap_or(&files[i]).display();
-                            let breakdown: Vec<String> = timings
-                                .iter()
-                                .filter(|(d, _)| d.as_millis() > 0)
-                                .map(|(d, name)| format!("{}={:.1}s", name, d.as_secs_f64()))
-                                .collect();
-                            eprintln!(
-                                "\n  ⚠ slow file ({:.1}s): {}\n    {}",
-                                file_elapsed.as_secs_f64(),
-                                display,
-                                breakdown.join(", "),
-                            );
+                        #[cfg(not(debug_assertions))]
+                        {
+                            backend.collect_fast_diagnostics(uri, content, &mut raw);
+                            backend.collect_unknown_class_diagnostics(uri, content, &mut raw);
+                            backend.collect_unknown_member_diagnostics(uri, content, &mut raw);
+                            backend.collect_unknown_function_diagnostics(uri, content, &mut raw);
+                            backend.collect_argument_count_diagnostics(uri, content, &mut raw);
+                            backend
+                                .collect_implementation_error_diagnostics(uri, content, &mut raw);
+                            backend.collect_deprecated_diagnostics(uri, content, &mut raw);
                         }
 
                         let mut filtered: Vec<FileDiagnostic> = raw

src/completion/resolver.rs

@@ -167,7 +167,7 @@ impl<'a> VarResolutionCtx<'a> {
 ///
 /// Scope boundaries and variable definitions are stored alongside the
 /// cache and set by [`set_diagnostic_subject_cache_scopes`].
-type DiagSubjectCache = HashMap<(String, AccessKind, u32, u32), Vec<Arc<ClassInfo>>>;
+type DiagSubjectCache = HashMap<(String, AccessKind, u32, u32, u32), Vec<Arc<ClassInfo>>>;
 
 /// File-level data stored alongside the diagnostic subject cache so
 /// that [`resolve_target_classes`] can compute the enclosing scope and
@@ -351,11 +351,23 @@ pub(crate) fn resolve_target_classes(
     // ── Fast path: check the thread-local diagnostic cache ──────
     let scope_start = diag_cache_enclosing_scope(ctx.cursor_offset);
     let var_def_offset = diag_cache_var_def_offset(subject, ctx.cursor_offset);
+    // For variable subjects (excluding $this), include the cursor
+    // offset in the cache key so that accesses inside different
+    // instanceof-narrowing contexts (e.g. different if-bodies) get
+    // independent cache entries.  Without this, the first access
+    // caches a narrowed type and subsequent accesses in a different
+    // narrowing context reuse the wrong result.
+    let narrowing_offset = if subject.starts_with('$') && !subject.starts_with("$this") {
+        ctx.cursor_offset
+    } else {
+        0
+    };
     let cache_key = (
         subject.to_string(),
         access_kind,
         scope_start,
         var_def_offset,
+        narrowing_offset,
     );
     let cached = DIAG_SUBJECT_CACHE.with(|cell| {
         let borrow = cell.borrow();

src/diagnostics/deprecated.rs

@@ -310,7 +310,7 @@ fn deprecated_diagnostic(
     Diagnostic {
         range,
         severity: Some(DiagnosticSeverity::HINT),
-        code: None,
+        code: Some(NumberOrString::String("deprecated".to_string())),
         code_description: None,
         source: Some("phpantom".to_string()),
         message,