From d7bedc85b3aecad57498b7e12decfa9bbf7db044 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Sat, 27 Jul 2024 23:52:40 +0200 Subject: [PATCH] Down to 1 failing test And that is due to a case where we have two ratomls in a source root, one of which is a `workspace_ratoml` and the other one is simple old ratoml. Since we are not checking to see if the source root is already populated with workspace ratoml, this test fails. Due to principles of clear code I believe it is reasonable to not have two HashMaps that are almost for the exact same thing. So next commit should remove `workspace_ratoml` and merge it with `krate_ratoml`s. --- crates/rust-analyzer/src/capabilities.rs | 2 +- crates/rust-analyzer/src/config.rs | 78 +++++++++++-------- crates/rust-analyzer/src/global_state.rs | 15 +++- crates/rust-analyzer/src/handlers/request.rs | 5 +- .../rust-analyzer/tests/slow-tests/ratoml.rs | 46 ++--------- 5 files changed, 66 insertions(+), 80 deletions(-) diff --git a/crates/rust-analyzer/src/capabilities.rs b/crates/rust-analyzer/src/capabilities.rs index 212294b5d3..9610808c27 100644 --- a/crates/rust-analyzer/src/capabilities.rs +++ b/crates/rust-analyzer/src/capabilities.rs @@ -67,7 +67,7 @@ pub fn server_capabilities(config: &Config) -> ServerCapabilities { code_action_provider: Some(config.caps().code_action_capabilities()), code_lens_provider: Some(CodeLensOptions { resolve_provider: Some(true) }), document_formatting_provider: Some(OneOf::Left(true)), - document_range_formatting_provider: match config.rustfmt() { + document_range_formatting_provider: match config.rustfmt(None) { RustfmtConfig::Rustfmt { enable_range_formatting: true, .. } => Some(OneOf::Left(true)), _ => Some(OneOf::Left(false)), }, diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 36419d4a97..d667ed9497 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -780,10 +780,10 @@ pub struct Config { user_config: Option<(GlobalLocalConfigInput, ConfigErrors)>, /// TODO : This file can be used to make global changes while having only a workspace-wide scope. - workspace_ratoml_change: FxHashMap, + workspace_ratoml: FxHashMap, /// For every `SourceRoot` there can be at most one RATOML file. - ratoml_files: FxHashMap, + krate_ratoml: FxHashMap, /// Clone of the value that is stored inside a `GlobalState`. source_root_parent_map: Arc>, @@ -927,7 +927,7 @@ impl Config { &mut String::new(), &mut toml_errors, ); - config.workspace_ratoml_change.insert( + config.workspace_ratoml.insert( source_root_id, ( GlobalLocalConfigInput::from_toml(table, &mut toml_errors), @@ -969,7 +969,7 @@ impl Config { &mut String::new(), &mut toml_errors, ); - config.ratoml_files.insert( + config.krate_ratoml.insert( source_root_id, ( LocalConfigInput::from_toml(&table, &mut toml_errors), @@ -1022,15 +1022,9 @@ impl Config { .1 .0 .iter() - .chain( - config - .workspace_ratoml_change - .values() - .into_iter() - .flat_map(|it| it.1 .0.iter()), - ) + .chain(config.workspace_ratoml.values().into_iter().flat_map(|it| it.1 .0.iter())) .chain(config.user_config.as_ref().into_iter().flat_map(|it| it.1 .0.iter())) - .chain(config.ratoml_files.values().flat_map(|it| it.1 .0.iter())) + .chain(config.krate_ratoml.values().flat_map(|it| it.1 .0.iter())) .chain(config.validation_errors.0.iter()) .cloned() .collect(), @@ -1070,6 +1064,7 @@ impl ConfigChange { vfs_path: VfsPath, content: Option>, ) -> Option<(VfsPath, Option>)> { + dbg!("change_ratoml", &vfs_path); self.ratoml_file_change .get_or_insert_with(Default::default) .insert(source_root, (vfs_path, content)) @@ -1086,6 +1081,7 @@ impl ConfigChange { vfs_path: VfsPath, content: Option>, ) -> Option<(VfsPath, Option>)> { + dbg!("change_workspace", &vfs_path); self.workspace_ratoml_change .get_or_insert_with(Default::default) .insert(source_root, (vfs_path, content)) @@ -1350,14 +1346,14 @@ impl Config { workspace_roots, visual_studio_code_version, client_config: (FullConfigInput::default(), ConfigErrors(vec![])), - ratoml_files: FxHashMap::default(), + krate_ratoml: FxHashMap::default(), default_config: DEFAULT_CONFIG_DATA.get_or_init(|| Box::leak(Box::default())), source_root_parent_map: Arc::new(FxHashMap::default()), user_config: None, user_config_path, detached_files: Default::default(), validation_errors: Default::default(), - workspace_ratoml_change: Default::default(), + workspace_ratoml: Default::default(), } } @@ -1877,7 +1873,7 @@ impl Config { } } - pub fn rustfmt(&self) -> RustfmtConfig { + pub fn rustfmt(&self, source_root_id: Option) -> RustfmtConfig { match &self.rustfmt_overrideCommand(None) { Some(args) if !args.is_empty() => { let mut args = args.clone(); @@ -1885,8 +1881,8 @@ impl Config { RustfmtConfig::CustomCommand { command, args } } Some(_) | None => RustfmtConfig::Rustfmt { - extra_args: self.rustfmt_extraArgs(None).clone(), - enable_range_formatting: *self.rustfmt_rangeFormatting_enable(None), + extra_args: self.rustfmt_extraArgs(source_root_id).clone(), + enable_range_formatting: *self.rustfmt_rangeFormatting_enable(source_root_id), }, } } @@ -2540,22 +2536,28 @@ macro_rules! _impl_for_config_data { $($doc)* #[allow(non_snake_case)] $vis fn $field(&self, source_root: Option) -> &$ty { - let mut par: Option = source_root; - while let Some(source_root_id) = par { - par = self.source_root_parent_map.get(&source_root_id).copied(); - if let Some((config, _)) = self.ratoml_files.get(&source_root_id) { + let follow = if stringify!($field) == "assist_emitMustUse" { dbg!("YEY"); true } else {false}; + let mut current: Option = None; + let mut next: Option = source_root; + if follow { dbg!(&self.krate_ratoml);} + while let Some(source_root_id) = next { + current = next; + next = self.source_root_parent_map.get(&source_root_id).copied(); + if let Some((config, _)) = self.krate_ratoml.get(&source_root_id) { if let Some(value) = config.$field.as_ref() { return value; } } } - // TODO - // if let Some((root_path_ratoml, _)) = self.root_ratoml.as_ref() { - // if let Some(v) = root_path_ratoml.local.$field.as_ref() { - // return &v; - // } - // } + if let Some(current) = current { + if follow { dbg!(&self.workspace_ratoml);} + if let Some((root_path_ratoml, _)) = self.workspace_ratoml.get(¤t).as_ref() { + if let Some(v) = root_path_ratoml.local.$field.as_ref() { + return &v; + } + } + } if let Some(v) = self.client_config.0.local.$field.as_ref() { return &v; @@ -2582,12 +2584,22 @@ macro_rules! _impl_for_config_data { $($doc)* #[allow(non_snake_case)] $vis fn $field(&self, source_root : Option) -> &$ty { - // TODO - // if let Some((root_path_ratoml, _)) = self.root_ratoml.as_ref() { - // if let Some(v) = root_path_ratoml.global.$field.as_ref() { - // return &v; - // } - // } + let follow = if stringify!($field) == "rustfmt_extraArgs" { dbg!("YEY"); true } else {false}; + let mut current: Option = None; + let mut next: Option = source_root; + while let Some(source_root_id) = next { + current = next; + next = self.source_root_parent_map.get(&source_root_id).copied(); + } + + if let Some(current) = current { + if follow { dbg!(&self.workspace_ratoml);} + if let Some((root_path_ratoml, _)) = self.workspace_ratoml.get(¤t).as_ref() { + if let Some(v) = root_path_ratoml.global.$field.as_ref() { + return &v; + } + } + } if let Some(v) = self.client_config.0.global.$field.as_ref() { return &v; diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index d78e760358..5bfdb67af2 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -386,12 +386,18 @@ impl GlobalState { let mut change = ConfigChange::default(); let db = self.analysis_host.raw_database(); - // FIXME @alibektas : This is silly. There is abs no reason to use VfsPaths when there is SourceRoots. But how + // FIXME @alibektas : This is silly. There is no reason to use VfsPaths when there is SourceRoots. But how // do I resolve a "workspace_root" to its corresponding id without having to rely on a cargo.toml's ( or project json etc.) file id? - let workspace_roots = self + let workspace_ratoml_paths = self .workspaces .iter() - .map(|ws| VfsPath::from(ws.workspace_root().to_owned())) + .map(|ws| { + VfsPath::from({ + let mut p = ws.workspace_root().to_owned(); + p.push("rust-analyzer.toml"); + p + }) + }) .collect_vec(); for (file_id, (_change_kind, vfs_path)) in modified_ratoml_files { @@ -405,7 +411,7 @@ impl GlobalState { let sr_id = db.file_source_root(file_id); let sr = db.source_root(sr_id); - if workspace_roots.contains(&vfs_path) { + if workspace_ratoml_paths.contains(&vfs_path) { change.change_workspace_ratoml( sr_id, vfs_path, @@ -422,6 +428,7 @@ impl GlobalState { ) { // SourceRoot has more than 1 RATOML files. In this case lexicographically smaller wins. if old_path < vfs_path { + dbg!("HARBIDEN"); span!(Level::ERROR, "Two `rust-analyzer.toml` files were found inside the same crate. {vfs_path} has no effect."); // Put the old one back in. change.change_ratoml(sr_id, old_path, old_text); diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs index 44ff273b5a..9680a5c827 100644 --- a/crates/rust-analyzer/src/handlers/request.rs +++ b/crates/rust-analyzer/src/handlers/request.rs @@ -2090,8 +2090,9 @@ fn run_rustfmt( let edition = editions.iter().copied().max(); let line_index = snap.file_line_index(file_id)?; + let sr = snap.analysis.source_root_id(file_id)?; - let mut command = match snap.config.rustfmt() { + let mut command = match snap.config.rustfmt(Some(sr)) { RustfmtConfig::Rustfmt { extra_args, enable_range_formatting } => { // FIXME: Set RUSTUP_TOOLCHAIN let mut cmd = process::Command::new(toolchain::Tool::Rustfmt.path()); @@ -2280,7 +2281,7 @@ pub(crate) fn internal_testing_fetch_config( serde_json::to_value(match &*params.config { "local" => state.config.assist(source_root).assist_emit_must_use, "global" => matches!( - state.config.rustfmt(), + state.config.rustfmt(source_root), RustfmtConfig::Rustfmt { enable_range_formatting: true, .. } ), _ => return Err(anyhow::anyhow!("Unknown test config key: {}", params.config)), diff --git a/crates/rust-analyzer/tests/slow-tests/ratoml.rs b/crates/rust-analyzer/tests/slow-tests/ratoml.rs index 298e10fb9e..8c111723fb 100644 --- a/crates/rust-analyzer/tests/slow-tests/ratoml.rs +++ b/crates/rust-analyzer/tests/slow-tests/ratoml.rs @@ -700,37 +700,6 @@ fn ratoml_multiple_ratoml_in_single_source_root() { ); assert!(server.query(QueryType::Local, 3)); - - let server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -[package] -name = "p1" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/src/rust-analyzer.toml -assist.emitMustUse = false -"#, - r#" -//- /p1/rust-analyzer.toml -assist.emitMustUse = true -"#, - r#" -//- /p1/src/lib.rs -enum Value { - Number(i32), - Text(String), -} -"#, - ], - vec!["p1"], - None, - ); - - assert!(server.query(QueryType::Local, 3)); } /// If a root is non-local, so we cannot find what its parent is @@ -808,7 +777,6 @@ enum Value { /// Having a ratoml file at the root of a project enables /// configuring global level configurations as well. #[test] -// #[ignore = "Root ratomls are not being looked for on startup. Fix this."] fn ratoml_in_root_is_global() { let server = RatomlTest::new( vec![ @@ -820,7 +788,7 @@ version = "0.1.0" edition = "2021" "#, r#" -//- /rust-analyzer.toml +//- /p1/rust-analyzer.toml rustfmt.rangeFormatting.enable = true "#, r#" @@ -829,7 +797,7 @@ fn main() { todo!() }"#, ], - vec![], + vec!["p1"], None, ); @@ -837,7 +805,6 @@ fn main() { } #[test] -#[ignore = "Root ratomls are not being looked for on startup. Fix this."] fn ratoml_root_is_updateable() { let mut server = RatomlTest::new( vec![ @@ -849,7 +816,7 @@ version = "0.1.0" edition = "2021" "#, r#" -//- /rust-analyzer.toml +//- /p1/rust-analyzer.toml rustfmt.rangeFormatting.enable = true "#, r#" @@ -858,7 +825,7 @@ fn main() { todo!() }"#, ], - vec![], + vec!["p1"], None, ); @@ -868,7 +835,6 @@ fn main() { } #[test] -#[ignore = "Root ratomls are not being looked for on startup. Fix this."] fn ratoml_root_is_deletable() { let mut server = RatomlTest::new( vec![ @@ -880,7 +846,7 @@ version = "0.1.0" edition = "2021" "#, r#" -//- /rust-analyzer.toml +//- /p1/rust-analyzer.toml rustfmt.rangeFormatting.enable = true "#, r#" @@ -889,7 +855,7 @@ fn main() { todo!() }"#, ], - vec![], + vec!["p1"], None, );