Skip to content
This repository was archived by the owner on Dec 29, 2022. It is now read-only.

Warn against deprecated config entries #1539

Merged
merged 3 commits into from
Aug 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions rls/src/actions/notifications.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
30 changes: 26 additions & 4 deletions rls/src/config.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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()]);
}
35 changes: 14 additions & 21 deletions rls/src/lsp_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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(|_| ())? })
}
}

Expand Down Expand Up @@ -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 }
}
Expand Down
30 changes: 28 additions & 2 deletions rls/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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>>,
Expand Down Expand Up @@ -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();

Expand All @@ -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) };
Expand Down
13 changes: 12 additions & 1 deletion tests/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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));
}

Expand Down