Track variable reassignment inside try/catch/finally blocks

Author: AJenbo

docs/CHANGELOG.md

@@ -56,6 +56,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Variable reassignment inside `try` blocks now tracked by the analyzer.** When a variable was reassigned to a different type inside a `try` block, subsequent accesses within the same block still resolved against the original type, producing false-positive `unknown_member` diagnostics. The same applied to `catch` and `finally` blocks. Assignments preceding the cursor within the same block are now treated as sequential.
 - **Diagnostics now work for vendor files open in the editor.** Projects using `--prefer-source` or monorepo setups where libraries are edited from the `vendor/` directory no longer have diagnostics suppressed. Previously all vendor files were unconditionally skipped, hiding syntax errors, deprecation warnings, and all other diagnostics. The `analyze` command also accepts vendor paths as explicit overrides (e.g. `phpantom_lsp analyze vendor/some-package/src/`).
 - **Full-line diagnostic suppression no longer requires a code mapping.** Full-line diagnostics (from tools like PHPStan that only report line numbers) are now suppressed whenever any precise diagnostic exists on the same line, regardless of error codes. Previously suppression relied on a hand-maintained mapping between error codes from different tools, which was incomplete and allowed redundant full-line underlines to obscure the precise location of the issue.
 

docs/todo.md

@@ -23,7 +23,6 @@ within the same impact tier.
 
 | #   | Item                                                                                                                                             | Impact | Effort |
 | --- | ------------------------------------------------------------------------------------------------------------------------------------------------ | ------ | ------ |
-| B3  | [Variable reassignment inside `try` block not tracked by analyzer](todo/bugs.md#b3-variable-reassignment-inside-try-block-not-tracked-by-analyzer) | Low | Low |
 | B4  | [Relationship property access and BelongsTo return type not resolved](todo/bugs.md#b4-relationship-property-access-and-belongsto-return-type-not-resolved-by-analyzer) | Medium | Medium |
 | B5  | [`$this->items` on custom Collection subclass not typed](todo/bugs.md#b5-thisitems-on-custom-collection-subclass-not-typed)                      | Low    | Medium |
 | B6  | [Scope methods not found on Builder in analyzer chains](todo/bugs.md#b6-scope-methods-not-found-on-builder-in-analyzer-chains)                   | High   | Medium |

docs/todo/bugs.md

@@ -1,34 +1,5 @@
 # PHPantom — Bug Fixes
 
-## B3: Variable reassignment inside `try` block not tracked by analyzer
-
-When a variable is assigned one type before a `try` block and then
-reassigned to a different type inside the `try`, the analyzer uses the
-original type instead of the reassigned type for subsequent accesses
-within the same block.
-
-Real-world example — `MollieGateway.php`:
-
-```php
-$customer = $request->getCustomerOrFail();   // type: Luxplus Customer
-// ...
-try {
-    $customer = $client->getOrCreateCustoemr($customer);  // reassigned to MollieCustomer
-    $molliePayment = $customer->createPayment($paymentData);  // ← analyzer uses original type
-}
-```
-
-Line 452 reassigns `$customer` from `Luxplus\...\Customer` to
-`Mollie\Api\Resources\Customer` (aliased as `MollieCustomer`). Line 453
-calls `->createPayment()` which exists on `MollieCustomer` but not on
-the Luxplus `Customer` model. The analyzer resolves `$customer` as the
-original type, producing an `unknown_member` diagnostic for
-`createPayment` and a cascading `unresolved_member_access` for
-`$molliePayment->getCheckoutUrl()`.
-
-**Impact:** 2 diagnostics in the shared project (`MollieGateway:453`,
-`MollieGateway:462`).
-
 ## B4: Relationship property access and BelongsTo return type not resolved by analyzer
 
 Eloquent relationship methods accessed as properties (without `()`) are

src/completion/variable/resolution.rs

@@ -2509,7 +2509,22 @@ fn walk_try_statement<'b>(
     ctx: &VarResolutionCtx<'_>,
     results: &mut Vec<ResolvedType>,
 ) {
-    walk_statements_for_assignments(try_stmt.block.statements.iter(), ctx, results, true);
+    // When the cursor is inside the try block body, assignments that
+    // precede the cursor have executed sequentially — use
+    // `conditional: false` so reassignments replace the previous type
+    // instead of forming a union with it.  When the cursor is outside
+    // (after the try statement), we don't know whether the try body
+    // ran to completion, so keep `conditional: true`.
+    let try_span = try_stmt.block.span();
+    let cursor_in_try =
+        ctx.cursor_offset >= try_span.start.offset && ctx.cursor_offset <= try_span.end.offset;
+    walk_statements_for_assignments(
+        try_stmt.block.statements.iter(),
+        ctx,
+        results,
+        !cursor_in_try,
+    );
+
     for catch in try_stmt.catch_clauses.iter() {
         // Seed the catch variable's type from the catch
         // clause's type hint(s) before recursing into the
@@ -2528,10 +2543,28 @@ fn walk_try_statement<'b>(
             );
             ResolvedType::extend_unique(results, ResolvedType::from_classes(resolved));
         }
-        walk_statements_for_assignments(catch.block.statements.iter(), ctx, results, true);
+        // Same logic: when the cursor is inside this catch block,
+        // treat preceding assignments as unconditional.
+        let catch_span = catch.block.span();
+        let cursor_in_catch = ctx.cursor_offset >= catch_span.start.offset
+            && ctx.cursor_offset <= catch_span.end.offset;
+        walk_statements_for_assignments(
+            catch.block.statements.iter(),
+            ctx,
+            results,
+            !cursor_in_catch,
+        );
     }
     if let Some(finally) = &try_stmt.finally_clause {
-        walk_statements_for_assignments(finally.block.statements.iter(), ctx, results, true);
+        let finally_span = finally.block.span();
+        let cursor_in_finally = ctx.cursor_offset >= finally_span.start.offset
+            && ctx.cursor_offset <= finally_span.end.offset;
+        walk_statements_for_assignments(
+            finally.block.statements.iter(),
+            ctx,
+            results,
+            !cursor_in_finally,
+        );
     }
 }
 

src/diagnostics/unknown_members.rs

@@ -4902,4 +4902,137 @@ class Svc {
             "Should flag 'unknown', got: {diags:?}"
         );
     }
+
+    #[test]
+    fn no_false_positive_when_variable_reassigned_inside_try_block() {
+        // When a variable is reassigned inside a `try` block, accesses
+        // after the reassignment (still inside the try) should resolve
+        // against the new type, not the original.
+        let php = r#"<?php
+class LuxplusCustomer {
+    public function getName(): string { return ''; }
+}
+class MollieCustomer {
+    public function createPayment(string $data): MolliePayment { return new MolliePayment(); }
+}
+class MolliePayment {
+    public function getCheckoutUrl(): string { return ''; }
+}
+class MollieClient {
+    public function getOrCreateCustomer(LuxplusCustomer $c): MollieCustomer { return new MollieCustomer(); }
+}
+class Gateway {
+    public function charge(LuxplusCustomer $customer): void {
+        $client = new MollieClient();
+        try {
+            $customer = $client->getOrCreateCustomer($customer);
+            $molliePayment = $customer->createPayment('data');
+            $url = $molliePayment->getCheckoutUrl();
+        } catch (\Exception $e) {
+        }
+    }
+}
+"#;
+        let backend = Backend::new_test();
+        let diags = collect(&backend, "file:///test.php", php);
+        assert!(
+            diags.is_empty(),
+            "expected no diagnostics for reassigned variable inside try block, got: {diags:?}"
+        );
+    }
+
+    #[test]
+    fn flags_unknown_member_after_reassignment_inside_try_block() {
+        // The flip side: after reassignment inside a try block, members
+        // from the OLD type that don't exist on the NEW type should be
+        // flagged.
+        let php = r#"<?php
+class OriginalType {
+    public function onlyOnOriginal(): void {}
+}
+class ReplacementType {
+    public function onlyOnReplacement(): void {}
+}
+class Service {
+    public function process(OriginalType $var): void {
+        try {
+            $var = new ReplacementType();
+            $var->onlyOnOriginal();
+        } catch (\Exception $e) {
+        }
+    }
+}
+"#;
+        let backend = Backend::new_test();
+        let diags = collect(&backend, "file:///test.php", php);
+        assert!(
+            diags
+                .iter()
+                .any(|d| d.message.contains("onlyOnOriginal")
+                    && d.message.contains("ReplacementType")),
+            "expected diagnostic for onlyOnOriginal() on ReplacementType after reassignment in try, got: {diags:?}"
+        );
+    }
+
+    #[test]
+    fn try_block_reassignment_is_conditional_after_try() {
+        // After the try/catch block, the variable could be either the
+        // original type (if the try threw before the reassignment) or
+        // the new type.  Both types' members should be accepted.
+        let php = r#"<?php
+class TypeA {
+    public function methodA(): void {}
+}
+class TypeB {
+    public function methodB(): void {}
+}
+class Svc {
+    public function run(TypeA $var): void {
+        try {
+            $var = new TypeB();
+        } catch (\Exception $e) {
+        }
+        $var->methodA();
+        $var->methodB();
+    }
+}
+"#;
+        let backend = Backend::new_test();
+        let diags = collect(&backend, "file:///test.php", php);
+        assert!(
+            diags.is_empty(),
+            "after try/catch, both original and reassigned types should be accepted, got: {diags:?}"
+        );
+    }
+
+    #[test]
+    fn catch_block_variable_reassignment_tracked() {
+        // Variable reassignment inside a catch block should also be
+        // tracked when the cursor is inside the catch block.
+        let php = r#"<?php
+class ErrorResult {
+    public function getErrorCode(): int { return 0; }
+}
+class SuccessResult {
+    public function getData(): string { return ''; }
+}
+class Handler {
+    public function handle(): void {
+        $result = new SuccessResult();
+        try {
+            $result->getData();
+        } catch (\Exception $e) {
+            $result = new ErrorResult();
+            $result->getErrorCode();
+        }
+    }
+}
+"#;
+        let backend = Backend::new_test();
+        let diags = collect(&backend, "file:///test.php", php);
+        assert!(
+            diags.is_empty(),
+            "expected no diagnostics for reassigned variable inside catch block, got: {diags:?}"
+        );
+    }
 }