Auto merge of #15047 - Veykril:crate-graph-root-deps, r=Veykril

internal: Record file dependencies in crate graph construction

Should fix the bug mentioned in #8623 where removing a crate root file will panic. I'm not too happy with the way this is done here but I can't think of a better way right now.

Author: bors

crates/ide/src/syntax_highlighting.rs

@@ -243,6 +243,9 @@ fn traverse(
     let mut attr_or_derive_item = None;
     let mut current_macro: Option<ast::Macro> = None;
     let mut macro_highlighter = MacroHighlighter::default();
+
+    // FIXME: these are not perfectly accurate, we determine them by the real file's syntax tree
+    // an an attribute nested in a macro call will not emit `inside_attribute`
     let mut inside_attribute = false;
     let mut inside_macro_call = false;
 

crates/rust-analyzer/src/global_state.rs

@@ -14,7 +14,7 @@ use nohash_hasher::IntMap;
 use parking_lot::{Mutex, RwLock};
 use proc_macro_api::ProcMacroServer;
 use project_model::{CargoWorkspace, ProjectWorkspace, Target, WorkspaceBuildScripts};
-use rustc_hash::FxHashMap;
+use rustc_hash::{FxHashMap, FxHashSet};
 use triomphe::Arc;
 use vfs::AnchoredPathBuf;
 
@@ -112,9 +112,11 @@ pub(crate) struct GlobalState {
     /// the user just adds comments or whitespace to Cargo.toml, we do not want
     /// to invalidate any salsa caches.
     pub(crate) workspaces: Arc<Vec<ProjectWorkspace>>,
+    pub(crate) crate_graph_file_dependencies: FxHashSet<vfs::VfsPath>,
 
     // op queues
-    pub(crate) fetch_workspaces_queue: OpQueue<(), Option<Vec<anyhow::Result<ProjectWorkspace>>>>,
+    pub(crate) fetch_workspaces_queue:
+        OpQueue<bool, Option<(Vec<anyhow::Result<ProjectWorkspace>>, bool)>>,
     pub(crate) fetch_build_data_queue:
         OpQueue<(), (Arc<Vec<ProjectWorkspace>>, Vec<anyhow::Result<WorkspaceBuildScripts>>)>,
     pub(crate) fetch_proc_macros_queue: OpQueue<Vec<ProcMacroPaths>, bool>,
@@ -196,6 +198,7 @@ impl GlobalState {
             vfs_progress_n_done: 0,
 
             workspaces: Arc::new(Vec::new()),
+            crate_graph_file_dependencies: FxHashSet::default(),
             fetch_workspaces_queue: OpQueue::default(),
             fetch_build_data_queue: OpQueue::default(),
             fetch_proc_macros_queue: OpQueue::default(),
@@ -209,10 +212,9 @@ impl GlobalState {
 
     pub(crate) fn process_changes(&mut self) -> bool {
         let _p = profile::span("GlobalState::process_changes");
-        let mut workspace_structure_change = None;
 
         let mut file_changes = FxHashMap::default();
-        let (change, changed_files) = {
+        let (change, changed_files, workspace_structure_change) = {
             let mut change = Change::new();
             let (vfs, line_endings_map) = &mut *self.vfs.write();
             let changed_files = vfs.take_changes();
@@ -267,16 +269,20 @@ impl GlobalState {
                 .map(|(file_id, (change_kind, _))| vfs::ChangedFile { file_id, change_kind })
                 .collect();
 
+            let mut workspace_structure_change = None;
             // A file was added or deleted
             let mut has_structure_changes = false;
             for file in &changed_files {
-                if let Some(path) = vfs.file_path(file.file_id).as_path() {
+                let vfs_path = &vfs.file_path(file.file_id);
+                if let Some(path) = vfs_path.as_path() {
                     let path = path.to_path_buf();
                     if reload::should_refresh_for_change(&path, file.change_kind) {
-                        workspace_structure_change = Some(path);
+                        workspace_structure_change = Some((path.clone(), false));
                     }
                     if file.is_created_or_deleted() {
                         has_structure_changes = true;
+                        workspace_structure_change =
+                            Some((path, self.crate_graph_file_dependencies.contains(vfs_path)));
                     }
                 }
 
@@ -301,7 +307,7 @@ impl GlobalState {
                 let roots = self.source_root_config.partition(vfs);
                 change.set_roots(roots);
             }
-            (change, changed_files)
+            (change, changed_files, workspace_structure_change)
         };
 
         self.analysis_host.apply_change(change);
@@ -311,9 +317,11 @@ impl GlobalState {
             // FIXME: ideally we should only trigger a workspace fetch for non-library changes
             // but something's going wrong with the source root business when we add a new local
             // crate see https://github.com/rust-lang/rust-analyzer/issues/13029
-            if let Some(path) = workspace_structure_change {
-                self.fetch_workspaces_queue
-                    .request_op(format!("workspace vfs file change: {}", path.display()), ());
+            if let Some((path, force_crate_graph_reload)) = workspace_structure_change {
+                self.fetch_workspaces_queue.request_op(
+                    format!("workspace vfs file change: {}", path.display()),
+                    force_crate_graph_reload,
+                );
             }
             self.proc_macro_changed =
                 changed_files.iter().filter(|file| !file.is_created_or_deleted()).any(|file| {

crates/rust-analyzer/src/handlers/notification.rs

@@ -127,7 +127,7 @@ pub(crate) fn handle_did_save_text_document(
             if reload::should_refresh_for_change(abs_path, ChangeKind::Modify) {
                 state
                     .fetch_workspaces_queue
-                    .request_op(format!("DidSaveTextDocument {}", abs_path.display()), ());
+                    .request_op(format!("DidSaveTextDocument {}", abs_path.display()), false);
             }
         }
 
@@ -205,7 +205,7 @@ pub(crate) fn handle_did_change_workspace_folders(
 
     if !config.has_linked_projects() && config.detached_files().is_empty() {
         config.rediscover_workspaces();
-        state.fetch_workspaces_queue.request_op("client workspaces changed".to_string(), ())
+        state.fetch_workspaces_queue.request_op("client workspaces changed".to_string(), false)
     }
 
     Ok(())

crates/rust-analyzer/src/handlers/request.rs

@@ -51,7 +51,7 @@ pub(crate) fn handle_workspace_reload(state: &mut GlobalState, _: ()) -> Result<
     state.proc_macro_clients = Arc::from(Vec::new());
     state.proc_macro_changed = false;
 
-    state.fetch_workspaces_queue.request_op("reload workspace request".to_string(), ());
+    state.fetch_workspaces_queue.request_op("reload workspace request".to_string(), false);
     Ok(())
 }
 

crates/rust-analyzer/src/main_loop.rs

@@ -115,9 +115,11 @@ impl GlobalState {
             self.register_did_save_capability();
         }
 
-        self.fetch_workspaces_queue.request_op("startup".to_string(), ());
-        if let Some((cause, ())) = self.fetch_workspaces_queue.should_start_op() {
-            self.fetch_workspaces(cause);
+        self.fetch_workspaces_queue.request_op("startup".to_string(), false);
+        if let Some((cause, force_crate_graph_reload)) =
+            self.fetch_workspaces_queue.should_start_op()
+        {
+            self.fetch_workspaces(cause, force_crate_graph_reload);
         }
 
         while let Some(event) = self.next_event(&inbox) {
@@ -367,8 +369,10 @@ impl GlobalState {
         }
 
         if self.config.cargo_autoreload() {
-            if let Some((cause, ())) = self.fetch_workspaces_queue.should_start_op() {
-                self.fetch_workspaces(cause);
+            if let Some((cause, force_crate_graph_reload)) =
+                self.fetch_workspaces_queue.should_start_op()
+            {
+                self.fetch_workspaces(cause, force_crate_graph_reload);
             }
         }
 
@@ -471,8 +475,9 @@ impl GlobalState {
                 let (state, msg) = match progress {
                     ProjectWorkspaceProgress::Begin => (Progress::Begin, None),
                     ProjectWorkspaceProgress::Report(msg) => (Progress::Report, Some(msg)),
-                    ProjectWorkspaceProgress::End(workspaces) => {
-                        self.fetch_workspaces_queue.op_completed(Some(workspaces));
+                    ProjectWorkspaceProgress::End(workspaces, force_reload_crate_graph) => {
+                        self.fetch_workspaces_queue
+                            .op_completed(Some((workspaces, force_reload_crate_graph)));
                         if let Err(e) = self.fetch_workspace_error() {
                             tracing::error!("FetchWorkspaceError:\n{e}");
                         }

crates/rust-analyzer/src/reload.rs

@@ -27,6 +27,7 @@ use ide_db::{
 use itertools::Itertools;
 use proc_macro_api::{MacroDylib, ProcMacroServer};
 use project_model::{PackageRoot, ProjectWorkspace, WorkspaceBuildScripts};
+use rustc_hash::FxHashSet;
 use stdx::{format_to, thread::ThreadIntent};
 use syntax::SmolStr;
 use triomphe::Arc;
@@ -46,7 +47,7 @@ use ::tt::token_id as tt;
 pub(crate) enum ProjectWorkspaceProgress {
     Begin,
     Report(String),
-    End(Vec<anyhow::Result<ProjectWorkspace>>),
+    End(Vec<anyhow::Result<ProjectWorkspace>>, bool),
 }
 
 #[derive(Debug)]
@@ -85,7 +86,7 @@ impl GlobalState {
             );
         }
         if self.config.linked_projects() != old_config.linked_projects() {
-            self.fetch_workspaces_queue.request_op("linked projects changed".to_string(), ())
+            self.fetch_workspaces_queue.request_op("linked projects changed".to_string(), false)
         } else if self.config.flycheck() != old_config.flycheck() {
             self.reload_flycheck();
         }
@@ -182,7 +183,7 @@ impl GlobalState {
         status
     }
 
-    pub(crate) fn fetch_workspaces(&mut self, cause: Cause) {
+    pub(crate) fn fetch_workspaces(&mut self, cause: Cause, force_crate_graph_reload: bool) {
         tracing::info!(%cause, "will fetch workspaces");
 
         self.task_pool.handle.spawn_with_sender(ThreadIntent::Worker, {
@@ -250,7 +251,10 @@ impl GlobalState {
 
                 tracing::info!("did fetch workspaces {:?}", workspaces);
                 sender
-                    .send(Task::FetchWorkspace(ProjectWorkspaceProgress::End(workspaces)))
+                    .send(Task::FetchWorkspace(ProjectWorkspaceProgress::End(
+                        workspaces,
+                        force_crate_graph_reload,
+                    )))
                     .unwrap();
             }
         });
@@ -336,15 +340,19 @@ impl GlobalState {
         let _p = profile::span("GlobalState::switch_workspaces");
         tracing::info!(%cause, "will switch workspaces");
 
+        let Some((workspaces, force_reload_crate_graph)) = self.fetch_workspaces_queue.last_op_result() else { return; };
+
         if let Err(_) = self.fetch_workspace_error() {
             if !self.workspaces.is_empty() {
+                if *force_reload_crate_graph {
+                    self.recreate_crate_graph(cause);
+                }
                 // It only makes sense to switch to a partially broken workspace
                 // if we don't have any workspace at all yet.
                 return;
             }
         }
 
-        let Some(workspaces) = self.fetch_workspaces_queue.last_op_result() else { return; };
         let workspaces =
             workspaces.iter().filter_map(|res| res.as_ref().ok().cloned()).collect::<Vec<_>>();
 
@@ -373,6 +381,9 @@ impl GlobalState {
                 self.workspaces = Arc::new(workspaces);
             } else {
                 tracing::info!("build scripts do not match the version of the active workspace");
+                if *force_reload_crate_graph {
+                    self.recreate_crate_graph(cause);
+                }
                 // Current build scripts do not match the version of the active
                 // workspace, so there's nothing for us to update.
                 return;
@@ -467,13 +478,24 @@ impl GlobalState {
         });
         self.source_root_config = project_folders.source_root_config;
 
+        self.recreate_crate_graph(cause);
+
+        tracing::info!("did switch workspaces");
+    }
+
+    fn recreate_crate_graph(&mut self, cause: String) {
         // Create crate graph from all the workspaces
-        let (crate_graph, proc_macro_paths) = {
+        let (crate_graph, proc_macro_paths, crate_graph_file_dependencies) = {
             let vfs = &mut self.vfs.write().0;
             let loader = &mut self.loader;
+            // crate graph construction relies on these paths, record them so when one of them gets
+            // deleted or created we trigger a reconstruction of the crate graph
+            let mut crate_graph_file_dependencies = FxHashSet::default();
+
             let mut load = |path: &AbsPath| {
                 let _p = profile::span("switch_workspaces::load");
                 let vfs_path = vfs::VfsPath::from(path.to_path_buf());
+                crate_graph_file_dependencies.insert(vfs_path.clone());
                 match vfs.file_id(&vfs_path) {
                     Some(file_id) => Some(file_id),
                     None => {
@@ -494,26 +516,25 @@ impl GlobalState {
                 crate_graph.extend(other, &mut crate_proc_macros);
                 proc_macros.push(crate_proc_macros);
             }
-            (crate_graph, proc_macros)
+            (crate_graph, proc_macros, crate_graph_file_dependencies)
         };
-        let mut change = Change::new();
 
         if self.config.expand_proc_macros() {
             self.fetch_proc_macros_queue.request_op(cause, proc_macro_paths);
         }
+        let mut change = Change::new();
         change.set_crate_graph(crate_graph);
         self.analysis_host.apply_change(change);
+        self.crate_graph_file_dependencies = crate_graph_file_dependencies;
         self.process_changes();
 
         self.reload_flycheck();
-
-        tracing::info!("did switch workspaces");
     }
 
     pub(super) fn fetch_workspace_error(&self) -> Result<(), String> {
         let mut buf = String::new();
 
-        let Some(last_op_result) = self.fetch_workspaces_queue.last_op_result() else { return Ok(()) };
+        let Some((last_op_result, _)) = self.fetch_workspaces_queue.last_op_result() else { return Ok(()) };
         if last_op_result.is_empty() {
             stdx::format_to!(buf, "rust-analyzer failed to discover workspace");
         } else {