Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #17483 - alibektas:ratoml/fixes, r=alibektas
minor : fixes for ratoml module
This is a follow-up PR to #17058.
- Parse errors are reflected as such by defining a new variant called `ConfigError::ParseError`
- New error collection has been added to store config level agnostic errors.
EDIT : Some things that this PR promised to solve are removed and will be addressed by other PRs
| -rw-r--r-- | crates/rust-analyzer/src/config.rs | 119 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/main_loop.rs | 8 | ||||
| -rw-r--r-- | crates/rust-analyzer/tests/slow-tests/ratoml.rs | 80 |
3 files changed, 92 insertions, 115 deletions
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 5905137967..14a0550e55 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -779,7 +779,6 @@ pub struct Config { /// | Windows | `{FOLDERID_RoamingAppData}` | C:\Users\Alice\AppData\Roaming | user_config_path: VfsPath, - /// FIXME @alibektas : Change this to sth better. /// Config node whose values apply to **every** Rust project. user_config: Option<(GlobalLocalConfigInput, ConfigErrors)>, @@ -795,6 +794,12 @@ pub struct Config { /// Clone of the value that is stored inside a `GlobalState`. source_root_parent_map: Arc<FxHashMap<SourceRootId, SourceRootId>>, + /// Use case : It is an error to have an empty value for `check_command`. + /// Since it is a `global` command at the moment, its final value can only be determined by + /// traversing through `global` configs and the `client` config. However the non-null value constraint + /// is config level agnostic, so this requires an independent error storage + validation_errors: ConfigErrors, + detached_files: Vec<AbsPathBuf>, } @@ -824,6 +829,7 @@ impl Config { /// The return tuple's bool component signals whether the `GlobalState` should call its `update_configuration()` method. fn apply_change_with_sink(&self, change: ConfigChange) -> (Config, bool) { let mut config = self.clone(); + config.validation_errors = ConfigErrors::default(); let mut should_update = false; @@ -852,6 +858,7 @@ impl Config { if let Some(mut json) = change.client_config_change { tracing::info!("updating config from JSON: {:#}", json); + if !(json.is_null() || json.as_object().map_or(false, |it| it.is_empty())) { let mut json_errors = vec![]; let detached_files = get_field_json::<Vec<Utf8PathBuf>>( @@ -867,6 +874,37 @@ impl Config { patch_old_style::patch_json_for_outdated_configs(&mut json); + // IMPORTANT : This holds as long as ` completion_snippets_custom` is declared `client`. + config.snippets.clear(); + + let snips = self.completion_snippets_custom().to_owned(); + + for (name, def) in snips.iter() { + if def.prefix.is_empty() && def.postfix.is_empty() { + continue; + } + let scope = match def.scope { + SnippetScopeDef::Expr => SnippetScope::Expr, + SnippetScopeDef::Type => SnippetScope::Type, + SnippetScopeDef::Item => SnippetScope::Item, + }; + match Snippet::new( + &def.prefix, + &def.postfix, + &def.body, + def.description.as_ref().unwrap_or(name), + &def.requires, + scope, + ) { + Some(snippet) => config.snippets.push(snippet), + None => json_errors.push(( + name.to_owned(), + <serde_json::Error as serde::de::Error>::custom(format!( + "snippet {name} is invalid or triggers are missing", + )), + )), + } + } config.client_config = ( FullConfigInput::from_json(json, &mut json_errors), ConfigErrors( @@ -906,8 +944,15 @@ impl Config { )); should_update = true; } - // FIXME - Err(_) => (), + Err(e) => { + config.root_ratoml = Some(( + GlobalLocalConfigInput::from_toml(toml::map::Map::default(), &mut vec![]), + ConfigErrors(vec![ConfigErrorInner::ParseError { + reason: e.message().to_owned(), + } + .into()]), + )); + } } } @@ -942,8 +987,18 @@ impl Config { ), ); } - // FIXME - Err(_) => (), + Err(e) => { + config.root_ratoml = Some(( + GlobalLocalConfigInput::from_toml( + toml::map::Map::default(), + &mut vec![], + ), + ConfigErrors(vec![ConfigErrorInner::ParseError { + reason: e.message().to_owned(), + } + .into()]), + )); + } } } } @@ -953,48 +1008,13 @@ impl Config { config.source_root_parent_map = source_root_map; } - // IMPORTANT : This holds as long as ` completion_snippets_custom` is declared `client`. - config.snippets.clear(); - - let snips = self.completion_snippets_custom().to_owned(); - - for (name, def) in snips.iter() { - if def.prefix.is_empty() && def.postfix.is_empty() { - continue; - } - let scope = match def.scope { - SnippetScopeDef::Expr => SnippetScope::Expr, - SnippetScopeDef::Type => SnippetScope::Type, - SnippetScopeDef::Item => SnippetScope::Item, - }; - #[allow(clippy::single_match)] - match Snippet::new( - &def.prefix, - &def.postfix, - &def.body, - def.description.as_ref().unwrap_or(name), - &def.requires, - scope, - ) { - Some(snippet) => config.snippets.push(snippet), - // FIXME - // None => error_sink.0.push(ConfigErrorInner::Json { - // config_key: "".to_owned(), - // error: <serde_json::Error as serde::de::Error>::custom(format!( - // "snippet {name} is invalid or triggers are missing", - // )), - // }), - None => (), - } + if config.check_command().is_empty() { + config.validation_errors.0.push(Arc::new(ConfigErrorInner::Json { + config_key: "/check/command".to_owned(), + error: serde_json::Error::custom("expected a non-empty string"), + })); } - // FIXME: bring this back - // if config.check_command().is_empty() { - // error_sink.0.push(ConfigErrorInner::Json { - // config_key: "/check/command".to_owned(), - // error: serde_json::Error::custom("expected a non-empty string"), - // }); - // } (config, should_update) } @@ -1012,6 +1032,7 @@ impl Config { .chain(config.root_ratoml.as_ref().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.validation_errors.0.iter()) .cloned() .collect(), ); @@ -1259,9 +1280,10 @@ pub struct ClientCommandsConfig { pub enum ConfigErrorInner { Json { config_key: String, error: serde_json::Error }, Toml { config_key: String, error: toml::de::Error }, + ParseError { reason: String }, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub struct ConfigErrors(Vec<Arc<ConfigErrorInner>>); impl ConfigErrors { @@ -1283,6 +1305,7 @@ impl fmt::Display for ConfigErrors { f(&": ")?; f(e) } + ConfigErrorInner::ParseError { reason } => f(reason), }); write!(f, "invalid config value{}:\n{}", if self.0.len() == 1 { "" } else { "s" }, errors) } @@ -1336,6 +1359,7 @@ impl Config { root_ratoml: None, root_ratoml_path, detached_files: Default::default(), + validation_errors: Default::default(), } } @@ -2575,6 +2599,7 @@ macro_rules! _impl_for_config_data { } } + &self.default_config.global.$field } )* @@ -3304,7 +3329,7 @@ fn validate_toml_table( ptr.push_str(k); match v { - // This is a table config, any entry in it is therefor valid + // This is a table config, any entry in it is therefore valid toml::Value::Table(_) if verify(ptr) => (), toml::Value::Table(table) => validate_toml_table(known_ptrs, table, ptr, error_sink), _ if !verify(ptr) => error_sink diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index db90d2d964..1b4bff6225 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -14,7 +14,7 @@ use ide_db::base_db::{SourceDatabase, SourceDatabaseExt, VfsPath}; use lsp_server::{Connection, Notification, Request}; use lsp_types::{notification::Notification as _, TextDocumentIdentifier}; use stdx::thread::ThreadIntent; -use tracing::{span, Level}; +use tracing::{error, span, Level}; use vfs::{AbsPathBuf, FileId}; use crate::{ @@ -674,7 +674,7 @@ impl GlobalState { 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}"); + error!("FetchWorkspaceError:\n{e}"); } self.switch_workspaces("fetched workspace".to_owned()); (Progress::End, None) @@ -719,7 +719,7 @@ impl GlobalState { BuildDataProgress::End(build_data_result) => { self.fetch_build_data_queue.op_completed(build_data_result); if let Err(e) = self.fetch_build_data_error() { - tracing::error!("FetchBuildDataError:\n{e}"); + error!("FetchBuildDataError:\n{e}"); } self.switch_workspaces("fetched build data".to_owned()); @@ -937,7 +937,7 @@ impl GlobalState { diag.fix, ), Err(err) => { - tracing::error!( + error!( "flycheck {id}: File with cargo diagnostic not found in VFS: {}", err ); diff --git a/crates/rust-analyzer/tests/slow-tests/ratoml.rs b/crates/rust-analyzer/tests/slow-tests/ratoml.rs index 218a9a32ad..3b05138e18 100644 --- a/crates/rust-analyzer/tests/slow-tests/ratoml.rs +++ b/crates/rust-analyzer/tests/slow-tests/ratoml.rs @@ -30,8 +30,6 @@ impl RatomlTest { const EMIT_MUST_USE: &'static str = r#"assist.emitMustUse = true"#; const EMIT_MUST_NOT_USE: &'static str = r#"assist.emitMustUse = false"#; - const GLOBAL_TRAIT_ASSOC_ITEMS_ZERO: &'static str = r#"hover.show.traitAssocItems = 0"#; - fn new( fixtures: Vec<&str>, roots: Vec<&str>, @@ -180,25 +178,6 @@ impl RatomlTest { } } -// /// Check if we are listening for changes in user's config file ( e.g on Linux `~/.config/rust-analyzer/.rust-analyzer.toml`) -// #[test] -// #[cfg(target_os = "windows")] -// fn listen_to_user_config_scenario_windows() { -// todo!() -// } - -// #[test] -// #[cfg(target_os = "linux")] -// fn listen_to_user_config_scenario_linux() { -// todo!() -// } - -// #[test] -// #[cfg(target_os = "macos")] -// fn listen_to_user_config_scenario_macos() { -// todo!() -// } - /// Check if made changes have had any effect on /// the client config. #[test] @@ -420,15 +399,6 @@ assist.emitMustUse = true"#, server.delete(2); assert!(!server.query(QueryType::Local, 1)); } -// #[test] -// fn delete_user_config() { -// todo!() -// } - -// #[test] -// fn modify_client_config() { -// todo!() -// } #[test] fn ratoml_inherit_config_from_ws_root() { @@ -609,6 +579,7 @@ pub fn add(left: usize, right: usize) -> usize { } #[test] +#[ignore = "Root ratomls are not being looked for on startup. Fix this."] fn ratoml_rm_ws_root_ratoml_child_has_client_as_parent_now() { let mut server = RatomlTest::new( vec![ @@ -837,6 +808,7 @@ 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![ @@ -849,30 +821,23 @@ edition = "2021" "#, r#" //- /rust-analyzer.toml -hover.show.traitAssocItems = 4 +rustfmt.rangeFormatting.enable = true "#, r#" //- /p1/src/lib.rs -trait RandomTrait { - type B; - fn abc() -> i32; - fn def() -> i64; -} - fn main() { - let a = RandomTrait; + todo!() }"#, ], vec![], None, ); - server.query(QueryType::Global, 2); + assert!(server.query(QueryType::Global, 2)); } -#[allow(unused)] -// #[test] -// FIXME: Re-enable this test when we have a global config we can check again +#[test] +#[ignore = "Root ratomls are not being looked for on startup. Fix this."] fn ratoml_root_is_updateable() { let mut server = RatomlTest::new( vec![ @@ -885,18 +850,12 @@ edition = "2021" "#, r#" //- /rust-analyzer.toml -hover.show.traitAssocItems = 4 - "#, +rustfmt.rangeFormatting.enable = true + "#, r#" //- /p1/src/lib.rs -trait RandomTrait { - type B; - fn abc() -> i32; - fn def() -> i64; -} - fn main() { - let a = RandomTrait; + todo!() }"#, ], vec![], @@ -904,13 +863,12 @@ fn main() { ); assert!(server.query(QueryType::Global, 2)); - server.edit(1, RatomlTest::GLOBAL_TRAIT_ASSOC_ITEMS_ZERO.to_owned()); + server.edit(1, "rustfmt.rangeFormatting.enable = false".to_owned()); assert!(!server.query(QueryType::Global, 2)); } -#[allow(unused)] -// #[test] -// FIXME: Re-enable this test when we have a global config we can check again +#[test] +#[ignore = "Root ratomls are not being looked for on startup. Fix this."] fn ratoml_root_is_deletable() { let mut server = RatomlTest::new( vec![ @@ -923,18 +881,12 @@ edition = "2021" "#, r#" //- /rust-analyzer.toml -hover.show.traitAssocItems = 4 - "#, +rustfmt.rangeFormatting.enable = true + "#, r#" //- /p1/src/lib.rs -trait RandomTrait { - type B; - fn abc() -> i32; - fn def() -> i64; -} - fn main() { - let a = RandomTrait; + todo!() }"#, ], vec![], |