Auto merge of #1539 - Xanewok:config, r=Xanewok

Warn against deprecated config entries

Addresses rust-lang/vscode-rust#643.

Brings back removed option from #1509 but only not to trigger the unknown option warning.

Author: bors

rls/src/actions/notifications.rs

@@ -140,9 +140,15 @@ impl BlockingNotificationAction for DidChangeConfiguration {
         use std::collections::HashMap;
         let mut dups = HashMap::new();
         let mut unknowns = vec![];
-        let settings =
-            ChangeConfigSettings::try_deserialize(&params.settings, &mut dups, &mut unknowns);
+        let mut deprecated = vec![];
+        let settings = ChangeConfigSettings::try_deserialize(
+            &params.settings,
+            &mut dups,
+            &mut unknowns,
+            &mut deprecated,
+        );
         crate::server::maybe_notify_unknown_configs(&out, &unknowns);
+        crate::server::maybe_notify_deprecated_configs(&out, &deprecated);
         crate::server::maybe_notify_duplicated_configs(&out, &dups);
 
         let new_config = match settings {

rls/src/config.rs

@@ -1,6 +1,7 @@
 //! Configuration for the workspace that RLS is operating within and options for
 //! tweaking the RLS's behavior itself.
 
+use std::collections::HashMap;
 use std::env;
 use std::fmt;
 use std::fmt::Debug;
@@ -172,6 +173,8 @@ pub struct Config {
     /// to be loaded by the RLS. The program given should output a list of
     /// resulting JSON files on stdout.
     pub build_command: Option<String>,
+    /// DEPRECATED: Use `crate_blacklist` instead.
+    pub use_crate_blacklist: Option<bool>,
 }
 
 impl Default for Config {
@@ -201,20 +204,32 @@ impl Default for Config {
             show_hover_context: true,
             rustfmt_path: None,
             build_command: None,
+            use_crate_blacklist: None,
         };
         result.normalise();
         result
     }
 }
 
+lazy_static::lazy_static! {
+    #[derive(Debug)]
+    pub static ref DEPRECATED_OPTIONS: HashMap<&'static str, Option<&'static str>> = {
+        [("use_crate_blacklist", Some("use `crate_blacklist` instead"))]
+            .iter()
+            .map(ToOwned::to_owned)
+            .collect()
+    };
+}
+
 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)>,
+        unknowns: &mut Vec<String>,
+        deprecated: &mut Vec<String>,
     ) -> Result<Config, ()> {
         #[derive(Clone)]
         struct JsonValue(serde_json::value::Value);
@@ -234,6 +249,10 @@ impl Config {
                 vec.push(k.to_string());
 
                 if vec.len() == 1 {
+                    if DEPRECATED_OPTIONS.contains_key(snake_case.as_str()) {
+                        deprecated.push(snake_case.clone());
+                    }
+
                     Some((snake_case, JsonValue(v.to_owned().clone())))
                 } else {
                     None
@@ -486,14 +505,17 @@ fn clippy_preference_from_str() {
 #[test]
 fn blacklist_default() {
     let value = serde_json::json!({});
-    let config = Config::try_deserialize(&value, &mut Default::default(), &mut vec![]).unwrap();
+    let config =
+        Config::try_deserialize(&value, &mut Default::default(), &mut vec![], &mut vec![]).unwrap();
     assert_eq!(config.crate_blacklist.as_ref(), &CrateBlacklist::default());
     let value = serde_json::json!({"crate_blacklist": []});
 
-    let config = Config::try_deserialize(&value, &mut Default::default(), &mut vec![]).unwrap();
+    let config =
+        Config::try_deserialize(&value, &mut Default::default(), &mut vec![], &mut vec![]).unwrap();
     assert_eq!(&*config.crate_blacklist.as_ref().0, &[] as &[String]);
 
     let value = serde_json::json!({"crate_blacklist": ["serde"]});
-    let config = Config::try_deserialize(&value, &mut Default::default(), &mut vec![]).unwrap();
+    let config =
+        Config::try_deserialize(&value, &mut Default::default(), &mut vec![], &mut vec![]).unwrap();
     assert_eq!(&*config.crate_blacklist.as_ref().0, &["serde".to_string()]);
 }

rls/src/lsp_data.rs

@@ -234,6 +234,7 @@ impl ChangeConfigSettings {
         val: &serde_json::value::Value,
         dups: &mut std::collections::HashMap<String, Vec<String>>,
         unknowns: &mut Vec<String>,
+        deprecated: &mut Vec<String>,
     ) -> Result<ChangeConfigSettings, ()> {
         let mut ret = Err(());
         if let serde_json::Value::Object(map) = val {
@@ -243,7 +244,8 @@ impl ChangeConfigSettings {
                     continue;
                 }
                 if let serde_json::Value::Object(_) = v {
-                    if let Ok(rust) = config::Config::try_deserialize(v, dups, unknowns) {
+                    if let Ok(rust) = config::Config::try_deserialize(v, dups, unknowns, deprecated)
+                    {
                         ret = Ok(ChangeConfigSettings { rust });
                     }
                 } else {
@@ -275,25 +277,16 @@ impl InitializationOptions {
     /// "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,
+        mut val: serde_json::value::Value,
         dups: &mut std::collections::HashMap<String, Vec<String>>,
         unknowns: &mut Vec<String>,
+        deprecated: &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)
+        let settings = val.get_mut("settings").map(|x| x.take()).and_then(|set| {
+            ChangeConfigSettings::try_deserialize(&set, dups, unknowns, deprecated).ok()
+        });
+
+        Ok(InitializationOptions { settings, ..serde_json::from_value(val).map_err(|_| ())? })
     }
 }
 
@@ -326,17 +319,17 @@ impl ClientCapabilities {
             .and_then(|doc| doc.completion.as_ref())
             .and_then(|comp| comp.completion_item.as_ref())
             .and_then(|item| item.snippet_support.as_ref())
-            .unwrap_or(&false)
-            .to_owned();
+            .copied()
+            .unwrap_or(false);
 
         let related_information_support = params
             .capabilities
             .text_document
             .as_ref()
             .and_then(|doc| doc.publish_diagnostics.as_ref())
             .and_then(|diag| diag.related_information.as_ref())
-            .unwrap_or(&false)
-            .to_owned();
+            .copied()
+            .unwrap_or(false);
 
         ClientCapabilities { code_completion_has_snippet_support, related_information_support }
     }

rls/src/server/mod.rs

@@ -3,7 +3,7 @@
 //! requests).
 
 use crate::actions::{notifications, requests, ActionContext};
-use crate::config::Config;
+use crate::config::{Config, DEPRECATED_OPTIONS};
 use crate::lsp_data;
 use crate::lsp_data::{
     InitializationOptions, LSPNotification, LSPRequest, MessageType, ShowMessageParams,
@@ -94,6 +94,24 @@ pub(crate) fn maybe_notify_unknown_configs<O: Output>(out: &O, unknowns: &[Strin
     }));
 }
 
+/// For each deprecated configuration key an appropriate warning is emitted via
+/// LSP, along with a deprecation notice (if there is one).
+pub(crate) fn maybe_notify_deprecated_configs<O: Output>(out: &O, keys: &[String]) {
+    for key in keys {
+        let notice = DEPRECATED_OPTIONS.get(key.as_str()).and_then(|x| *x);
+        let message = format!(
+            "RLS configuration option `{}` is deprecated{}",
+            key,
+            notice.map(|notice| format!(": {}", notice)).unwrap_or_default()
+        );
+
+        out.notify(Notification::<ShowMessage>::new(ShowMessageParams {
+            typ: MessageType::Warning,
+            message,
+        }));
+    }
+}
+
 pub(crate) fn maybe_notify_duplicated_configs<O: Output>(
     out: &O,
     dups: &std::collections::HashMap<String, Vec<String>>,
@@ -129,11 +147,18 @@ impl BlockingRequestAction for InitializeRequest {
     ) -> Result<NoResponse, ResponseError> {
         let mut dups = std::collections::HashMap::new();
         let mut unknowns = Vec::new();
+        let mut deprecated = Vec::new();
         let init_options = params
             .initialization_options
             .take()
             .and_then(|opt| {
-                InitializationOptions::try_deserialize(&opt, &mut dups, &mut unknowns).ok()
+                InitializationOptions::try_deserialize(
+                    opt,
+                    &mut dups,
+                    &mut unknowns,
+                    &mut deprecated,
+                )
+                .ok()
             })
             .unwrap_or_default();
 
@@ -148,6 +173,7 @@ impl BlockingRequestAction for InitializeRequest {
         }
 
         maybe_notify_unknown_configs(&out, &unknowns);
+        maybe_notify_deprecated_configs(&out, &deprecated);
         maybe_notify_duplicated_configs(&out, &dups);
 
         let result = InitializeResult { capabilities: server_caps(ctx) };

tests/client.rs

@@ -1272,6 +1272,11 @@ fn is_notification_for_unknown_config(msg: &serde_json::Value) -> bool {
         && msg["params"]["message"].as_str().unwrap().contains("Unknown")
 }
 
+fn is_notification_for_deprecated_config(msg: &serde_json::Value) -> bool {
+    msg["method"] == ShowMessage::METHOD
+        && msg["params"]["message"].as_str().unwrap().contains("is deprecated")
+}
+
 fn is_notification_for_duplicated_config(msg: &serde_json::Value) -> bool {
     msg["method"] == ShowMessage::METHOD
         && msg["params"]["message"].as_str().unwrap().contains("Duplicate")
@@ -1305,14 +1310,20 @@ fn client_init_duplicated_and_unknown_settings() {
                 "dup_val": false,
                 "dup_licated": "dup_lacated",
                 "DupLicated": "DupLicated",
-                "dup-licated": "dup-licated"
+                "dup-licated": "dup-licated",
+                // Deprecated
+                "use_crate_blacklist": true
             }
         }
     });
 
     rls.request::<Initialize>(0, initialize_params_with_opts(root_path, opts));
 
     assert!(rls.messages().iter().any(is_notification_for_unknown_config));
+    assert!(
+        rls.messages().iter().any(is_notification_for_deprecated_config),
+        "`use_crate_blacklist` should be marked as deprecated"
+    );
     assert!(rls.messages().iter().any(is_notification_for_duplicated_config));
 }