Auto merge of #1300 - lijinpei:warn-unused-config, r=alexheretic

Warning on unused config

This commit send a "windows/showMessage" notification, when some unknown or duplicated config is met during initialization request. see #1153

Author: bors

Cargo.lock

@@ -46,6 +46,7 @@ rustc-serialize = "0.3"
 serde = "1.0"
 serde_json = "1.0"
 serde_derive = "1.0"
+serde_ignored = "0.0"
 url = "1"
 walkdir = "2"
 regex = "1"

Cargo.toml

@@ -306,7 +306,7 @@ impl InitActionContext {
         let needs_inference = {
             let mut config = self.config.lock().unwrap();
 
-            if let Some(init_config) = init_options.settings.map(|s| s.rust.0) {
+            if let Some(init_config) = init_options.settings.map(|s| s.rust) {
                 config.update(init_config);
             }
             config.needs_inference()

src/actions/mod.rs

@@ -14,7 +14,6 @@ use crate::actions::{FileWatch, InitActionContext, VersionOrdering};
 use crate::Span;
 use log::{debug, trace, warn};
 use rls_vfs::{Change, VfsSpan};
-use serde::Deserialize;
 use std::sync::atomic::Ordering;
 
 use crate::build::*;
@@ -150,18 +149,22 @@ impl BlockingNotificationAction for Cancel {
 
 impl BlockingNotificationAction for DidChangeConfiguration {
     fn handle<O: Output>(
-        mut params: DidChangeConfigurationParams,
+        params: DidChangeConfigurationParams,
         ctx: &mut InitActionContext,
         out: O,
     ) -> Result<(), ()> {
         trace!("config change: {:?}", params.settings);
-	params.settings = crate::lsp_data::json_key_to_snake_case(params.settings);
-        let settings = ChangeConfigSettings::deserialize(&params.settings);
+        use std::collections::HashMap;
+        let mut dups = HashMap::new();
+        let mut unknowns = vec![];
+        let settings = ChangeConfigSettings::try_deserialize(&params.settings, &mut dups, &mut unknowns);
+        crate::server::maybe_notify_unknown_configs(&out, &unknowns);
+        crate::server::maybe_notify_duplicated_configs(&out, &dups);
 
         let new_config = match settings {
             Ok(mut value) => {
-                value.rust.0.normalise();
-                value.rust.0
+                value.rust.normalise();
+                value.rust
             }
             Err(err) => {
                 warn!(

src/actions/notifications.rs

@@ -201,6 +201,50 @@ impl Default for Config {
 }
 
 impl Config {
+    /// try to deserialize a Config from a json value, val is expected to be a
+    /// Value::Object, all first level keys of val are converted to snake_case,
+    /// duplicated and unknown keys are reported
+    pub fn try_deserialize(val: &serde_json::value::Value,
+        dups:&mut std::collections::HashMap<String, Vec<String>>,
+        unknowns: &mut Vec<(String)>)
+            -> Result<Config, ()> {
+
+        #[derive(Clone)]
+        struct JsonValue(serde_json::value::Value);
+
+        impl<'de> serde::de::IntoDeserializer<'de, serde_json::Error> for JsonValue {
+            type Deserializer =  serde_json::value::Value;
+            fn into_deserializer(self) -> Self::Deserializer {
+                self.0
+            }
+        }
+
+        if let serde_json::Value::Object(map) = val {
+            let seq = serde::de::value::MapDeserializer::new(map.iter().filter_map(|(k, v)| {
+                use heck::SnakeCase;
+                let snake_case = k.to_snake_case();
+                let (_, ref mut vec) = dups.raw_entry_mut().from_key(&snake_case).or_insert(snake_case.clone(), vec![]);
+                vec.push(k.to_string());
+                if vec.len() == 1 {
+                    Some((snake_case, JsonValue(v.to_owned().clone())))
+                } else {
+                    None
+                }
+            }));
+            match serde_ignored::deserialize(seq, |path|unknowns.push(path.to_string())) {
+                Ok(conf) => {
+                    dups.retain(|_, v| v.len() > 1);
+                    return Ok(conf)
+
+                },
+                _ => {
+                    dups.retain(|_, v| v.len() > 1);
+                },
+            }
+        }
+        Err(())
+    }
+
     /// Join this configuration with the new config.
     pub fn update(&mut self, mut new: Config) {
         new.normalise();

src/config.rs

@@ -5,7 +5,7 @@
 //! functionality such as 'goto definition', symbol search, reformatting, and
 //! code completion, and enables renaming and refactorings.
 
-#![feature(rustc_private, integer_atomics, drain_filter)]
+#![feature(rustc_private, integer_atomics, drain_filter, hash_raw_entry)]
 #![allow(unknown_lints)]
 #![warn(clippy::all, rust_2018_idioms)]
 #![allow(

src/lib.rs

@@ -246,31 +246,38 @@ impl RangeExt for Range {
     }
 }
 
-pub struct Config(pub config::Config);
-
-impl<'de> serde::Deserialize<'de> for Config {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: serde::Deserializer<'de> {
-            use serde::de::Error;
-            let val = json_key_to_snake_case(serde_json::Value::deserialize(deserializer)?);
-            match serde_json::from_value(val) {
-                Ok(config) => Ok(Config(config)),
-                _ => Err(D::Error::custom("unable to deserialize Config")),
-            }
-        }
-}
-
-impl fmt::Debug for Config {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        fmt::Debug::fmt(&self.0, f)
-    }
-}
-
 /// `DidChangeConfigurationParams.settings` payload reading the { rust: {...} } bit.
 #[derive(Debug, Deserialize)]
 pub struct ChangeConfigSettings {
-    pub rust: Config,
+    pub rust: config::Config,
+}
+
+impl ChangeConfigSettings {
+    /// try to deserialize a ChangeConfigSettings from a json value, val is
+    /// expected to be a Value::Object containing only one key "rust", all first
+    /// level keys of rust's value are converted to snake_case, duplicated and
+    /// unknown keys are reported
+    pub fn try_deserialize(val: &serde_json::value::Value,
+        dups:&mut std::collections::HashMap<String, Vec<String>>, unknowns: &mut Vec<String>)
+            -> Result<ChangeConfigSettings, ()> {
+        let mut ret = Err(());
+        if let serde_json::Value::Object(map) = val {
+            for (k, v) in map.iter() {
+                if k != "rust" {
+                    unknowns.push(k.to_string());
+                    continue;
+                }
+                if let serde_json::Value::Object(_) = v {
+                    if let Ok(rust) = config::Config::try_deserialize(v, dups, unknowns) {
+                        ret = Ok(ChangeConfigSettings{rust: rust});
+                    }
+                } else {
+                    return Err(());
+                }
+            }
+        }
+        ret
+    }
 }
 
 /* -----------------  JSON-RPC protocol types ----------------- */
@@ -287,6 +294,33 @@ pub struct InitializationOptions {
     pub settings: Option<ChangeConfigSettings>,
 }
 
+impl InitializationOptions {
+    /// try to deserialize a Initialization from a json value. If exists,
+    /// val.settings is expected to be a Value::Object containing only one key, 
+    /// "rust", all first level keys of rust's value are converted to
+    /// snake_case, duplicated and unknown keys are reported
+    pub fn try_deserialize(val: &serde_json::value::Value,
+        dups:&mut std::collections::HashMap<String, Vec<String>>, unknowns: &mut Vec<String>)
+            -> Result<InitializationOptions, ()> {
+        let mut val = val.to_owned();
+        let mut set = None;
+        if let Some(set1) = val.get_mut("settings") {
+            set = Some(set1.take());
+        }
+        let mut ret:InitializationOptions = 
+        match serde_json::from_value(val) {
+            Ok(ret) => ret,
+            _ => return Err(()),
+        };
+        if let Some(set) = set {
+            if let Ok(set) = ChangeConfigSettings::try_deserialize(&set, dups, unknowns) {
+                ret.settings = Some(set);
+            }
+        }
+        Ok(ret)
+    }
+}
+
 impl Default for InitializationOptions {
     fn default() -> Self {
         InitializationOptions {
@@ -417,28 +451,3 @@ pub struct ProgressParams {
     pub done: Option<bool>,
 }
 
-pub fn json_key_to_snake_case(mut val: serde_json::Value) -> serde_json::Value {
-    fn helper(val: &mut serde_json::Value) -> &mut serde_json::Value {
-        use heck::SnakeCase;
-        use serde_json::Value;
-        match val {
-            Value::Object(map) => {
-                let mut map1 = serde_json::map::Map::<String, Value>::with_capacity(map.len());
-                for kv in map.into_iter() {
-                    match map1.insert(kv.0.as_str().to_snake_case(), kv.1.clone()) {
-                        Some(val) => {
-                            log::error!("Multiple different case uses of `{}` config with value {} and value {}", kv.0, val, kv.1);
-                        }
-                        _ => (),
-                    }
-                }
-                std::mem::replace(map, map1);
-            }
-            _ => (),
-        }
-        val
-    }
-    helper(&mut val);
-    val
-}
-

src/lsp_data.rs

@@ -15,7 +15,7 @@
 use crate::actions::{notifications, requests, ActionContext};
 use crate::config::Config;
 use crate::lsp_data;
-use crate::lsp_data::{InitializationOptions, LSPNotification, LSPRequest};
+use crate::lsp_data::{InitializationOptions, LSPNotification, LSPRequest, ShowMessageParams, MessageType};
 use crate::server::dispatch::Dispatcher;
 pub use crate::server::dispatch::{RequestAction, DEFAULT_REQUEST_TIMEOUT};
 pub use crate::server::io::{MessageReader, Output};
@@ -27,7 +27,7 @@ pub use crate::server::message::{
 };
 use crate::version;
 use jsonrpc_core::{self as jsonrpc, types::error::ErrorCode, Id};
-pub use lsp_types::notification::Exit as ExitNotification;
+pub use lsp_types::notification::{Exit as ExitNotification, ShowMessage};
 pub use lsp_types::request::Initialize as InitializeRequest;
 pub use lsp_types::request::Shutdown as ShutdownRequest;
 use lsp_types::{
@@ -38,7 +38,6 @@ use lsp_types::{
 use log::{debug, error, trace, warn};
 use rls_analysis::AnalysisHost;
 use rls_vfs::Vfs;
-use serde_json;
 use std::path::PathBuf;
 use std::sync::atomic::Ordering;
 use std::sync::{Arc, Mutex};
@@ -86,26 +85,56 @@ impl BlockingRequestAction for ShutdownRequest {
     }
 }
 
+pub(crate) fn maybe_notify_unknown_configs<O: Output>(out: &O, unknowns: &[String]) {
+    use std::fmt::Write;
+    if unknowns.is_empty() {
+        return;
+    }
+    let mut msg = "Unknown RLS configuration:".to_string();
+    let mut first = true;
+    for key in unknowns {
+        write!(msg, "{}`{}` ", if first {' '} else {','}, key).unwrap();
+        first = false;
+    }
+    out.notify(Notification::<ShowMessage>::new(ShowMessageParams {
+        typ: MessageType::Warning,
+        message: msg,
+    }));
+}
+
+pub(crate) fn maybe_notify_duplicated_configs<O: Output>(out: &O, dups: &std::collections::HashMap<String, Vec<String>>) {
+    use std::fmt::Write;
+    if dups.is_empty() {
+        return;
+    }
+    let mut msg = String::new();
+    for kv in dups {
+        write!(msg, "{}:", kv.0).unwrap();
+        let mut first = true;
+        for v in kv.1 {
+            write!(msg, "{}{}, ", if first {' '} else {','}, v).unwrap();
+            first = false;
+        }
+        msg += "; ";
+    }
+    out.notify(Notification::<ShowMessage>::new(ShowMessageParams {
+        typ: MessageType::Warning,
+        message: format!("Duplicated RLS configuration: {}", msg.clone()),
+    }));
+}
+
 impl BlockingRequestAction for InitializeRequest {
     type Response = NoResponse;
 
     fn handle<O: Output>(
         id: RequestId,
-        params: Self::Params,
+        mut params: Self::Params,
         ctx: &mut ActionContext,
         out: O,
     ) -> Result<NoResponse, ResponseError> {
-        let init_options: InitializationOptions = params
-            .initialization_options
-            .as_ref()
-            .and_then(|options| {
-                let de = serde_json::from_value(options.to_owned());
-                if let Err(ref e) = de {
-                    warn!("initialization_options: {}", e);
-                }
-                de.ok()
-            })
-            .unwrap_or_default();
+        let mut dups = std::collections::HashMap::new();
+        let mut unknowns = Vec::new();
+        let init_options = params.initialization_options.take().and_then(|opt| InitializationOptions::try_deserialize(&opt, &mut dups, &mut unknowns).ok()).unwrap_or_default();
 
         trace!("init: {:?} -> {:?}", params.initialization_options, init_options);
 
@@ -117,6 +146,9 @@ impl BlockingRequestAction for InitializeRequest {
             ));
         }
 
+        maybe_notify_unknown_configs(&out, &unknowns);
+        maybe_notify_duplicated_configs(&out, &dups);
+
         let result = InitializeResult {
             capabilities: server_caps(ctx),
         };