11958: Show config deseralization failures on start up r=Veykril a=Veykril

We now also show deserialization errors to the user when starting the server.
This PR also adds a small validation "pass" on the config that we will probably populate over time with more checks.

Fixes https://github.com/rust-analyzer/rust-analyzer/issues/11950

bors r+

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
This commit is contained in:
bors[bot] 2022-04-11 11:12:55 +00:00 committed by GitHub
commit b1854ceecf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 97 additions and 61 deletions

View File

@ -150,7 +150,17 @@ fn run_server() -> Result<()> {
let mut config = Config::new(root_path, initialize_params.capabilities);
if let Some(json) = initialize_params.initialization_options {
let _ = config.update(json);
if let Err(e) = config.update(json) {
use lsp_types::{
notification::{Notification, ShowMessage},
MessageType, ShowMessageParams,
};
let not = lsp_server::Notification::new(
ShowMessage::METHOD.to_string(),
ShowMessageParams { typ: MessageType::WARNING, message: e.to_string() },
);
connection.sender.send(lsp_server::Message::Notification(not)).unwrap();
}
}
let server_capabilities = rust_analyzer::server_capabilities(&config);
@ -161,7 +171,11 @@ fn run_server() -> Result<()> {
name: String::from("rust-analyzer"),
version: Some(String::from(env!("REV"))),
}),
offset_encoding: if supports_utf8(&config.caps) { Some("utf-8".to_string()) } else { None },
offset_encoding: if supports_utf8(config.caps()) {
Some("utf-8".to_string())
} else {
None
},
};
let initialize_result = serde_json::to_value(initialize_result).unwrap();
@ -183,7 +197,7 @@ fn run_server() -> Result<()> {
.collect::<Vec<_>>()
})
.filter(|workspaces| !workspaces.is_empty())
.unwrap_or_else(|| vec![config.root_path.clone()]);
.unwrap_or_else(|| vec![config.root_path().clone()]);
let discovered = ProjectManifest::discover_all(&workspace_roots);
tracing::info!("discovered projects: {:?}", discovered);

View File

@ -27,7 +27,7 @@ pub fn server_capabilities(config: &Config) -> ServerCapabilities {
})),
hover_provider: Some(HoverProviderCapability::Simple(true)),
completion_provider: Some(CompletionOptions {
resolve_provider: completions_resolve_provider(&config.caps),
resolve_provider: completions_resolve_provider(config.caps()),
trigger_characters: Some(vec![":".to_string(), ".".to_string(), "'".to_string()]),
all_commit_characters: None,
completion_item: None,
@ -46,7 +46,7 @@ pub fn server_capabilities(config: &Config) -> ServerCapabilities {
document_highlight_provider: Some(OneOf::Left(true)),
document_symbol_provider: Some(OneOf::Left(true)),
workspace_symbol_provider: Some(OneOf::Left(true)),
code_action_provider: Some(code_action_capabilities(&config.caps)),
code_action_provider: Some(code_action_capabilities(config.caps())),
code_lens_provider: Some(CodeLensOptions { resolve_provider: Some(true) }),
document_formatting_provider: Some(OneOf::Left(true)),
document_range_formatting_provider: match config.rustfmt() {

View File

@ -123,7 +123,7 @@ pub(crate) fn for_file(
file_id: FileId,
) -> Result<Option<CargoTargetSpec>> {
let crate_id = match global_state_snapshot.analysis.crate_for(file_id)?.first() {
Some(crate_id) => *crate_id,
Some(&crate_id) => crate_id,
None => return Ok(None),
};
let (cargo_ws, target) = match global_state_snapshot.cargo_target_for_crate_root(crate_id) {

View File

@ -7,7 +7,7 @@
//! configure the server itself, feature flags are passed into analysis, and
//! tweak things like automatic insertion of `()` in completions.
use std::{ffi::OsString, iter, path::PathBuf};
use std::{ffi::OsString, fmt, iter, path::PathBuf};
use flycheck::FlycheckConfig;
use ide::{
@ -19,6 +19,7 @@
imports::insert_use::{ImportGranularity, InsertUseConfig, PrefixKind},
SnippetCap,
};
use itertools::Itertools;
use lsp_types::{ClientCapabilities, MarkupKind};
use project_model::{
CargoConfig, ProjectJson, ProjectJsonData, ProjectManifest, RustcSource, UnsetTestCrates,
@ -31,9 +32,7 @@
caps::completion_item_edit_resolve,
diagnostics::DiagnosticsMapConfig,
line_index::OffsetEncoding,
lsp_ext::supports_utf8,
lsp_ext::WorkspaceSymbolSearchScope,
lsp_ext::{self, WorkspaceSymbolSearchKind},
lsp_ext::{self, supports_utf8, WorkspaceSymbolSearchKind, WorkspaceSymbolSearchScope},
};
// Defines the server-side configuration of the rust-analyzer. We generate
@ -369,11 +368,11 @@ fn default() -> Self {
#[derive(Debug, Clone)]
pub struct Config {
pub caps: lsp_types::ClientCapabilities,
pub discovered_projects: Option<Vec<ProjectManifest>>,
caps: lsp_types::ClientCapabilities,
root_path: AbsPathBuf,
data: ConfigData,
detached_files: Vec<AbsPathBuf>,
pub discovered_projects: Option<Vec<ProjectManifest>>,
pub root_path: AbsPathBuf,
snippets: Vec<Snippet>,
}
@ -505,6 +504,27 @@ pub struct ClientCommandsConfig {
pub trigger_parameter_hints: bool,
}
pub struct ConfigUpdateError {
errors: Vec<(String, serde_json::Error)>,
}
impl fmt::Display for ConfigUpdateError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let errors = self.errors.iter().format_with("\n", |(key, e), f| {
f(key)?;
f(&": ")?;
f(e)
});
write!(
f,
"rust-analyzer found {} invalid config value{}:\n{}",
self.errors.len(),
if self.errors.len() == 1 { "" } else { "s" },
errors
)
}
}
impl Config {
pub fn new(root_path: AbsPathBuf, caps: ClientCapabilities) -> Self {
Config {
@ -516,10 +536,8 @@ pub fn new(root_path: AbsPathBuf, caps: ClientCapabilities) -> Self {
snippets: Default::default(),
}
}
pub fn update(
&mut self,
mut json: serde_json::Value,
) -> Result<(), Vec<(String, serde_json::Error)>> {
pub fn update(&mut self, mut json: serde_json::Value) -> Result<(), ConfigUpdateError> {
tracing::info!("updating config from JSON: {:#}", json);
if json.is_null() || json.as_object().map_or(false, |it| it.is_empty()) {
return Ok(());
@ -553,16 +571,41 @@ pub fn update(
None => tracing::info!("Invalid snippet {}", name),
}
}
self.validate(&mut errors);
if errors.is_empty() {
Ok(())
} else {
Err(errors)
Err(ConfigUpdateError { errors })
}
}
fn validate(&self, error_sink: &mut Vec<(String, serde_json::Error)>) {
use serde::de::Error;
if self.data.checkOnSave_command.is_empty() {
error_sink.push((
"/checkOnSave/command".to_string(),
serde_json::Error::custom("expected a non-empty string"),
));
}
}
pub fn json_schema() -> serde_json::Value {
ConfigData::json_schema()
}
pub fn root_path(&self) -> &AbsPathBuf {
&self.root_path
}
pub fn caps(&self) -> &lsp_types::ClientCapabilities {
&self.caps
}
pub fn detached_files(&self) -> &[AbsPathBuf] {
&self.detached_files
}
}
macro_rules! try_ {
@ -578,43 +621,31 @@ macro_rules! try_or {
impl Config {
pub fn linked_projects(&self) -> Vec<LinkedProject> {
if self.data.linkedProjects.is_empty() {
self.discovered_projects
.as_ref()
.into_iter()
.flatten()
.cloned()
.map(LinkedProject::from)
.collect()
} else {
self.data
.linkedProjects
match self.data.linkedProjects.as_slice() {
[] => match self.discovered_projects.as_ref() {
Some(discovered_projects) => {
discovered_projects.iter().cloned().map(LinkedProject::from).collect()
}
None => Vec::new(),
},
linked_projects => linked_projects
.iter()
.filter_map(|linked_project| {
let res = match linked_project {
.filter_map(|linked_project| match linked_project {
ManifestOrProjectJson::Manifest(it) => {
let path = self.root_path.join(it);
ProjectManifest::from_manifest_file(path)
.map_err(|e| {
tracing::error!("failed to load linked project: {}", e)
})
.ok()?
.into()
.map_err(|e| tracing::error!("failed to load linked project: {}", e))
.ok()
.map(Into::into)
}
ManifestOrProjectJson::ProjectJson(it) => {
ProjectJson::new(&self.root_path, it.clone()).into()
Some(ProjectJson::new(&self.root_path, it.clone()).into())
}
};
Some(res)
})
.collect()
.collect(),
}
}
pub fn detached_files(&self) -> &[AbsPathBuf] {
&self.detached_files
}
pub fn did_save_text_document_dynamic_registration(&self) -> bool {
let caps =
try_or!(self.caps.text_document.as_ref()?.synchronization.clone()?, Default::default());

View File

@ -9,7 +9,6 @@
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};
@ -747,16 +746,8 @@ fn on_notification(&mut self, not: Notification) -> Result<()> {
// 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);
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);
if let Err(error) = config.update(json.take()) {
this.show_message(lsp_types::MessageType::WARNING, error.to_string());
}
this.update_configuration(config);
}