Merge #11214

11214: feat: poke user when supplying faulty configurations r=Veykril a=Veykril

bors r+

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

4 files changed, 36 insertions, 12 deletions

diff --git a/crates/rust-analyzer/src/bin/main.rs b/crates/rust-analyzer/src/bin/main.rs
index 567cdc48ef4..9a2e4355915 100644
--- a/crates/rust-analyzer/src/bin/main.rs
+++ b/crates/rust-analyzer/src/bin/main.rs
@@ -150,7 +150,7 @@ fn run_server() -> Result<()> {
 
     let mut config = Config::new(root_path, initialize_params.capabilities);
     if let Some(json) = initialize_params.initialization_options {
-        config.update(json);
+        let _ = config.update(json);
     }
 
     let server_capabilities = rust_analyzer::server_capabilities(&config);
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
index a7041392554..de0aba0caa5 100644
--- a/crates/rust-analyzer/src/config.rs
+++ b/crates/rust-analyzer/src/config.rs
@@ -341,7 +341,7 @@ config_data! {
 
 impl Default for ConfigData {
     fn default() -> Self {
-        ConfigData::from_json(serde_json::Value::Null)
+        ConfigData::from_json(serde_json::Value::Null, &mut Vec::new())
     }
 }
 
@@ -492,16 +492,21 @@ impl Config {
             snippets: Default::default(),
         }
     }
-    pub fn update(&mut self, mut json: serde_json::Value) {
+    pub fn update(
+        &mut self,
+        mut json: serde_json::Value,
+    ) -> Result<(), Vec<(String, serde_json::Error)>> {
         tracing::info!("updating config from JSON: {:#}", json);
         if json.is_null() || json.as_object().map_or(false, |it| it.is_empty()) {
-            return;
+            return Ok(());
         }
-        self.detached_files = get_field::<Vec<PathBuf>>(&mut json, "detachedFiles", None, "[]")
-            .into_iter()
-            .map(AbsPathBuf::assert)
-            .collect();
-        self.data = ConfigData::from_json(json);
+        let mut errors = Vec::new();
+        self.detached_files =
+            get_field::<Vec<PathBuf>>(&mut json, &mut errors, "detachedFiles", None, "[]")
+                .into_iter()
+                .map(AbsPathBuf::assert)
+                .collect();
+        self.data = ConfigData::from_json(json, &mut errors);
         self.snippets.clear();
         for (name, def) in self.data.completion_snippets.iter() {
             if def.prefix.is_empty() && def.postfix.is_empty() {
@@ -524,6 +529,11 @@ impl Config {
                 None => tracing::info!("Invalid snippet {}", name),
             }
         }
+        if errors.is_empty() {
+            Ok(())
+        } else {
+            Err(errors)
+        }
     }
 
     pub fn json_schema() -> serde_json::Value {
@@ -1116,10 +1126,11 @@ macro_rules! _config_data {
         #[derive(Debug, Clone)]
         struct $name { $($field: $ty,)* }
         impl $name {
-            fn from_json(mut json: serde_json::Value) -> $name {
+            fn from_json(mut json: serde_json::Value, error_sink: &mut Vec<(String, serde_json::Error)>) -> $name {
                 $name {$(
                     $field: get_field(
                         &mut json,
+                        error_sink,
                         stringify!($field),
                         None$(.or(Some(stringify!($alias))))*,
                         $default,
@@ -1156,6 +1167,7 @@ use _config_data as config_data;
 
 fn get_field<T: DeserializeOwned>(
     json: &mut serde_json::Value,
+    error_sink: &mut Vec<(String, serde_json::Error)>,
     field: &'static str,
     alias: Option<&'static str>,
     default: &str,
@@ -1174,6 +1186,7 @@ fn get_field<T: DeserializeOwned>(
                 Ok(it) => Some(it),
                 Err(e) => {
                     tracing::warn!("Failed to deserialize config field at {}: {:?}", pointer, e);
+                    error_sink.push((pointer, e));
                     None
                 }
             })
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index 65f9c873673..af987230def 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -9,6 +9,7 @@ use std::{
 use always_assert::always;
 use crossbeam_channel::{select, Receiver};
 use ide_db::base_db::{SourceDatabaseExt, VfsPath};
+use itertools::Itertools;
 use lsp_server::{Connection, Notification, Request};
 use lsp_types::notification::Notification as _;
 use vfs::{ChangeKind, FileId};
@@ -731,7 +732,17 @@ impl GlobalState {
                                     // Note that json can be null according to the spec if the client can't
                                     // provide a configuration. This is handled in Config::update below.
                                     let mut config = Config::clone(&*this.config);
-                                    config.update(json.take());
+                                    if let Err(errors) = config.update(json.take()) {
+                                        let errors = errors
+                                            .iter()
+                                            .format_with("\n", |(key, e),f| {
+                                                f(key)?;
+                                                f(&": ")?;
+                                                f(e)
+                                            });
+                                        let msg= format!("Failed to deserialize config key(s):\n{}", errors);
+                                        this.show_message(lsp_types::MessageType::WARNING, msg);
+                                    }
                                     this.update_configuration(config);
                                 }
                             }
diff --git a/crates/rust-analyzer/tests/slow-tests/support.rs b/crates/rust-analyzer/tests/slow-tests/support.rs
index 6e7ecec5b2b..78aa4119159 100644
--- a/crates/rust-analyzer/tests/slow-tests/support.rs
+++ b/crates/rust-analyzer/tests/slow-tests/support.rs
@@ -137,7 +137,7 @@ impl<'a> Project<'a> {
             },
         );
         config.discovered_projects = Some(discovered_projects);
-        config.update(self.config);
+        let _ = config.update(self.config);
 
         Server::new(tmp_dir, config)
     }