Fix crash when in variable resolver

AJenbo · AJenbo · commit 8d14adca9098 · 2026-02-19T05:04:46.000+01:00
diff --git a/src/completion/variable_completion.rs b/src/completion/variable_completion.rs
@@ -5,6 +5,7 @@
 /// position, respecting PHP scoping rules (function, method, closure, and
 /// top-level scope).
 use std::collections::HashSet;
+use std::panic;
 
 use bumpalo::Bump;
 use mago_span::HasSpan;
@@ -205,13 +206,25 @@ impl Backend {
 /// The returned set contains variable names including the `$` prefix
 /// (e.g. `"$user"`, `"$this"`).
 fn collect_variables_in_scope(content: &str, cursor_offset: u32) -> HashSet<String> {
-    let arena = Bump::new();
-    let file_id = mago_database::file::FileId::new("input.php");
-    let program = parse_file_content(&arena, file_id, content);
-
-    let mut vars = HashSet::new();
-    find_scope_and_collect(program.statements.iter(), cursor_offset, &mut vars);
-    vars
+    // Wrap in catch_unwind so a mago-syntax parser panic doesn't
+    // crash the LSP server (producing a zombie process).
+    let result = panic::catch_unwind(panic::AssertUnwindSafe(|| {
+        let arena = Bump::new();
+        let file_id = mago_database::file::FileId::new("input.php");
+        let program = parse_file_content(&arena, file_id, content);
+
+        let mut vars = HashSet::new();
+        find_scope_and_collect(program.statements.iter(), cursor_offset, &mut vars);
+        vars
+    }));
+
+    match result {
+        Ok(vars) => vars,
+        Err(_) => {
+            log::error!("PHPantomLSP: parser panicked during variable scope collection");
+            HashSet::new()
+        }
+    }
 }
 
 /// Walk top-level statements to find the scope enclosing the cursor,
diff --git a/src/completion/variable_resolution.rs b/src/completion/variable_resolution.rs
@@ -29,6 +29,8 @@
 /// Type narrowing (instanceof, assert, custom type guards) is delegated
 /// to the [`super::type_narrowing`] module.  Closure/arrow-function scope
 /// handling is delegated to [`super::closure_resolution`].
+use std::panic;
+
 use bumpalo::Bump;
 use mago_span::HasSpan;
 use mago_syntax::ast::*;
@@ -61,23 +63,38 @@ impl Backend {
         class_loader: &dyn Fn(&str) -> Option<ClassInfo>,
         function_loader: FunctionLoaderFn<'_>,
     ) -> Vec<ClassInfo> {
-        let arena = Bump::new();
-        let file_id = mago_database::file::FileId::new("input.php");
-        let program = parse_file_content(&arena, file_id, content);
-
-        let ctx = VarResolutionCtx {
-            var_name,
-            current_class,
-            all_classes,
-            content,
-            cursor_offset,
-            class_loader,
-            function_loader,
-        };
+        // Wrap in catch_unwind so a mago-syntax parser panic doesn't
+        // crash the LSP server (producing a zombie process).
+        let result = panic::catch_unwind(panic::AssertUnwindSafe(|| {
+            let arena = Bump::new();
+            let file_id = mago_database::file::FileId::new("input.php");
+            let program = parse_file_content(&arena, file_id, content);
+
+            let ctx = VarResolutionCtx {
+                var_name,
+                current_class,
+                all_classes,
+                content,
+                cursor_offset,
+                class_loader,
+                function_loader,
+            };
 
-        // Walk top-level (and namespace-nested) statements to find the
-        // class + method containing the cursor.
-        Self::resolve_variable_in_statements(program.statements.iter(), &ctx)
+            // Walk top-level (and namespace-nested) statements to find the
+            // class + method containing the cursor.
+            Self::resolve_variable_in_statements(program.statements.iter(), &ctx)
+        }));
+
+        match result {
+            Ok(classes) => classes,
+            Err(_) => {
+                log::error!(
+                    "PHPantomLSP: parser panicked during variable resolution for '{}'",
+                    var_name
+                );
+                vec![]
+            }
+        }
     }
 
     pub(super) fn resolve_variable_in_statements<'b>(
@@ -1398,7 +1415,25 @@ impl Backend {
             }
 
             // Delegate all RHS resolution to the shared helper.
-            let resolved = Self::resolve_rhs_expression(assignment.rhs, ctx);
+            //
+            // Use the assignment's own start offset as cursor_offset so
+            // that any recursive variable resolution only considers
+            // assignments *before* this one.  Without this, a
+            // self-referential assignment like `$value = $value->value`
+            // would infinitely recurse: resolving the RHS `$value`
+            // would re-discover the same assignment, resolve its RHS
+            // again, and so on until a stack overflow crashes the
+            // process.
+            let rhs_ctx = VarResolutionCtx {
+                var_name: ctx.var_name,
+                current_class: ctx.current_class,
+                all_classes: ctx.all_classes,
+                content: ctx.content,
+                cursor_offset: assignment.span().start.offset,
+                class_loader: ctx.class_loader,
+                function_loader: ctx.function_loader,
+            };
+            let resolved = Self::resolve_rhs_expression(assignment.rhs, &rhs_ctx);
             push_results(results, resolved, conditional);
         }
     }
diff --git a/src/parser/ast_update.rs b/src/parser/ast_update.rs
@@ -7,6 +7,7 @@
 /// (`resolve_parent_class_names`, `resolve_name`) used to convert short
 /// class names to fully-qualified names.
 use std::collections::HashMap;
+use std::panic;
 
 use crate::docblock::types::is_scalar;
 
@@ -24,6 +25,34 @@ impl Backend {
     /// Update the ast_map, use_map, and namespace_map for a given file URI
     /// by parsing its content.
     pub fn update_ast(&self, uri: &str, content: &str) {
+        // The mago-syntax parser contains `unreachable!()` and `.expect()`
+        // calls that can panic on malformed PHP (e.g. partially-written
+        // heredocs/nowdocs, which are common while editing).  Wrap the
+        // entire parse + extraction in `catch_unwind` so a parser panic
+        // doesn't crash the LSP server and produce a zombie process.
+        //
+        // On panic the file is simply skipped — no maps are updated, and
+        // the user gets stale (but not missing) completions until the
+        // file is saved in a parseable state.
+        let content_owned = content.to_string();
+        let uri_owned = uri.to_string();
+
+        let result = panic::catch_unwind(panic::AssertUnwindSafe(|| {
+            self.update_ast_inner(&uri_owned, &content_owned);
+        }));
+
+        if result.is_err() {
+            log::error!(
+                "PHPantomLSP: parser panicked while parsing {}. Skipping file.",
+                uri
+            );
+        }
+    }
+
+    /// Inner implementation of [`update_ast`] that performs the actual
+    /// parsing and map updates.  Separated so that [`update_ast`] can
+    /// wrap the call in [`std::panic::catch_unwind`].
+    fn update_ast_inner(&self, uri: &str, content: &str) {
         let arena = Bump::new();
         let file_id = mago_database::file::FileId::new("input.php");
         let program = parse_file_content(&arena, file_id, content);
diff --git a/src/parser/mod.rs b/src/parser/mod.rs
@@ -151,46 +151,68 @@ impl Backend {
     /// Parse PHP source text and extract class information.
     /// Returns a Vec of ClassInfo for all classes found in the file.
     pub fn parse_php(&self, content: &str) -> Vec<ClassInfo> {
-        let arena = bumpalo::Bump::new();
-        let file_id = mago_database::file::FileId::new("input.php");
-        let program = mago_syntax::parser::parse_file_content(&arena, file_id, content);
-
-        let doc_ctx = DocblockCtx {
-            trivias: program.trivia.as_slice(),
-            content,
-        };
-
-        let mut classes = Vec::new();
-        Self::extract_classes_from_statements(
-            program.statements.iter(),
-            &mut classes,
-            Some(&doc_ctx),
-        );
-        classes
+        let content_owned = content.to_string();
+        let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+            let arena = bumpalo::Bump::new();
+            let file_id = mago_database::file::FileId::new("input.php");
+            let program = mago_syntax::parser::parse_file_content(&arena, file_id, &content_owned);
+
+            let doc_ctx = DocblockCtx {
+                trivias: program.trivia.as_slice(),
+                content: &content_owned,
+            };
+
+            let mut classes = Vec::new();
+            Self::extract_classes_from_statements(
+                program.statements.iter(),
+                &mut classes,
+                Some(&doc_ctx),
+            );
+            classes
+        }));
+
+        match result {
+            Ok(classes) => classes,
+            Err(_) => {
+                log::error!("PHPantomLSP: parser panicked in parse_php");
+                Vec::new()
+            }
+        }
     }
 
     /// Parse PHP source text and extract standalone function definitions.
     ///
     /// Returns a list of `FunctionInfo` for every `function` declaration
     /// found at the top level (or inside a namespace block).
     pub fn parse_functions(&self, content: &str) -> Vec<FunctionInfo> {
-        let arena = bumpalo::Bump::new();
-        let file_id = mago_database::file::FileId::new("input.php");
-        let program = mago_syntax::parser::parse_file_content(&arena, file_id, content);
-
-        let doc_ctx = DocblockCtx {
-            trivias: program.trivia.as_slice(),
-            content,
-        };
-
-        let mut functions = Vec::new();
-        Self::extract_functions_from_statements(
-            program.statements.iter(),
-            &mut functions,
-            &None,
-            Some(&doc_ctx),
-        );
-        functions
+        let content_owned = content.to_string();
+        let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+            let arena = bumpalo::Bump::new();
+            let file_id = mago_database::file::FileId::new("input.php");
+            let program = mago_syntax::parser::parse_file_content(&arena, file_id, &content_owned);
+
+            let doc_ctx = DocblockCtx {
+                trivias: program.trivia.as_slice(),
+                content: &content_owned,
+            };
+
+            let mut functions = Vec::new();
+            Self::extract_functions_from_statements(
+                program.statements.iter(),
+                &mut functions,
+                &None,
+                Some(&doc_ctx),
+            );
+            functions
+        }));
+
+        match result {
+            Ok(functions) => functions,
+            Err(_) => {
+                log::error!("PHPantomLSP: parser panicked in parse_functions");
+                Vec::new()
+            }
+        }
     }
 
     /// Parse PHP source text and extract constant names from `define()` calls.
@@ -199,13 +221,24 @@ impl Backend {
     /// call found at the top level, inside namespace blocks, block
     /// statements, or `if` guards.
     pub fn parse_defines(&self, content: &str) -> Vec<String> {
-        let arena = bumpalo::Bump::new();
-        let file_id = mago_database::file::FileId::new("input.php");
-        let program = mago_syntax::parser::parse_file_content(&arena, file_id, content);
+        let content_owned = content.to_string();
+        let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+            let arena = bumpalo::Bump::new();
+            let file_id = mago_database::file::FileId::new("input.php");
+            let program = mago_syntax::parser::parse_file_content(&arena, file_id, &content_owned);
 
-        let mut defines = Vec::new();
-        Self::extract_defines_from_statements(program.statements.iter(), &mut defines);
-        defines
+            let mut defines = Vec::new();
+            Self::extract_defines_from_statements(program.statements.iter(), &mut defines);
+            defines
+        }));
+
+        match result {
+            Ok(defines) => defines,
+            Err(_) => {
+                log::error!("PHPantomLSP: parser panicked in parse_defines");
+                Vec::new()
+            }
+        }
     }
 
     /// Parse PHP source text and extract `use` statement mappings.
@@ -224,24 +257,46 @@ impl Backend {
     ///     (function / const imports are skipped — we only track classes)
     ///   - Use statements inside namespace bodies
     pub fn parse_use_statements(&self, content: &str) -> std::collections::HashMap<String, String> {
-        let arena = bumpalo::Bump::new();
-        let file_id = mago_database::file::FileId::new("input.php");
-        let program = mago_syntax::parser::parse_file_content(&arena, file_id, content);
+        let content_owned = content.to_string();
+        let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+            let arena = bumpalo::Bump::new();
+            let file_id = mago_database::file::FileId::new("input.php");
+            let program = mago_syntax::parser::parse_file_content(&arena, file_id, &content_owned);
 
-        let mut use_map = std::collections::HashMap::new();
-        Self::extract_use_statements_from_statements(program.statements.iter(), &mut use_map);
-        use_map
+            let mut use_map = std::collections::HashMap::new();
+            Self::extract_use_statements_from_statements(program.statements.iter(), &mut use_map);
+            use_map
+        }));
+
+        match result {
+            Ok(use_map) => use_map,
+            Err(_) => {
+                log::error!("PHPantomLSP: parser panicked in parse_use_statements");
+                std::collections::HashMap::new()
+            }
+        }
     }
 
     /// Parse PHP source text and extract the declared namespace (if any).
     ///
     /// Returns the namespace string (e.g. `"Klarna\Rest\Checkout"`) or
     /// `None` if the file has no namespace declaration.
     pub fn parse_namespace(&self, content: &str) -> Option<String> {
-        let arena = bumpalo::Bump::new();
-        let file_id = mago_database::file::FileId::new("input.php");
-        let program = mago_syntax::parser::parse_file_content(&arena, file_id, content);
+        let content_owned = content.to_string();
+        let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+            let arena = bumpalo::Bump::new();
+            let file_id = mago_database::file::FileId::new("input.php");
+            let program = mago_syntax::parser::parse_file_content(&arena, file_id, &content_owned);
+
+            Self::extract_namespace_from_statements(program.statements.iter())
+        }));
 
-        Self::extract_namespace_from_statements(program.statements.iter())
+        match result {
+            Ok(ns) => ns,
+            Err(_) => {
+                log::error!("PHPantomLSP: parser panicked in parse_namespace");
+                None
+            }
+        }
     }
 }
diff --git a/tests/completion_enums.rs b/tests/completion_enums.rs
