From c3a6c963e5f18677a3680e6066a2b9a5b90dfd21 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 15 Sep 2022 21:19:57 +0200 Subject: [PATCH] Amalgamate file changes for the same file ids in process_changes When receiving multiple change events for a single file id where the last change is a delete the server panics, as it tries to access the file contents of a deleted file. This occurs due to the VFS changes and the in memory file contents being updated immediately, while `process_changes` processes the events afterwards in sequence which no longer works as it will only observe the final file contents. By folding these events together, we will no longer try to process these intermediate changes, as they aren't relevant anyways. Potentially fixes https://github.com/rust-lang/rust-analyzer/issues/13236 --- crates/rust-analyzer/src/global_state.rs | 39 +++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 92df4d70fd..55fa616d50 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -185,11 +185,48 @@ impl GlobalState { let (change, changed_files) = { let mut change = Change::new(); let (vfs, line_endings_map) = &mut *self.vfs.write(); - let changed_files = vfs.take_changes(); + let mut changed_files = vfs.take_changes(); if changed_files.is_empty() { return false; } + // important: this needs to be a stable sort, the order between changes is relevant + // for the same file ids + changed_files.sort_by_key(|file| file.file_id); + // We need to fix up the changed events a bit, if we have a create or modify for a file + // id that is followed by a delete we actually no longer observe the file text from the + // create or modify which may cause problems later on + changed_files.dedup_by(|a, b| { + use vfs::ChangeKind::*; + + if a.file_id != b.file_id { + return false; + } + + match (a.change_kind, b.change_kind) { + // duplicate can be merged + (Create, Create) | (Modify, Modify) | (Delete, Delete) => true, + // just leave the create, modify is irrelevant + (Create, Modify) => { + std::mem::swap(a, b); + true + } + // modify becomes irrelevant if the file is deleted + (Modify, Delete) => true, + // we should fully remove this occurrence, + // but leaving just a delete works as well + (Create, Delete) => true, + // this is equivalent to a modify + (Delete, Create) => { + a.change_kind = Modify; + true + } + // can't really occur + (Modify, Create) => false, + (Delete, Modify) => false, + } + }); + for file in &changed_files { if let Some(path) = vfs.file_path(file.file_id).as_path() { let path = path.to_path_buf();