Skip to content

Commit bd9758b

Browse files
committed
Minor fixes for ratoml module
- 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.
1 parent 1bb376c commit bd9758b

File tree

3 files changed

+133
-117
lines changed

3 files changed

+133
-117
lines changed

crates/rust-analyzer/src/config.rs

Lines changed: 72 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,6 @@ pub struct Config {
679679
/// | Windows | `{FOLDERID_RoamingAppData}` | C:\Users\Alice\AppData\Roaming |
680680
user_config_path: VfsPath,
681681

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

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

697+
/// Use case : It is an error to have an empty value for `check_command`.
698+
/// Since it is a `global` command at the moment, its final value can only be determined by
699+
/// traversing through `global` configs and the `client` config. However the non-null value constraint
700+
/// is config level agnostic, so this requires an independent error storage
701+
/// FIXME : bad name I know...
702+
other_errors: ConfigErrors,
703+
698704
detached_files: Vec<AbsPathBuf>,
699705
}
700706

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

719726
let mut should_update = false;
720727

@@ -741,8 +748,10 @@ impl Config {
741748
}
742749
}
743750

751+
744752
if let Some(mut json) = change.client_config_change {
745753
tracing::info!("updating config from JSON: {:#}", json);
754+
746755
if !(json.is_null() || json.as_object().map_or(false, |it| it.is_empty())) {
747756
let mut json_errors = vec![];
748757
let detached_files = get_field::<Vec<Utf8PathBuf>>(
@@ -758,6 +767,35 @@ impl Config {
758767

759768
patch_old_style::patch_json_for_outdated_configs(&mut json);
760769

770+
let snips = self.completion_snippets_custom().to_owned();
771+
772+
for (name, def) in snips.iter() {
773+
if def.prefix.is_empty() && def.postfix.is_empty() {
774+
continue;
775+
}
776+
let scope = match def.scope {
777+
SnippetScopeDef::Expr => SnippetScope::Expr,
778+
SnippetScopeDef::Type => SnippetScope::Type,
779+
SnippetScopeDef::Item => SnippetScope::Item,
780+
};
781+
#[allow(clippy::single_match)]
782+
match Snippet::new(
783+
&def.prefix,
784+
&def.postfix,
785+
&def.body,
786+
def.description.as_ref().unwrap_or(name),
787+
&def.requires,
788+
scope,
789+
) {
790+
Some(snippet) => config.snippets.push(snippet),
791+
None => json_errors.push((
792+
name.to_owned(),
793+
<serde_json::Error as serde::de::Error>::custom(format!(
794+
"snippet {name} is invalid or triggers are missing",
795+
)),
796+
)),
797+
}
798+
}
761799
config.client_config = (
762800
FullConfigInput::from_json(json, &mut json_errors),
763801
ConfigErrors(
@@ -797,8 +835,15 @@ impl Config {
797835
));
798836
should_update = true;
799837
}
800-
// FIXME
801-
Err(_) => (),
838+
Err(e) => {
839+
config.root_ratoml = Some((
840+
GlobalLocalConfigInput::from_toml(toml::map::Map::default(), &mut vec![]),
841+
ConfigErrors(vec![ConfigErrorInner::ParseError {
842+
reason: e.message().to_owned(),
843+
}
844+
.into()]),
845+
));
846+
}
802847
}
803848
}
804849

@@ -833,8 +878,18 @@ impl Config {
833878
),
834879
);
835880
}
836-
// FIXME
837-
Err(_) => (),
881+
Err(e) => {
882+
config.root_ratoml = Some((
883+
GlobalLocalConfigInput::from_toml(
884+
toml::map::Map::default(),
885+
&mut vec![],
886+
),
887+
ConfigErrors(vec![ConfigErrorInner::ParseError {
888+
reason: e.message().to_owned(),
889+
}
890+
.into()]),
891+
));
892+
}
838893
}
839894
}
840895
}
@@ -844,45 +899,13 @@ impl Config {
844899
config.source_root_parent_map = source_root_map;
845900
}
846901

847-
let snips = self.completion_snippets_custom().to_owned();
848-
849-
for (name, def) in snips.iter() {
850-
if def.prefix.is_empty() && def.postfix.is_empty() {
851-
continue;
852-
}
853-
let scope = match def.scope {
854-
SnippetScopeDef::Expr => SnippetScope::Expr,
855-
SnippetScopeDef::Type => SnippetScope::Type,
856-
SnippetScopeDef::Item => SnippetScope::Item,
857-
};
858-
#[allow(clippy::single_match)]
859-
match Snippet::new(
860-
&def.prefix,
861-
&def.postfix,
862-
&def.body,
863-
def.description.as_ref().unwrap_or(name),
864-
&def.requires,
865-
scope,
866-
) {
867-
Some(snippet) => config.snippets.push(snippet),
868-
// FIXME
869-
// None => error_sink.0.push(ConfigErrorInner::Json {
870-
// config_key: "".to_owned(),
871-
// error: <serde_json::Error as serde::de::Error>::custom(format!(
872-
// "snippet {name} is invalid or triggers are missing",
873-
// )),
874-
// }),
875-
None => (),
876-
}
902+
if config.check_command().is_empty() {
903+
config.other_errors.0.push(Arc::new(ConfigErrorInner::Json {
904+
config_key: "/check/command".to_owned(),
905+
error: serde_json::Error::custom("expected a non-empty string"),
906+
}));
877907
}
878908

879-
// FIXME: bring this back
880-
// if config.check_command().is_empty() {
881-
// error_sink.0.push(ConfigErrorInner::Json {
882-
// config_key: "/check/command".to_owned(),
883-
// error: serde_json::Error::custom("expected a non-empty string"),
884-
// });
885-
// }
886909
(config, should_update)
887910
}
888911

@@ -900,6 +923,7 @@ impl Config {
900923
.chain(config.root_ratoml.as_ref().into_iter().flat_map(|it| it.1 .0.iter()))
901924
.chain(config.user_config.as_ref().into_iter().flat_map(|it| it.1 .0.iter()))
902925
.chain(config.ratoml_files.values().flat_map(|it| it.1 .0.iter()))
926+
.chain(config.other_errors.0.iter())
903927
.cloned()
904928
.collect(),
905929
);
@@ -1140,9 +1164,10 @@ pub struct ClientCommandsConfig {
11401164
pub enum ConfigErrorInner {
11411165
Json { config_key: String, error: serde_json::Error },
11421166
Toml { config_key: String, error: toml::de::Error },
1167+
ParseError { reason: String },
11431168
}
11441169

1145-
#[derive(Clone, Debug)]
1170+
#[derive(Clone, Debug, Default)]
11461171
pub struct ConfigErrors(Vec<Arc<ConfigErrorInner>>);
11471172

11481173
impl ConfigErrors {
@@ -1164,6 +1189,7 @@ impl fmt::Display for ConfigErrors {
11641189
f(&": ")?;
11651190
f(e)
11661191
}
1192+
ConfigErrorInner::ParseError { reason } => f(reason),
11671193
});
11681194
write!(f, "invalid config value{}:\n{}", if self.0.len() == 1 { "" } else { "s" }, errors)
11691195
}
@@ -1217,6 +1243,7 @@ impl Config {
12171243
root_ratoml: None,
12181244
root_ratoml_path,
12191245
detached_files: Default::default(),
1246+
other_errors: Default::default(),
12201247
}
12211248
}
12221249

@@ -2597,6 +2624,7 @@ macro_rules! _impl_for_config_data {
25972624
}
25982625
}
25992626

2627+
26002628
&self.default_config.global.$field
26012629
}
26022630
)*
@@ -3299,7 +3327,7 @@ fn validate_toml_table(
32993327
ptr.push_str(k);
33003328

33013329
match v {
3302-
// This is a table config, any entry in it is therefor valid
3330+
// This is a table config, any entry in it is therefore valid
33033331
toml::Value::Table(_) if verify(ptr) => (),
33043332
toml::Value::Table(table) => validate_toml_table(known_ptrs, table, ptr, error_sink),
33053333
_ if !verify(ptr) => error_sink

crates/rust-analyzer/src/main_loop.rs

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
44
use std::{
55
fmt,
6-
ops::Div as _,
6+
ops::{Div as _, Not},
77
time::{Duration, Instant},
88
};
99

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

1920
use crate::{
20-
config::Config,
21+
config::{Config, ConfigChange},
2122
diagnostics::{fetch_native_diagnostics, DiagnosticsGeneration},
2223
dispatch::{NotificationDispatcher, RequestDispatcher},
2324
global_state::{file_id_to_url, url_to_file_id, GlobalState},
@@ -146,6 +147,8 @@ impl GlobalState {
146147
self.register_did_save_capability();
147148
}
148149

150+
self.fetch_aux_files();
151+
149152
self.fetch_workspaces_queue.request_op("startup".to_owned(), false);
150153
if let Some((cause, force_crate_graph_reload)) =
151154
self.fetch_workspaces_queue.should_start_op()
@@ -259,7 +262,7 @@ impl GlobalState {
259262
self.handle_queued_task(task);
260263
// Coalesce multiple task events into one loop turn
261264
while let Ok(task) = self.deferred_task_queue.receiver.try_recv() {
262-
self.handle_queued_task(task);
265+
self.handle_queued_task(task);
263266
}
264267
}
265268
Event::Task(task) => {
@@ -1038,4 +1041,45 @@ impl GlobalState {
10381041
.finish();
10391042
Ok(())
10401043
}
1044+
1045+
1046+
fn fetch_aux_files(&mut self) {
1047+
1048+
let user_config_path = self.config.user_config_path();
1049+
let root_ratoml_path = self.config.root_ratoml_path();
1050+
1051+
{
1052+
let vfs = &mut self.vfs.write().0;
1053+
let loader = &mut self.loader;
1054+
1055+
if vfs.file_id(user_config_path).is_none() {
1056+
if let Some(user_cfg_abs) = user_config_path.as_path() {
1057+
let contents = loader.handle.load_sync(user_cfg_abs);
1058+
vfs.set_file_contents(user_config_path.clone(), contents);
1059+
} else {
1060+
error!("Non-abs virtual path for user config.");
1061+
}
1062+
}
1063+
1064+
if vfs.file_id(root_ratoml_path).is_none() {
1065+
// FIXME @alibektas : Sometimes root_path_ratoml collide with a regular ratoml.
1066+
// Although this shouldn't be a problem because everything is mapped to a `FileId`.
1067+
// We may want to further think about this.
1068+
if let Some(root_ratoml_abs) = root_ratoml_path.as_path() {
1069+
let contents = loader.handle.load_sync(root_ratoml_abs);
1070+
vfs.set_file_contents(root_ratoml_path.clone(), contents);
1071+
} else {
1072+
error!("Non-abs virtual path for user config.");
1073+
}
1074+
}
1075+
}
1076+
1077+
let mut config_change = ConfigChange::default();
1078+
config_change.change_source_root_parent_map(self.local_roots_parent_map.clone());
1079+
1080+
let (config, e, _) = self.config.apply_change(config_change);
1081+
self.config_errors = e.is_empty().not().then_some(e);
1082+
1083+
self.config = Arc::new(config);
1084+
}
10411085
}

0 commit comments

Comments
 (0)