Skip to content

Commit

Permalink
Minor fixes for ratoml module
Browse files Browse the repository at this point in the history 
- Parse errors are reflected as such by defining a new variant called `ConfigError::ParseError`
- Amend `config::ConfigChange::apply_change`.
- User config can be detected once again.
- New error collection has been added to store config level agnostic errors.
  • Loading branch information
@alibektas
alibektas committed Jun 23, 2024
1 parent 1bb376c commit c81594d
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 115 deletions.
115 changes: 71 additions & 44 deletions crates/rust-analyzer/src/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,6 @@ pub struct Config {
/// | Windows | `{FOLDERID_RoamingAppData}` | C:\Users\Alice\AppData\Roaming |
user_config_path: VfsPath,

/// FIXME @alibektas : Change this to sth better.
/// Config node whose values apply to **every** Rust project.
user_config: Option<(GlobalLocalConfigInput, ConfigErrors)>,

Expand All @@ -695,6 +694,13 @@ pub struct Config {
/// Clone of the value that is stored inside a `GlobalState`.
source_root_parent_map: Arc<FxHashMap<SourceRootId, SourceRootId>>,

/// Use case : It is an error to have an empty value for `check_command`.
/// Since it is a `global` command at the moment, its final value can only be determined by
/// traversing through `global` configs and the `client` config. However the non-null value constraint
/// is config level agnostic, so this requires an independent error storage
/// FIXME : bad name I know...
other_errors: ConfigErrors,

detached_files: Vec<AbsPathBuf>,
}

Expand All @@ -715,6 +721,7 @@ impl Config {
/// The return tuple's bool component signals whether the `GlobalState` should call its `update_configuration()` method.
fn apply_change_with_sink(&self, change: ConfigChange) -> (Config, bool) {
let mut config = self.clone();
config.other_errors = ConfigErrors::default();

let mut should_update = false;

Expand Down Expand Up @@ -743,6 +750,7 @@ impl Config {

if let Some(mut json) = change.client_config_change {
tracing::info!("updating config from JSON: {:#}", json);

if !(json.is_null() || json.as_object().map_or(false, |it| it.is_empty())) {
let mut json_errors = vec![];
let detached_files = get_field::<Vec<Utf8PathBuf>>(
Expand All @@ -758,6 +766,35 @@ impl Config {

patch_old_style::patch_json_for_outdated_configs(&mut json);

let snips = self.completion_snippets_custom().to_owned();

for (name, def) in snips.iter() {
if def.prefix.is_empty() && def.postfix.is_empty() {
continue;
}
let scope = match def.scope {
SnippetScopeDef::Expr => SnippetScope::Expr,
SnippetScopeDef::Type => SnippetScope::Type,
SnippetScopeDef::Item => SnippetScope::Item,
};
#[allow(clippy::single_match)]
match Snippet::new(
&def.prefix,
&def.postfix,
&def.body,
def.description.as_ref().unwrap_or(name),
&def.requires,
scope,
) {
Some(snippet) => config.snippets.push(snippet),
None => json_errors.push((
name.to_owned(),
<serde_json::Error as serde::de::Error>::custom(format!(
"snippet {name} is invalid or triggers are missing",
)),
)),
}
}
config.client_config = (
FullConfigInput::from_json(json, &mut json_errors),
ConfigErrors(
Expand Down Expand Up @@ -797,8 +834,15 @@ impl Config {
));
should_update = true;
}
// FIXME
Err(_) => (),
Err(e) => {
config.root_ratoml = Some((
GlobalLocalConfigInput::from_toml(toml::map::Map::default(), &mut vec![]),
ConfigErrors(vec![ConfigErrorInner::ParseError {
reason: e.message().to_owned(),
}
.into()]),
));
}
}
}

Expand Down Expand Up @@ -833,8 +877,18 @@ impl Config {
),
);
}
// FIXME
Err(_) => (),
Err(e) => {
config.root_ratoml = Some((
GlobalLocalConfigInput::from_toml(
toml::map::Map::default(),
&mut vec![],
),
ConfigErrors(vec![ConfigErrorInner::ParseError {
reason: e.message().to_owned(),
}
.into()]),
));
}
}
}
}
Expand All @@ -844,45 +898,13 @@ impl Config {
config.source_root_parent_map = source_root_map;
}

let snips = self.completion_snippets_custom().to_owned();

for (name, def) in snips.iter() {
if def.prefix.is_empty() && def.postfix.is_empty() {
continue;
}
let scope = match def.scope {
SnippetScopeDef::Expr => SnippetScope::Expr,
SnippetScopeDef::Type => SnippetScope::Type,
SnippetScopeDef::Item => SnippetScope::Item,
};
#[allow(clippy::single_match)]
match Snippet::new(
&def.prefix,
&def.postfix,
&def.body,
def.description.as_ref().unwrap_or(name),
&def.requires,
scope,
) {
Some(snippet) => config.snippets.push(snippet),
// FIXME
// None => error_sink.0.push(ConfigErrorInner::Json {
// config_key: "".to_owned(),
// error: <serde_json::Error as serde::de::Error>::custom(format!(
// "snippet {name} is invalid or triggers are missing",
// )),
// }),
None => (),
}
if config.check_command().is_empty() {
config.other_errors.0.push(Arc::new(ConfigErrorInner::Json {
config_key: "/check/command".to_owned(),
error: serde_json::Error::custom("expected a non-empty string"),
}));
}

// FIXME: bring this back
// if config.check_command().is_empty() {
// error_sink.0.push(ConfigErrorInner::Json {
// config_key: "/check/command".to_owned(),
// error: serde_json::Error::custom("expected a non-empty string"),
// });
// }
(config, should_update)
}

Expand All @@ -900,6 +922,7 @@ impl Config {
.chain(config.root_ratoml.as_ref().into_iter().flat_map(|it| it.1 .0.iter()))
.chain(config.user_config.as_ref().into_iter().flat_map(|it| it.1 .0.iter()))
.chain(config.ratoml_files.values().flat_map(|it| it.1 .0.iter()))
.chain(config.other_errors.0.iter())
.cloned()
.collect(),
);
Expand Down Expand Up @@ -1140,9 +1163,10 @@ pub struct ClientCommandsConfig {
pub enum ConfigErrorInner {
Json { config_key: String, error: serde_json::Error },
Toml { config_key: String, error: toml::de::Error },
ParseError { reason: String },
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, Default)]
pub struct ConfigErrors(Vec<Arc<ConfigErrorInner>>);

impl ConfigErrors {
Expand All @@ -1164,6 +1188,7 @@ impl fmt::Display for ConfigErrors {
f(&": ")?;
f(e)
}
ConfigErrorInner::ParseError { reason } => f(reason),
});
write!(f, "invalid config value{}:\n{}", if self.0.len() == 1 { "" } else { "s" }, errors)
}
Expand Down Expand Up @@ -1217,6 +1242,7 @@ impl Config {
root_ratoml: None,
root_ratoml_path,
detached_files: Default::default(),
other_errors: Default::default(),
}
}

Expand Down Expand Up @@ -2597,6 +2623,7 @@ macro_rules! _impl_for_config_data {
}
}


&self.default_config.global.$field
}
)*
Expand Down Expand Up @@ -3299,7 +3326,7 @@ fn validate_toml_table(
ptr.push_str(k);

match v {
// This is a table config, any entry in it is therefor valid
// This is a table config, any entry in it is therefore valid
toml::Value::Table(_) if verify(ptr) => (),
toml::Value::Table(table) => validate_toml_table(known_ptrs, table, ptr, error_sink),
_ if !verify(ptr) => error_sink
Expand Down
48 changes: 45 additions & 3 deletions crates/rust-analyzer/src/main_loop.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use std::{
fmt,
ops::Div as _,
ops::{Div as _, Not},
time::{Duration, Instant},
};

Expand All @@ -13,11 +13,12 @@ use ide_db::base_db::{SourceDatabase, SourceDatabaseExt, VfsPath};
use lsp_server::{Connection, Notification, Request};
use lsp_types::{notification::Notification as _, TextDocumentIdentifier};
use stdx::thread::ThreadIntent;
use tracing::{span, Level};
use tracing::{error, span, Level};
use triomphe::Arc;
use vfs::FileId;

use crate::{
config::Config,
config::{Config, ConfigChange},
diagnostics::{fetch_native_diagnostics, DiagnosticsGeneration},
dispatch::{NotificationDispatcher, RequestDispatcher},
global_state::{file_id_to_url, url_to_file_id, GlobalState},
Expand Down Expand Up @@ -146,6 +147,8 @@ impl GlobalState {
self.register_did_save_capability();
}

self.fetch_aux_files();

self.fetch_workspaces_queue.request_op("startup".to_owned(), false);
if let Some((cause, force_crate_graph_reload)) =
self.fetch_workspaces_queue.should_start_op()
Expand Down Expand Up @@ -1038,4 +1041,43 @@ impl GlobalState {
.finish();
Ok(())
}

fn fetch_aux_files(&mut self) {
let user_config_path = self.config.user_config_path();
let root_ratoml_path = self.config.root_ratoml_path();

{
let vfs = &mut self.vfs.write().0;
let loader = &mut self.loader;

if vfs.file_id(user_config_path).is_none() {
if let Some(user_cfg_abs) = user_config_path.as_path() {
let contents = loader.handle.load_sync(user_cfg_abs);
vfs.set_file_contents(user_config_path.clone(), contents);
} else {
error!("Non-abs virtual path for user config.");
}
}

if vfs.file_id(root_ratoml_path).is_none() {
// FIXME @alibektas : Sometimes root_path_ratoml collide with a regular ratoml.
// Although this shouldn't be a problem because everything is mapped to a `FileId`.
// We may want to further think about this.
if let Some(root_ratoml_abs) = root_ratoml_path.as_path() {
let contents = loader.handle.load_sync(root_ratoml_abs);
vfs.set_file_contents(root_ratoml_path.clone(), contents);
} else {
error!("Non-abs virtual path for user config.");
}
}
}

let mut config_change = ConfigChange::default();
config_change.change_source_root_parent_map(self.local_roots_parent_map.clone());

let (config, e, _) = self.config.apply_change(config_change);
self.config_errors = e.is_empty().not().then_some(e);

self.config = Arc::new(config);
}
}
Loading

0 comments on commit c81594d

Please sign in to comment.