Preserve class_index and fqn_index after didClose to fix GTD on closed

files

Author: AJenbo

docs/todo.md

@@ -25,6 +25,7 @@ within the same impact tier.
 
 | #   | Item                                                                                                                    | Impact      | Effort      |
 | --- | ----------------------------------------------------------------------------------------------------------------------- | ----------- | ------ |
+| P13 | [Tiered storage: drop per-file maps for non-open files](todo/performance.md#p13-tiered-storage-drop-per-file-maps-for-non-open-files) | Medium-High | Medium-High |
 | D10 | [PHPMD diagnostic proxy](todo/diagnostics.md#d10-phpmd-diagnostic-proxy)                                                | Low         | Medium      |
 |     | **Release 0.8.0**                                                                                                       |             |             |
 

src/definition/member/file_lookup.rs

@@ -111,7 +111,12 @@ impl Backend {
                     .find(|(u, classes)| class_in_file(u, classes))
                     .map(|(u, _)| u.clone())
             }
-        }?;
+        }
+        // Fallback: the target file may have been closed (didClose clears
+        // ast_map).  Check class_index which survives close (issue #99).
+        .or_else(|| {
+            self.class_index.read().get(class_name).cloned()
+        })?;
 
         // Get the file content.
         let file_content = if uri == current_uri {

src/definition/resolve.rs

@@ -13,6 +13,7 @@
 /// Variable definition resolution (`$var` → most recent assignment /
 /// declaration) is handled by the sibling [`super::variable`] module.
 use std::collections::HashMap;
+use std::sync::Arc;
 
 use crate::symbol_map::VarDefKind;
 use tower_lsp::lsp_types::*;
@@ -683,7 +684,18 @@ impl Backend {
     ) -> Option<Location> {
         let sn = short_name(fqn);
 
-        let classes = self.ast_map.read().get(target_uri).cloned()?;
+        let classes = self.ast_map.read().get(target_uri).cloned().or_else(|| {
+            // The target file may have been closed (didClose clears
+            // ast_map).  Try fqn_index which retains ClassInfo across
+            // close, then fall back to re-parsing from disk (issue #99).
+            if let Some(cls) = self.fqn_index.read().get(fqn) {
+                return Some(vec![Arc::clone(cls)]);
+            }
+            let file_path = Url::parse(target_uri)
+                .ok()
+                .and_then(|u| u.to_file_path().ok())?;
+            self.parse_and_cache_file(&file_path)
+        })?;
 
         // Match by short name + namespace, same logic as
         // `find_definition_in_ast_map`.

src/util.rs

@@ -1670,37 +1670,20 @@ impl Backend {
     /// method *reads* the three maps, this method *clears* them for a given URI.
     /// Called from `did_close` to clean up state when a file is closed.
     pub(crate) fn clear_file_maps(&self, uri: &str) {
-        // Collect FQNs for targeted class_index removal BEFORE clearing
-        // ast_map — O(file_classes) instead of O(total_class_index).
-        let old_fqns: Vec<String> = self
-            .ast_map
-            .read()
-            .get(uri)
-            .map(|classes| {
-                classes
-                    .iter()
-                    .filter(|c| !c.name.starts_with("__anonymous@"))
-                    .map(|c| match &c.file_namespace {
-                        Some(ns) if !ns.is_empty() => format!("{}\\{}", ns, c.name),
-                        _ => c.name.to_string(),
-                    })
-                    .collect()
-            })
-            .unwrap_or_default();
-
+        // Drop per-file maps that are only needed while the file is
+        // open.  ast_map is redundant with fqn_index once indexing is
+        // complete — GTD falls back to fqn_index + parse_and_cache_file
+        // when the ast_map entry is missing.
         self.ast_map.write().remove(uri);
         self.symbol_maps.write().remove(uri);
         self.use_map.write().remove(uri);
         self.resolved_names.write().remove(uri);
         self.namespace_map.write().remove(uri);
-        // Remove class_index entries that belonged to this file so
-        // stale FQNs don't linger after the file is closed.
-        if !old_fqns.is_empty() {
-            let mut idx = self.class_index.write();
-            for fqn in &old_fqns {
-                idx.remove(fqn);
-            }
-        }
+        // NOTE: We intentionally keep class_index and fqn_index intact.
+        // class_index maps FQN → URI so GTD can locate the file, and
+        // fqn_index keeps the full ClassInfo for cross-file resolution.
+        // The file will be re-parsed from disk on next access via
+        // parse_and_cache_file when needed (issue #99).
     }
 
     pub(crate) async fn log(&self, typ: MessageType, message: String) {

tests/integration/definition_classes.rs

@@ -199,6 +199,129 @@ async fn test_goto_definition_cross_file_psr4() {
     }
 }
 
+#[tokio::test]
+async fn test_goto_definition_after_target_did_close() {
+    // Regression test for issue #99: go-to-definition stops working
+    // after the target file is closed via textDocument/didClose.
+    let backend = create_test_backend();
+    let dir = tempfile::tempdir().expect("failed to create temp dir");
+    let src_dir = dir.path().join("src");
+    std::fs::create_dir_all(&src_dir).unwrap();
+
+    let text_b = concat!(
+        "<?php\n",
+        "namespace App;\n",
+        "\n",
+        "class ClassB {\n",
+        "    public function doSomething() {}\n",
+        "}\n",
+    );
+    std::fs::write(src_dir.join("ClassB.php"), text_b).unwrap();
+
+    let uri_b = Url::from_file_path(src_dir.join("ClassB.php")).unwrap();
+
+    // Open ClassB so it gets indexed.
+    backend
+        .did_open(DidOpenTextDocumentParams {
+            text_document: TextDocumentItem {
+                uri: uri_b.clone(),
+                language_id: "php".to_string(),
+                version: 1,
+                text: text_b.to_string(),
+            },
+        })
+        .await;
+
+    // Close ClassB — simulates VS Code peek preview closing.
+    backend
+        .did_close(DidCloseTextDocumentParams {
+            text_document: TextDocumentIdentifier { uri: uri_b.clone() },
+        })
+        .await;
+
+    // Open ClassA which references ClassB.
+    let uri_a = Url::from_file_path(src_dir.join("ClassA.php")).unwrap();
+    let text_a = concat!(
+        "<?php\n",
+        "namespace App;\n",
+        "\n",
+        "class ClassA {\n",
+        "    public function test() {\n",
+        "        $b = new ClassB();\n",
+        "        $b->doSomething();\n",
+        "    }\n",
+        "}\n",
+    );
+    backend
+        .did_open(DidOpenTextDocumentParams {
+            text_document: TextDocumentItem {
+                uri: uri_a.clone(),
+                language_id: "php".to_string(),
+                version: 1,
+                text: text_a.to_string(),
+            },
+        })
+        .await;
+
+    // Go-to-definition on "ClassB" (line 5, char 21 = inside "ClassB")
+    let params = GotoDefinitionParams {
+        text_document_position_params: TextDocumentPositionParams {
+            text_document: TextDocumentIdentifier { uri: uri_a.clone() },
+            position: Position {
+                line: 5,
+                character: 21,
+            },
+        },
+        work_done_progress_params: WorkDoneProgressParams::default(),
+        partial_result_params: PartialResultParams::default(),
+    };
+
+    let result = backend.goto_definition(params).await.unwrap();
+    assert!(
+        result.is_some(),
+        "Go-to-definition should work after target file is closed (issue #99)"
+    );
+
+    match result.unwrap() {
+        GotoDefinitionResponse::Scalar(location) => {
+            assert_eq!(
+                location.uri, uri_b,
+                "Should jump to ClassB.php"
+            );
+        }
+        other => panic!("Expected Scalar location, got: {:?}", other),
+    }
+
+    // Also test member access: $b->doSomething() (line 6, char 14)
+    let params = GotoDefinitionParams {
+        text_document_position_params: TextDocumentPositionParams {
+            text_document: TextDocumentIdentifier { uri: uri_a },
+            position: Position {
+                line: 6,
+                character: 14,
+            },
+        },
+        work_done_progress_params: WorkDoneProgressParams::default(),
+        partial_result_params: PartialResultParams::default(),
+    };
+
+    let result = backend.goto_definition(params).await.unwrap();
+    assert!(
+        result.is_some(),
+        "Go-to-definition on member should work after target file is closed (issue #99)"
+    );
+
+    match result.unwrap() {
+        GotoDefinitionResponse::Scalar(location) => {
+            assert_eq!(
+                location.uri, uri_b,
+                "Member definition should jump to ClassB.php"
+            );
+        }
+        other => panic!("Expected Scalar location for member, got: {:?}", other),
+    }
+}
+
 #[tokio::test]
 async fn test_goto_definition_cross_file_with_use_statement() {
     let (backend, _dir) = create_psr4_workspace(

tests/integration/server_lifecycle.rs

@@ -168,10 +168,15 @@ async fn test_did_close_removes_file() {
     };
     backend.did_close(close_params).await;
 
-    // AST map entry should be removed after close
+    // ast_map is dropped on close to free memory, but class_index
+    // is kept so GTD can still locate the file.
     assert!(
         backend.get_classes_for_uri(uri.as_ref()).is_none(),
-        "After close, AST map should not have an entry"
+        "After close, AST map should be cleared"
+    );
+    assert!(
+        backend.class_index().read().contains_key("Z"),
+        "class_index should retain entries after close"
     );
 }
 
@@ -283,7 +288,7 @@ async fn test_did_close_cleans_up_ast_map() {
     };
     backend.did_close(close_params).await;
 
-    // Verify ast_map entry was removed
+    // Verify ast_map entry is dropped on close to free memory
     assert!(
         backend.get_classes_for_uri(uri.as_ref()).is_none(),
         "ast_map should be cleaned up after did_close"