Move ide::AnalysisChange -> base_db::Change

This seems like a better factoring logically; ideally, clients shouldn't touch
`set_` methods of the database directly. Additionally, I think this
should remove the unfortunate duplication in fixture code.

Author: matklad

crates/base_db/src/change.rs

@@ -0,0 +1,97 @@
+//! Defines a unit of change that can applied to the database to get the next
+//! state. Changes are transactional.
+
+use std::{fmt, sync::Arc};
+
+use rustc_hash::FxHashSet;
+use salsa::Durability;
+use vfs::FileId;
+
+use crate::{CrateGraph, SourceDatabaseExt, SourceRoot, SourceRootId};
+
+/// Encapsulate a bunch of raw `.set` calls on the database.
+#[derive(Default)]
+pub struct Change {
+    pub roots: Option<Vec<SourceRoot>>,
+    pub files_changed: Vec<(FileId, Option<Arc<String>>)>,
+    pub crate_graph: Option<CrateGraph>,
+}
+
+impl fmt::Debug for Change {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        let mut d = fmt.debug_struct("AnalysisChange");
+        if let Some(roots) = &self.roots {
+            d.field("roots", roots);
+        }
+        if !self.files_changed.is_empty() {
+            d.field("files_changed", &self.files_changed.len());
+        }
+        if self.crate_graph.is_some() {
+            d.field("crate_graph", &self.crate_graph);
+        }
+        d.finish()
+    }
+}
+
+impl Change {
+    pub fn new() -> Change {
+        Change::default()
+    }
+
+    pub fn set_roots(&mut self, roots: Vec<SourceRoot>) {
+        self.roots = Some(roots);
+    }
+
+    pub fn change_file(&mut self, file_id: FileId, new_text: Option<Arc<String>>) {
+        self.files_changed.push((file_id, new_text))
+    }
+
+    pub fn set_crate_graph(&mut self, graph: CrateGraph) {
+        self.crate_graph = Some(graph);
+    }
+
+    pub fn apply(self, db: &mut dyn SourceDatabaseExt) {
+        let _p = profile::span("RootDatabase::apply_change");
+        // db.request_cancellation();
+        // log::info!("apply_change {:?}", change);
+        if let Some(roots) = self.roots {
+            let mut local_roots = FxHashSet::default();
+            let mut library_roots = FxHashSet::default();
+            for (idx, root) in roots.into_iter().enumerate() {
+                let root_id = SourceRootId(idx as u32);
+                let durability = durability(&root);
+                if root.is_library {
+                    library_roots.insert(root_id);
+                } else {
+                    local_roots.insert(root_id);
+                }
+                for file_id in root.iter() {
+                    db.set_file_source_root_with_durability(file_id, root_id, durability);
+                }
+                db.set_source_root_with_durability(root_id, Arc::new(root), durability);
+            }
+            // db.set_local_roots_with_durability(Arc::new(local_roots), Durability::HIGH);
+            // db.set_library_roots_with_durability(Arc::new(library_roots), Durability::HIGH);
+        }
+
+        for (file_id, text) in self.files_changed {
+            let source_root_id = db.file_source_root(file_id);
+            let source_root = db.source_root(source_root_id);
+            let durability = durability(&source_root);
+            // XXX: can't actually remove the file, just reset the text
+            let text = text.unwrap_or_default();
+            db.set_file_text_with_durability(file_id, text, durability)
+        }
+        if let Some(crate_graph) = self.crate_graph {
+            db.set_crate_graph_with_durability(Arc::new(crate_graph), Durability::HIGH)
+        }
+    }
+}
+
+fn durability(source_root: &SourceRoot) -> Durability {
+    if source_root.is_library {
+        Durability::HIGH
+    } else {
+        Durability::LOW
+    }
+}

crates/base_db/src/lib.rs

@@ -1,6 +1,7 @@
 //! base_db defines basic database traits. The concrete DB is defined by ide.
 mod cancellation;
 mod input;
+mod change;
 pub mod fixture;
 
 use std::{panic, sync::Arc};
@@ -10,6 +11,7 @@ use syntax::{ast, Parse, SourceFile, TextRange, TextSize};
 
 pub use crate::{
     cancellation::Canceled,
+    change::Change,
     input::{
         CrateData, CrateGraph, CrateId, CrateName, Dependency, Edition, Env, FileId, ProcMacroId,
         SourceRoot, SourceRootId,

crates/ide/src/lib.rs

@@ -87,12 +87,11 @@ pub use assists::{
     utils::MergeBehaviour, Assist, AssistConfig, AssistId, AssistKind, ResolvedAssist,
 };
 pub use base_db::{
-    Canceled, CrateGraph, CrateId, Edition, FileId, FilePosition, FileRange, SourceRoot,
+    Canceled, Change, CrateGraph, CrateId, Edition, FileId, FilePosition, FileRange, SourceRoot,
     SourceRootId,
 };
 pub use hir::{Documentation, Semantics};
 pub use ide_db::{
-    change::AnalysisChange,
     label::Label,
     line_index::{LineCol, LineIndex},
     search::SearchScope,
@@ -141,7 +140,7 @@ impl AnalysisHost {
 
     /// Applies changes to the current state of the world. If there are
     /// outstanding snapshots, they will be canceled.
-    pub fn apply_change(&mut self, change: AnalysisChange) {
+    pub fn apply_change(&mut self, change: Change) {
         self.db.apply_change(change)
     }
 
@@ -195,7 +194,7 @@ impl Analysis {
         file_set.insert(file_id, VfsPath::new_virtual_path("/main.rs".to_string()));
         let source_root = SourceRoot::new_local(file_set);
 
-        let mut change = AnalysisChange::new();
+        let mut change = Change::new();
         change.set_roots(vec![source_root]);
         let mut crate_graph = CrateGraph::default();
         // FIXME: cfg options

crates/ide/src/mock_analysis.rs

@@ -7,9 +7,7 @@ use test_utils::{
     extract_annotations, extract_range_or_offset, Fixture, RangeOrOffset, CURSOR_MARKER,
 };
 
-use crate::{
-    Analysis, AnalysisChange, AnalysisHost, CrateGraph, Edition, FileId, FilePosition, FileRange,
-};
+use crate::{Analysis, AnalysisHost, Change, CrateGraph, Edition, FileId, FilePosition, FileRange};
 
 /// Mock analysis is used in test to bootstrap an AnalysisHost/Analysis
 /// from a set of in-memory files.
@@ -95,7 +93,7 @@ impl MockAnalysis {
     }
     pub(crate) fn analysis_host(self) -> AnalysisHost {
         let mut host = AnalysisHost::default();
-        let mut change = AnalysisChange::new();
+        let mut change = Change::new();
         let mut file_set = FileSet::default();
         let mut crate_graph = CrateGraph::default();
         let mut root_crate = None;

crates/ide/src/parent_module.rs

@@ -69,7 +69,7 @@ mod tests {
 
     use crate::{
         mock_analysis::{analysis_and_position, MockAnalysis},
-        AnalysisChange, CrateGraph,
+        Change, CrateGraph,
         Edition::Edition2018,
     };
 
@@ -146,7 +146,7 @@ mod foo;
             Env::default(),
             Default::default(),
         );
-        let mut change = AnalysisChange::new();
+        let mut change = Change::new();
         change.set_crate_graph(crate_graph);
         host.apply_change(change);
 

crates/ide_db/src/change.rs renamed to crates/ide_db/src/apply_change.rs

@@ -1,58 +1,16 @@
-//! Defines a unit of change that can applied to a state of IDE to get the next
-//! state. Changes are transactional.
+//! Applies changes to the IDE state transactionally.
 
 use std::{fmt, sync::Arc};
 
 use base_db::{
     salsa::{Database, Durability, SweepStrategy},
-    CrateGraph, FileId, SourceDatabase, SourceDatabaseExt, SourceRoot, SourceRootId,
+    Change, FileId, SourceRootId,
 };
 use profile::{memory_usage, Bytes};
 use rustc_hash::FxHashSet;
 
 use crate::{symbol_index::SymbolsDatabase, RootDatabase};
 
-#[derive(Default)]
-pub struct AnalysisChange {
-    roots: Option<Vec<SourceRoot>>,
-    files_changed: Vec<(FileId, Option<Arc<String>>)>,
-    crate_graph: Option<CrateGraph>,
-}
-
-impl fmt::Debug for AnalysisChange {
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        let mut d = fmt.debug_struct("AnalysisChange");
-        if let Some(roots) = &self.roots {
-            d.field("roots", roots);
-        }
-        if !self.files_changed.is_empty() {
-            d.field("files_changed", &self.files_changed.len());
-        }
-        if self.crate_graph.is_some() {
-            d.field("crate_graph", &self.crate_graph);
-        }
-        d.finish()
-    }
-}
-
-impl AnalysisChange {
-    pub fn new() -> AnalysisChange {
-        AnalysisChange::default()
-    }
-
-    pub fn set_roots(&mut self, roots: Vec<SourceRoot>) {
-        self.roots = Some(roots);
-    }
-
-    pub fn change_file(&mut self, file_id: FileId, new_text: Option<Arc<String>>) {
-        self.files_changed.push((file_id, new_text))
-    }
-
-    pub fn set_crate_graph(&mut self, graph: CrateGraph) {
-        self.crate_graph = Some(graph);
-    }
-}
-
 #[derive(Debug)]
 struct AddFile {
     file_id: FileId,
@@ -87,41 +45,25 @@ impl RootDatabase {
         self.salsa_runtime_mut().synthetic_write(Durability::LOW);
     }
 
-    pub fn apply_change(&mut self, change: AnalysisChange) {
+    pub fn apply_change(&mut self, change: Change) {
         let _p = profile::span("RootDatabase::apply_change");
         self.request_cancellation();
         log::info!("apply_change {:?}", change);
-        if let Some(roots) = change.roots {
+        if let Some(roots) = &change.roots {
             let mut local_roots = FxHashSet::default();
             let mut library_roots = FxHashSet::default();
-            for (idx, root) in roots.into_iter().enumerate() {
+            for (idx, root) in roots.iter().enumerate() {
                 let root_id = SourceRootId(idx as u32);
-                let durability = durability(&root);
                 if root.is_library {
                     library_roots.insert(root_id);
                 } else {
                     local_roots.insert(root_id);
                 }
-                for file_id in root.iter() {
-                    self.set_file_source_root_with_durability(file_id, root_id, durability);
-                }
-                self.set_source_root_with_durability(root_id, Arc::new(root), durability);
             }
             self.set_local_roots_with_durability(Arc::new(local_roots), Durability::HIGH);
             self.set_library_roots_with_durability(Arc::new(library_roots), Durability::HIGH);
         }
-
-        for (file_id, text) in change.files_changed {
-            let source_root_id = self.file_source_root(file_id);
-            let source_root = self.source_root(source_root_id);
-            let durability = durability(&source_root);
-            // XXX: can't actually remove the file, just reset the text
-            let text = text.unwrap_or_default();
-            self.set_file_text_with_durability(file_id, text, durability)
-        }
-        if let Some(crate_graph) = change.crate_graph {
-            self.set_crate_graph_with_durability(Arc::new(crate_graph), Durability::HIGH)
-        }
+        change.apply(self);
     }
 
     pub fn collect_garbage(&mut self) {
@@ -295,11 +237,3 @@ impl RootDatabase {
         acc
     }
 }
-
-fn durability(source_root: &SourceRoot) -> Durability {
-    if source_root.is_library {
-        Durability::HIGH
-    } else {
-        Durability::LOW
-    }
-}

crates/ide_db/src/lib.rs

@@ -2,10 +2,10 @@
 //!
 //! It is mainly a `HirDatabase` for semantic analysis, plus a `SymbolsDatabase`, for fuzzy search.
 
+mod apply_change;
 pub mod label;
 pub mod line_index;
 pub mod symbol_index;
-pub mod change;
 pub mod defs;
 pub mod search;
 pub mod imports_locator;

crates/rust-analyzer/src/cli/analysis_bench.rs

@@ -8,8 +8,7 @@ use base_db::{
     FileId,
 };
 use ide::{
-    Analysis, AnalysisChange, AnalysisHost, CompletionConfig, DiagnosticsConfig, FilePosition,
-    LineCol,
+    Analysis, AnalysisHost, Change, CompletionConfig, DiagnosticsConfig, FilePosition, LineCol,
 };
 use vfs::AbsPathBuf;
 
@@ -143,7 +142,7 @@ fn do_work<F: Fn(&Analysis) -> T, T>(host: &mut AnalysisHost, file_id: FileId, w
         {
             let mut text = host.analysis().file_text(file_id).unwrap().to_string();
             text.push_str("\n/* Hello world */\n");
-            let mut change = AnalysisChange::new();
+            let mut change = Change::new();
             change.change_file(file_id, Some(Arc::new(text)));
             host.apply_change(change);
         }
@@ -156,7 +155,7 @@ fn do_work<F: Fn(&Analysis) -> T, T>(host: &mut AnalysisHost, file_id: FileId, w
         {
             let mut text = host.analysis().file_text(file_id).unwrap().to_string();
             text.push_str("\npub fn _dummy() {}\n");
-            let mut change = AnalysisChange::new();
+            let mut change = Change::new();
             change.change_file(file_id, Some(Arc::new(text)));
             host.apply_change(change);
         }

crates/rust-analyzer/src/cli/load_cargo.rs

@@ -5,7 +5,7 @@ use std::{path::Path, sync::Arc};
 use anyhow::Result;
 use base_db::CrateGraph;
 use crossbeam_channel::{unbounded, Receiver};
-use ide::{AnalysisChange, AnalysisHost};
+use ide::{AnalysisHost, Change};
 use project_model::{CargoConfig, ProcMacroClient, ProjectManifest, ProjectWorkspace};
 use vfs::{loader::Handle, AbsPath, AbsPathBuf};
 
@@ -62,7 +62,7 @@ fn load(
 ) -> AnalysisHost {
     let lru_cap = std::env::var("RA_LRU_CAP").ok().and_then(|it| it.parse::<usize>().ok());
     let mut host = AnalysisHost::new(lru_cap);
-    let mut analysis_change = AnalysisChange::new();
+    let mut analysis_change = Change::new();
 
     // wait until Vfs has loaded all roots
     for task in receiver {

crates/rust-analyzer/src/global_state.rs

@@ -8,7 +8,7 @@ use std::{sync::Arc, time::Instant};
 use base_db::{CrateId, VfsPath};
 use crossbeam_channel::{unbounded, Receiver, Sender};
 use flycheck::FlycheckHandle;
-use ide::{Analysis, AnalysisChange, AnalysisHost, FileId};
+use ide::{Analysis, AnalysisHost, Change, FileId};
 use lsp_types::{SemanticTokens, Url};
 use parking_lot::{Mutex, RwLock};
 use project_model::{CargoWorkspace, ProcMacroClient, ProjectWorkspace, Target};
@@ -139,7 +139,7 @@ impl GlobalState {
         let mut has_fs_changes = false;
 
         let change = {
-            let mut change = AnalysisChange::new();
+            let mut change = Change::new();
             let (vfs, line_endings_map) = &mut *self.vfs.write();
             let changed_files = vfs.take_changes();
             if changed_files.is_empty() {