Minor fixes for ratoml module

- 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.

3 files changed, 90 insertions, 115 deletions

diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs
index f3ee7a98ac6..159098d120c 100644
--- a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs
+++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs
@@ -782,7 +782,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)>,
 
@@ -798,6 +797,13 @@ 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
+    /// FIXME : bad name I know...
+    other_errors: ConfigErrors,
+
     detached_files: Vec<AbsPathBuf>,
 }
 
@@ -827,6 +833,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.other_errors = ConfigErrors::default();
 
         let mut should_update = false;
 
@@ -855,6 +862,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::<Vec<Utf8PathBuf>>(
@@ -870,6 +878,38 @@ 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,
+                    };
+                    #[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),
+                        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(
@@ -909,8 +949,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()]),
+                    ));
+                }
             }
         }
 
@@ -945,8 +992,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()]),
+                            ));
+                        }
                     }
                 }
             }
@@ -956,48 +1013,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.other_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)
     }
 
@@ -1015,6 +1037,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.other_errors.0.iter())
                 .cloned()
                 .collect(),
         );
@@ -1262,9 +1285,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 {
@@ -1286,6 +1310,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)
     }
@@ -1339,6 +1364,7 @@ impl Config {
             root_ratoml: None,
             root_ratoml_path,
             detached_files: Default::default(),
+            other_errors: Default::default(),
         }
     }
 
@@ -2580,6 +2606,7 @@ macro_rules! _impl_for_config_data {
                         }
                     }
 
+
                     &self.default_config.global.$field
                 }
             )*
@@ -3309,7 +3336,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/src/tools/rust-analyzer/crates/rust-analyzer/src/main_loop.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/main_loop.rs
index db90d2d964c..1b4bff62257 100644
--- a/src/tools/rust-analyzer/crates/rust-analyzer/src/main_loop.rs
+++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs b/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs
index 218a9a32adb..9e9e418f511 100644
--- a/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs
+++ b/src/tools/rust-analyzer/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() {
@@ -849,30 +819,22 @@ 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]
 fn ratoml_root_is_updateable() {
     let mut server = RatomlTest::new(
         vec![
@@ -885,18 +847,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 +860,11 @@ 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]
 fn ratoml_root_is_deletable() {
     let mut server = RatomlTest::new(
         vec![
@@ -923,18 +877,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![],