Skip to content

Commit

Permalink
Fix file loading of r-a toml files
Browse files Browse the repository at this point in the history
  • Loading branch information
@Veykril
Veykril committed Jun 6, 2024
1 parent 047b8d9 commit 1316ba4
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 90 deletions.
24 changes: 12 additions & 12 deletions crates/rust-analyzer/src/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -902,10 +902,10 @@ impl Config {

#[derive(Default, Debug)]
pub struct ConfigChange {
user_config_change: Option<String>,
root_ratoml_change: Option<String>,
user_config_change: Option<Arc<str>>,
root_ratoml_change: Option<Arc<str>>,
client_config_change: Option<serde_json::Value>,
ratoml_file_change: Option<FxHashMap<SourceRootId, (VfsPath, Option<String>)>>,
ratoml_file_change: Option<FxHashMap<SourceRootId, (VfsPath, Option<Arc<str>>)>>,
source_map_change: Option<Arc<FxHashMap<SourceRootId, SourceRootId>>>,
}

Expand All @@ -914,19 +914,19 @@ impl ConfigChange {
&mut self,
source_root: SourceRootId,
vfs_path: VfsPath,
content: Option<String>,
) -> Option<(VfsPath, Option<String>)> {
content: Option<Arc<str>>,
) -> Option<(VfsPath, Option<Arc<str>>)> {
self.ratoml_file_change
.get_or_insert_with(Default::default)
.insert(source_root, (vfs_path, content))
}

pub fn change_user_config(&mut self, content: Option<String>) {
pub fn change_user_config(&mut self, content: Option<Arc<str>>) {
assert!(self.user_config_change.is_none()); // Otherwise it is a double write.
self.user_config_change = content;
}

pub fn change_root_ratoml(&mut self, content: Option<String>) {
pub fn change_root_ratoml(&mut self, content: Option<Arc<str>>) {
assert!(self.user_config_change.is_none()); // Otherwise it is a double write.
self.root_ratoml_change = content;
}
Expand Down Expand Up @@ -1206,8 +1206,7 @@ impl Config {
client_config: (FullConfigInput::default(), ConfigErrors(vec![])),
user_config: None,
ratoml_files: FxHashMap::default(),
default_config: DEFAULT_CONFIG_DATA
.get_or_init(|| Box::leak(Box::new(DefaultConfigData::default()))),
default_config: DEFAULT_CONFIG_DATA.get_or_init(|| Box::leak(Box::default())),
source_root_parent_map: Arc::new(FxHashMap::default()),
user_config_path,
root_ratoml: None,
Expand Down Expand Up @@ -3548,7 +3547,8 @@ mod tests {
[cargo.sysroot]
non-table = "expected"
}
.to_string(),
.to_string()
.into(),
));

let (config, e, _) = config.apply_change(change);
Expand Down Expand Up @@ -3582,9 +3582,9 @@ mod tests {
be = "be"
valid = "valid"
}
.to_string(),
.to_string()
.into(),
));

let (_, e, _) = config.apply_change(change);
expect_test::expect![[r#"
ConfigErrors(
Expand Down
61 changes: 23 additions & 38 deletions crates/rust-analyzer/src/global_state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use project_model::{
use rustc_hash::{FxHashMap, FxHashSet};
use tracing::{span, Level};
use triomphe::Arc;
use vfs::{AnchoredPathBuf, Vfs, VfsPath};
use vfs::{AnchoredPathBuf, ChangeKind, Vfs, VfsPath};

Check failure on line 28 in crates/rust-analyzer/src/global_state.rs

View workflow job for this annotation

GitHub Actions / Rust (ubuntu-latest) 

unused import: `VfsPath`

use crate::{
config::{Config, ConfigChange, ConfigErrors},
Expand Down Expand Up @@ -262,19 +262,9 @@ impl GlobalState {
// that can be used by the config module because config talks
// in `SourceRootId`s instead of `FileId`s and `FileId` -> `SourceRootId`
// mapping is not ready until `AnalysisHost::apply_changes` has been called.
let mut modified_ratoml_files: FxHashMap<FileId, vfs::VfsPath> = FxHashMap::default();
let mut ratoml_text_map: FxHashMap<FileId, (vfs::VfsPath, Option<String>)> =
let mut modified_ratoml_files: FxHashMap<FileId, (ChangeKind, vfs::VfsPath)> =
FxHashMap::default();

let mut user_config_file: Option<Option<String>> = None;
let mut root_path_ratoml: Option<Option<String>> = None;

let root_vfs_path = {
let mut root_vfs_path = self.config.root_path().to_path_buf();
root_vfs_path.push("rust-analyzer.toml");
VfsPath::new_real_path(root_vfs_path.to_string())
};

let (change, modified_rust_files, workspace_structure_change) = {
let mut change = ChangeWithProcMacros::new();
let mut guard = self.vfs.write();
Expand All @@ -296,7 +286,7 @@ impl GlobalState {
let vfs_path = vfs.file_path(file.file_id);
if let Some(("rust-analyzer", Some("toml"))) = vfs_path.name_and_extension() {
// Remember ids to use them after `apply_changes`
modified_ratoml_files.insert(file.file_id, vfs_path.clone());
modified_ratoml_files.insert(file.file_id, (file.kind(), vfs_path.clone()));
}

if let Some(path) = vfs_path.as_path() {
Expand Down Expand Up @@ -344,17 +334,7 @@ impl GlobalState {
Some(text)
}
};

change.change_file(file_id, text.clone());
if let Some(vfs_path) = modified_ratoml_files.get(&file_id) {
if vfs_path == self.config.user_config_path() {
user_config_file = Some(text);
} else if vfs_path == &root_vfs_path {
root_path_ratoml = Some(text);
} else {
ratoml_text_map.insert(file_id, (vfs_path.clone(), text));
}
}
change.change_file(file_id, text);
});
if has_structure_changes {
let roots = self.source_root_config.partition(vfs);
Expand All @@ -365,22 +345,35 @@ impl GlobalState {

let _p = span!(Level::INFO, "GlobalState::process_changes/apply_change").entered();
self.analysis_host.apply_change(change);
if !(ratoml_text_map.is_empty() && user_config_file.is_none() && root_path_ratoml.is_none())
{
if !modified_ratoml_files.is_empty() {
let config_change = {
let user_config_path = self.config.user_config_path();
let root_ratoml_path = self.config.root_ratoml_path();
let mut change = ConfigChange::default();
let db = self.analysis_host.raw_database();

for (file_id, (vfs_path, text)) in ratoml_text_map {
for (file_id, (_change_kind, vfs_path)) in modified_ratoml_files {
if vfs_path == *user_config_path {
change.change_user_config(Some(db.file_text(file_id)));
continue;
}

if vfs_path == *root_ratoml_path {
change.change_root_ratoml(Some(db.file_text(file_id)));
continue;
}

// If change has been made to a ratoml file that
// belongs to a non-local source root, we will ignore it.
// As it doesn't make sense a users to use external config files.
let sr_id = db.file_source_root(file_id);
let sr = db.source_root(sr_id);
if !sr.is_library {
if let Some((old_path, old_text)) =
change.change_ratoml(sr_id, vfs_path.clone(), text)
{
if let Some((old_path, old_text)) = change.change_ratoml(
sr_id,
vfs_path.clone(),
Some(db.file_text(file_id)),
) {
// SourceRoot has more than 1 RATOML files. In this case lexicographically smaller wins.
if old_path < vfs_path {
span!(Level::ERROR, "Two `rust-analyzer.toml` files were found inside the same crate. {vfs_path} has no effect.");
Expand All @@ -394,14 +387,6 @@ impl GlobalState {
}
}

if let Some(Some(txt)) = user_config_file {
change.change_user_config(Some(txt));
}

if let Some(Some(txt)) = root_path_ratoml {
change.change_root_ratoml(Some(txt));
}

change
};

Expand Down
42 changes: 2 additions & 40 deletions crates/rust-analyzer/src/reload.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
//! project is currently loading and we don't have a full project model, we
//! still want to respond to various requests.
// FIXME: This is a mess that needs some untangling work
use std::{iter, mem, ops::Not as _};
use std::{iter, mem};

use flycheck::{FlycheckConfig, FlycheckHandle};
use hir::{db::DefDatabase, ChangeWithProcMacros, ProcMacros};
Expand All @@ -28,12 +28,11 @@ use lsp_types::FileSystemWatcher;
use proc_macro_api::ProcMacroServer;
use project_model::{ManifestPath, ProjectWorkspace, ProjectWorkspaceKind, WorkspaceBuildScripts};
use stdx::{format_to, thread::ThreadIntent};
use tracing::error;
use triomphe::Arc;
use vfs::{AbsPath, AbsPathBuf, ChangeKind};

use crate::{
config::{Config, ConfigChange, FilesWatcher, LinkedProject},
config::{Config, FilesWatcher, LinkedProject},
global_state::GlobalState,
lsp_ext,
main_loop::Task,
Expand Down Expand Up @@ -573,43 +572,6 @@ impl GlobalState {
self.source_root_config = project_folders.source_root_config;
self.local_roots_parent_map = Arc::new(self.source_root_config.source_root_parent_map());

let user_config_path = self.config.user_config_path();
let root_ratoml_path = self.config.root_ratoml_path();

{
let vfs = &mut self.vfs.write().0;
let loader = &mut self.loader;

if vfs.file_id(user_config_path).is_none() {
if let Some(user_cfg_abs) = user_config_path.as_path() {
let contents = loader.handle.load_sync(user_cfg_abs);
vfs.set_file_contents(user_config_path.clone(), contents);
} else {
error!("Non-abs virtual path for user config.");
}
}

if vfs.file_id(root_ratoml_path).is_none() {
// FIXME @alibektas : Sometimes root_path_ratoml collide with a regular ratoml.
// Although this shouldn't be a problem because everything is mapped to a `FileId`.
// We may want to further think about this.
if let Some(root_ratoml_abs) = root_ratoml_path.as_path() {
let contents = loader.handle.load_sync(root_ratoml_abs);
vfs.set_file_contents(root_ratoml_path.clone(), contents);
} else {
error!("Non-abs virtual path for user config.");
}
}
}

let mut config_change = ConfigChange::default();
config_change.change_source_root_parent_map(self.local_roots_parent_map.clone());

let (config, e, _) = self.config.apply_change(config_change);
self.config_errors = e.is_empty().not().then_some(e);

self.config = Arc::new(config);

self.recreate_crate_graph(cause);

tracing::info!("did switch workspaces");
Expand Down

0 comments on commit 1316ba4

Please sign in to comment.